Prev: Small FSM is too large
Next: deprecating =>, take two
From: David Christensen on 18 Jul 2010 13:29 On Jul 18, 2010, at 12:17 PM, David Christensen wrote: > > On Jun 21, 2010, at 9:00 AM, Tom Lane wrote: > >> Robert Haas <robertmhaas(a)gmail.com> writes: >>> On Sun, Jun 20, 2010 at 10:51 PM, Steve Singer <ssinger_pg(a)sympatico.ca> wrote: >>>> One comment I have on the output format is that values (ie the database >>>> name) are enclosed in double quotes but the values being quoted can contain >>>> double quotes that are not being escaped. >> >> This is the same as standard practice in just about every other >> message... >> >>> It seems like for user and database it might be sensible to apply >>> PQescapeIdentifier to the value before printing it. >> >> I think this would actually be a remarkably bad idea in this particular >> instance, because in the majority of cases psql does not apply >> identifier dequoting rules to user and database names. What is printed >> should be the same as what you'd need to give to \connect, for example. > > > So I'm not quite sure how the above two paragraphs resolve? Should the user/database names be quoted or not? I have a new version of this patch available which has incorporated the feedback to this point? > > As an example of the current behavior, consider: > > machack:machack:5432=# create database "foo""bar" > machack-# ; > CREATE DATABASE > > [Sun Jul 18 12:14:49 CDT 2010] > machack:machack:5432=# \c foo"bar > unterminated quoted string > You are now connected to database "machack". > > [Sun Jul 18 12:14:53 CDT 2010] > machack:machack:5432=# \c "foo"bar" > unterminated quoted string > You are now connected to database "machack". > > [Sun Jul 18 12:14:59 CDT 2010] > machack:machack:5432=# \c "foo""bar" > You are now connected to database "foo"bar". > > As you can see, the value passed to connect differs from the output in the "connected to database" string. It's helpful when you attach said patch. This has been rebased to current HEAD. Regards, David -- David Christensen End Point Corporation david(a)endpoint.com
From: Tom Lane on 18 Jul 2010 13:30 David Christensen <david(a)endpoint.com> writes: > machack:machack:5432=# \c "foo""bar" > You are now connected to database "foo"bar". What this is reflecting is that backslash commands have their own weird rules for processing double quotes. What I was concerned about was that double quotes in SQL are normally used for protecting mixed case, and you don't need that for \c: regression=# create database "FooBar"; CREATE DATABASE regression=# \c foobar FATAL: database "foobar" does not exist Previous connection kept regression=# \c FooBar You are now connected to database "FooBar". FooBar=# The fact that there are double quotes around the database name in the "You are now connected..." message is *not* meant to imply that that is a valid double-quoted SQL identifier, either. It's just an artifact of how we set off names in English-language message style. In another language it might look like <<FooBar>> or some such. My opinion remains that you should just print the user and database names as-is, without trying to inject any quoting into the mix. You're more likely to confuse people than help them if you do that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: David Christensen on 18 Jul 2010 13:33 On Jul 18, 2010, at 12:30 PM, Tom Lane wrote: > David Christensen <david(a)endpoint.com> writes: >> machack:machack:5432=# \c "foo""bar" >> You are now connected to database "foo"bar". > > What this is reflecting is that backslash commands have their own weird > rules for processing double quotes. What I was concerned about was that > double quotes in SQL are normally used for protecting mixed case, and > you don't need that for \c: > > regression=# create database "FooBar"; > CREATE DATABASE > regression=# \c foobar > FATAL: database "foobar" does not exist > Previous connection kept > regression=# \c FooBar > You are now connected to database "FooBar". > FooBar=# > > The fact that there are double quotes around the database name in the > "You are now connected..." message is *not* meant to imply that that is > a valid double-quoted SQL identifier, either. It's just an artifact of > how we set off names in English-language message style. In another > language it might look like <<FooBar>> or some such. > > My opinion remains that you should just print the user and database > names as-is, without trying to inject any quoting into the mix. You're > more likely to confuse people than help them if you do that. Okay, understood. Then consider my updated patch (just sent attached to a recent message) to reflect the desired behavior. (I'll update the commitfest patch entry when it shows up in the archives.) Thanks, David -- David Christensen End Point Corporation david(a)endpoint.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Steve Singer on 18 Jul 2010 22:00
On Sun, 18 Jul 2010, David Christensen wrote: > > > It's helpful when you attach said patch. This has been rebased to current HEAD. One minor thing I noticed in the updated patch. You moved the '{' after the if(host) in command.c to it's own line(good) but you used spaces instead of tabstops there, the same with the else. > > Regards, > > David > -- > David Christensen > End Point Corporation > david(a)endpoint.com > > > > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |