Re: quoting psql varible as identifier

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: quoting psql varible as identifier
Date: 2010-01-29 07:08:25
Message-ID: 162867791001282308y110a4c53yb53a36c81e2c1352@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
> First, you can't just remove support for the escape syntax from \d
> commands without some discussion of whether or not that's the right
> thing to do, and I don't think it is.  The cases where this will
> potentially cause a problem are limited to those where the input is
> invalidly encoded, and I don't think that's important enough to
> justify the surprise factor of having backslash commands behave
> differently from everything else.
>
> Second, even if it were OK to remove support for the escape syntax
> from \d commands, you failed to update the documentation you cribbed
> from my patch to match the behavior you implemented.

we can discus about programming style, but in this case I am sure. The
problem is \set command. We cannot ignore error in this case. In other
cases invalid escaping raises error, not in this case. So there is two
ways again:

a) remove escaped expansion from \command
b) implement \set command differently

>
> Third, you've reintroduced all of the code duplication that I
> eliminated in my version of this patch, as well as at least one bug -
> you've used free() where I believe you need PQfreemem().

you have a true.

I am also
> thinking that it doesn't make sense to push the result of
> PQescapeLiteral() or PQescapeIdentifier() as a new buffer, because we
> don't want to process variable expansions recursively.  At first I
> thought this was a security hole, but on further reflection I don't
> think it is: it'll rescan as a quoted string anyway, so any
> colon-escapes will be ignored.  But I believe it's unnecessary at any
> rate.
>

I think so it was a back door for scripting support in psql. It can
break backward compatibility!

> I would like to go ahead and commit my version of this patch and will
> do so later today if no one else objects.

yes, I have.

* your patch remove some feature without any warning and documentation
* your patch doesn't solve issue with \set command

Regards
Pavel

>
> ...Robert
>

Attachment Content-Type Size
variable-escaping-simple.patch text/x-patch 10.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Takahiro Itagaki 2010-01-29 07:15:54 Re: Largeobject Access Controls (r2460)
Previous Message Fujii Masao 2010-01-29 07:04:45 Re: Segmentation fault occurs when the standby becomes primary, in SR