Re: quoting psql varible as identifier

2010/1/21 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Thu, Jan 21, 2010 at 11:57 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> 2010/1/21 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>> On Tue, Jan 19, 2010 at 11:19 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>>> On Tue, Jan 19, 2010 at 4:40 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>>>> I'd like to proceed by committing an initial patch which changes the
>>>>>> "Escaping Strings for Inclusion in SQL Commands" to use a
>>>>>> <variablelist> with one <varlistentry> per function (as we do in
>>>>>> surrounding functions) and consolidates it with the following section,
>>>>>>  "Escaping Binary Strings for Inclusion in SQL Commands".  Then I'll
>>>>>> submit a patch implementing pqEscapeLiteral() and pqEscapeIdentifier()
>>>>>> as discussed here, and the doc diff hunks will actually be readable.
>>>>>
>>>>> Sounds like a plan.
>>>>
>>>> Initial commit done, and follow-on patch attached.  The docs took
>>>> longer to write than the code.  I spent a fair amount of time trying
>>>> to make it all make sense, but suggestions are welcome.
>>>
>>> Committed after fixing a couple of oversights in my doc changes.
>>
>> thank you.
>>
>> I actualised patch
>>
>> I thing, we need one libpq change more.
>>
>> + static void
>> + appendLiteral(PGconn *conn, PQExpBuffer buf, const char *str)
>> + {
>> +       char    *escaped_str;
>> +       size_t          len;
>> +
>> +       len = strlen(str);
>> +       escaped_str = PQescapeLiteral(conn, str, len);
>> +
>> +       if (escaped_str == NULL)
>> +       {
>> +               const char *error_message = PQerrorMessage(pset.db);
>> +
>> +               if (strlen(error_message))
>> +                       psql_error("%s", error_message);
>> +       }
>> +       else
>> +       {
>> +               appendPQExpBufferStr(buf, escaped_str);
>> +               free(escaped_str);
>> +       }
>> + }
>>
>> the correct result of this function (when is some error) is broken
>> buffer. But function markPQExpBufferBroken is static.
>
> markPQExpBufferBroken is specifically designed to handle out of memory
> errors.  I don't think we should try to generalize that to handle
> encoding violations or other things, at least not without some very
> careful thought about the consequences of so doing.  I think we need
> to find some other way of signalling an error back to the caller,
> although it's not exactly obvious to me what the best way to do that
> is.

BufferBroken flag is used as signal for out of memory now. But
everybody can use it for his purposes. There isn't any limit (what I
know). More - the behave is similar like NULL. If you have a broken
buffer, then everything with this is broken. Theoretically we could to
add some parser state flag and check it over every scanner call - but
it is duplicate. minimally when escape function returns null because
is out of memory, then breaking the buffer is semantically correct.

Currently there isn't consistency in memory handling :( - from good
reasons. Bad allocation from libpq is finished with raising a error
message. Out of memory from psql is raising fatal error. I am not
sure, if we can better join these two worlds. Maybe we can add malloc
handler to libpq (maybe in future)?

Still there are two kind of bugs - memory and encoding. These bugs
should be handled differently. In both cases we cannot stop lexers -
because we have to find end of statement, we cannot to execute broken
command.

my plan

add to state structure field like lexer_error. This field will be
checked before execution
it could be ugly for metacommands, there will be lot of new checks :(

Pavel

>
> ...Robert
>