quoting psql varible as identifier

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

regards, tom lane

On Dec 29, 2009, at 8:57 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hello

I am working on new patch. I see a problem with copy quote_ident on
client side. This function call ScanKeywordLookup function.

const ScanKeyword *keyword = ScanKeywordLookup(ident,

ScanKeywords,

NumScanKeywords);

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No. It has to be client-side.

...Robert

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

ok

this function have to live in libpq - it needs some not exported
functionality. But, it would not be a problem.

Pavel

Pavel Stehule escribi�:

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

ok

this function have to live in libpq - it needs some not exported
functionality. But, it would not be a problem.

Can we use a trick similar to pg_dump's?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Pavel Stehule escribió:

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

ok

this function have to live in libpq - it needs some not exported
functionality. But, it would not be a problem.

Can we use a trick similar to pg_dump's?

??

Pavel

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Pavel Stehule escribió:

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

ok

this function have to live in libpq - it needs some not exported
functionality. But, it would not be a problem.

Can we use a trick similar to pg_dump's?

I see it - we can move function (content) fmtId from dumputils.c to libpq.

Pavel

Pavel Stehule escribi�:

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Pavel Stehule escribi�:

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

ok

this function have to live in libpq - it needs some not exported
functionality. But, it would not be a problem.

Can we use a trick similar to pg_dump's?

??

See src/bin/pg_dump/keywords.c and fmtId in dumputils.c

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Pavel Stehule escribió:

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Pavel Stehule escribió:

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

so we cannot simply implement quote_ident on client side :(. So we
have to use a real query.

It is acceptable for you?

No, certainly not --- that adds a boatload of failure conditions.
Just quote it unconditionally.

ok

this function have to live in libpq - it needs some not exported
functionality. But, it would not be a problem.

Can we use a trick similar to pg_dump's?

??

See src/bin/pg_dump/keywords.c and fmtId in dumputils.c

I found it. It is maybe too complex for using in psql.

Pavel

Pavel Stehule <pavel.stehule@gmail.com> writes:

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Can we use a trick similar to pg_dump's?

I see it - we can move function (content) fmtId from dumputils.c to libpq.

This is not a good idea. pg_dump can be expected to be up-to-date with
the backend's keyword list, but libpq cannot.

Just quote the thing unconditionally. It's not worth working harder
than that anyway.

regards, tom lane

2009/12/29 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

2009/12/29 Alvaro Herrera <alvherre@commandprompt.com>:

Can we use a trick similar to pg_dump's?

I see it - we can move function (content) fmtId from dumputils.c to libpq.

This is not a good idea.  pg_dump can be expected to be up-to-date with
the backend's keyword list, but libpq cannot.

Just quote the thing unconditionally.  It's not worth working harder
than that anyway.

I see it.

Pavel

hello

here is patch

pavel@postgres:5432=# \set foo 'hello world'
pavel@postgres:5432=# SELECT :'foo' AS :"foo";
hello world
-------------
hello world
(1 row)

Regards
Pavel

2009/12/29 Pavel Stehule <pavel.stehule@gmail.com>:

On Tue, Dec 29, 2009 at 3:19 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

here is patch

The error handling in quote_literal() doesn't look right to me. The
documentation for PQescapeStringConn says that it stores an error
message in the conn object, but your code ignores that and prints out
a generic message instead. That doesn't seem right: but then it
further goes on to call exit(1), which seems like a considerable
overreaction to an encoding violation, which is apparently the only
class of error PQescapeStringConn() is documented to throw.

...Robert

2009/12/30 Robert Haas <robertmhaas@gmail.com>:

On Tue, Dec 29, 2009 at 3:19 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

here is patch

The error handling in quote_literal() doesn't look right to me.  The
documentation for PQescapeStringConn says that it stores an error
message in the conn object, but your code ignores that and prints out
a generic message instead.  That doesn't seem right: but then it
further goes on to call exit(1), which seems like a considerable
overreaction to an encoding violation, which is apparently the only
class of error PQescapeStringConn() is documented to throw.

Actually I am not sure about the adequate solution. Now, the lexer
doesn't return any error. Any handled errors are fatal, and lexer (and
psql) is finished with exit(1) - so there are not checking status
after any lexer call. It is question if we have to do it. Because
error will be raised in next stage:

