Re: quoting psql varible as identifier
Pavel Stehule <pavel.stehule@gmail.com>
From: Pavel Stehule <pavel.stehule@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@commandprompt.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2010-01-29T07:08:25Z
Lists: pgsql-hackers
Attachments
- variable-escaping-simple.patch (text/x-patch) patch
> > 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 >