From: David Christensen on

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
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

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
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

First  |  Prev  | 
Pages: 1 2
Prev: Small FSM is too large
Next: deprecating =>, take two