"Presently the only possible error conditions involve invalid
multibyte encoding in the source string. The output string is still
generated on error, but it can be expected that the server will reject
it as malformed. On error, a suitable message is stored in the conn
object, whether or not error is NULL."

http://www.postgresql.org/docs/8.4/static/libpq-exec.html#LIBPQ-EXEC-ESCAPE-STRING

so probably - we can quietly ignore this error without security or
functionality issues.

I see two solution:

a) print correct message and exit(1)
b) quietly ignore this error - it's more warning than error, because
error will be raised immediately

Third variant, checking lexer status over every call is maybe non
adequate to frequency of this error and an importance of this patch. I
am for a.

Regards, and happy new year
Pavel

Pavel Stehule <pavel.stehule@gmail.com> writes:

a) print correct message and exit(1)

Which part of "no, you're not doing that" wasn't clear to you?

The error handling in this function should be no different from that
in the existing escaping functions. exit(1) is completely unacceptable.

regards, tom lane

Pavel Stehule <pavel.stehule@gmail.com> writes:

here is patch

I looked at this patch a bit, and I think the real problem with it is
that it's not multibyte safe. You've copied backend code that is
allowed to assume it's in a safe encoding (ie, one where multibyte
characters can't contain non-high-bit-set bytes). This is not okay
on the client side, see SJIS and similar encodings.

Where you need to start out is by cloning PQescapeStringConn, which does
a similar type of transformation correctly even in unsafe encodings.
I think we'd agreed upthread that libpq should provide
PQescapeIdentifier functionality anyhow.

Once you've actually read that code, you'll realize that it's okay to
treat the error result as a warning, which resolves the other point
of concern. Just print the message and use the result anyway.

regards, tom lane

2010/1/2 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

a) print correct message and exit(1)

Which part of "no, you're not doing that" wasn't clear to you?

The error handling in this function should be no different from that
in the existing escaping functions.  exit(1) is completely unacceptable.

Are we talking we about error handling in psql lexer?

What I know, in psql there are no any escaping function now. Yes,
exit(1) is maybe too hard - but it is safe. I didn't write
PQescapeStringConn function, and I am not able to speak, we can ignore
this error result. Full handling of this possible error, means to add
some CHECK over any lexer call.

Pavel

2010/1/2 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

here is patch

I looked at this patch a bit, and I think the real problem with it is
that it's not multibyte safe.  You've copied backend code that is
allowed to assume it's in a safe encoding (ie, one where multibyte
characters can't contain non-high-bit-set bytes).  This is not okay
on the client side, see SJIS and similar encodings.

this code is taken from pg_dump, so if I understand it well, this is
littl bit different case.

Where you need to start out is by cloning PQescapeStringConn, which does
a similar type of transformation correctly even in unsafe encodings.
I think we'd agreed upthread that libpq should provide
PQescapeIdentifier functionality anyhow.

ok

Once you've actually read that code, you'll realize that it's okay to
treat the error result as a warning, which resolves the other point
of concern.  Just print the message and use the result anyway.

ok.

Pavel

hello

2010/1/2 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

here is patch

I looked at this patch a bit, and I think the real problem with it is
that it's not multibyte safe.  You've copied backend code that is
allowed to assume it's in a safe encoding (ie, one where multibyte
characters can't contain non-high-bit-set bytes).  This is not okay
on the client side, see SJIS and similar encodings.

Where you need to start out is by cloning PQescapeStringConn, which does
a similar type of transformation correctly even in unsafe encodings.
I think we'd agreed upthread that libpq should provide
PQescapeIdentifier functionality anyhow.

I am looking on psql directory. Now I found, so in this directory is
linked dumputil.c - It could little bit to help us.

I have one question. If I understand well, the function fmtId isn't
multibyte safe? So why is possible to use it in pg_dump?

Pavel

Hello

I talked with Hitoshi Harada, and fmtId function is safe (minimally
for Japanese case). This function working without any errors, so we
must not duplicate a code.

Pavel

2010/1/4 Pavel Stehule <pavel.stehule@gmail.com>: