quoting psql varible as identifier
in [PgSql]
|
Prev: Port for Microsoft Services for Unix (SFU) or SUA
Next: [HACKERS] solution to make static changes in pg_hba.conf file?
From: Robert Haas on 21 Jan 2010 12:33 On Thu, Jan 21, 2010 at 11:57 AM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > 2010/1/21 Robert Haas <robertmhaas(a)gmail.com>: >> On Tue, Jan 19, 2010 at 11:19 PM, Robert Haas <robertmhaas(a)gmail.com> wrote: >>> On Tue, Jan 19, 2010 at 4:40 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >>>> Robert Haas <robertmhaas(a)gmail.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. ....Robert -- 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: Pavel Stehule on 21 Jan 2010 12:53 2010/1/21 Robert Haas <robertmhaas(a)gmail.com>: > On Thu, Jan 21, 2010 at 11:57 AM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >> 2010/1/21 Robert Haas <robertmhaas(a)gmail.com>: >>> On Tue, Jan 19, 2010 at 11:19 PM, Robert Haas <robertmhaas(a)gmail.com> wrote: >>>> On Tue, Jan 19, 2010 at 4:40 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >>>>> Robert Haas <robertmhaas(a)gmail.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 > -- 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: Robert Haas on 21 Jan 2010 13:50 On Thu, Jan 21, 2010 at 12:53 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > 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 :( Eh? The only places where we should need new tests are the places that check PQExpBufferBroken() now - there are only 6 calls to that function in src/bin/psql and not all of them need to be changed. The places that do need to be changed will need to be modified to check PQExpBufferBroken() || lexer_coughed_up_a_lung. It should be possible to do this pretty simply, I think. ....Robert -- 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: Pavel Stehule on 21 Jan 2010 14:25 2010/1/21 Robert Haas <robertmhaas(a)gmail.com>: > On Thu, Jan 21, 2010 at 12:53 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >> 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 :( > > Eh? Â The only places where we should need new tests are the places > that check PQExpBufferBroken() now - there are only 6 calls to that > function in src/bin/psql and not all of them need to be changed. Â The > places that do need to be changed will need to be modified to check > PQExpBufferBroken() || lexer_coughed_up_a_lung. no, it is only 6 calls because we don't check psql_scan_slash_option result. When we don't allow escaping syntax in slash statement, then it could be simple. we have to do some like: if buffer_is_broken(..) report "out of memory" else if global_error != true do .... end if Pavel > > It should be possible to do this pretty simply, I think. > > ...Robert > -- 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: Robert Haas on 21 Jan 2010 23:34
On Thu, Jan 21, 2010 at 2:25 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > 2010/1/21 Robert Haas <robertmhaas(a)gmail.com>: >> On Thu, Jan 21, 2010 at 12:53 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: >>> 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 :( >> >> Eh? The only places where we should need new tests are the places >> that check PQExpBufferBroken() now - there are only 6 calls to that >> function in src/bin/psql and not all of them need to be changed. The >> places that do need to be changed will need to be modified to check >> PQExpBufferBroken() || lexer_coughed_up_a_lung. > > no, it is only 6 calls because we don't check psql_scan_slash_option result. psql_scan_slash_option() already has a way to signal errors - it can return NULL. Type any backslash command followed by a single quote... I'm not saying I love the way those errors are handled, but if we make this patch about revising the way psql does error handling, this is not going to get committed for this release... what we need to do is fit what we're trying to do into the existing model. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |