2010/1/28 Robert Haas <robertmh...@gmail.com>:
> On Thu, Jan 28, 2010 at 4:53 AM, Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
>> 2010/1/27 Robert Haas <robertmh...@gmail.com>:
>>> On Mon, Jan 25, 2010 at 7:36 AM, Pavel Stehule <pavel.steh...@gmail.com>
>>> wrote:
>>>> I hope, so this version is more readable and more clean. I removed
>>>> some not necessary checks.
>>>
>>> This still seems overly complicated to me. I spent a few hours today
>>> working up the attached patch. Let me know your thoughts.
>>
>> There is serious issue. The "psql_error" only shows some message on
>> output, but do nothing more - you don't set a result status for
>> commands and for statements. So some potential error from parsing is
>> pseudo quietly ignored - without respect to your setting
>> ON_ERROR_STOP. This could be a problem for commands. Execution of
>> broken SQL statements will raise syntax error. But for \set some
>> variable will be a broken and the content can be used. I don't thing
>> so it is good. It is limited.
>
> Well, what you seem to be proposing to do is allow the command to
> execute (on the screwed-up data) and then afterwards pretend that it
> failed by overriding the return status. I think that's unacceptable.
> The root of the problem here is that the parsing and execution stages
> for backslash commands are not cleanly separated. There's no clean
> way for the lexer to return an error that allows the command to finish
> parsing normally but then doesn't execute it. Fixing that is going to
> require an extensive refactoring of commands.c which I don't think it
> makes sense to undertake at this point in the release cycle. Even if
> it did, it seems like material for a separate patch rather than
> something which has to be done before this goes in.
so I removed support for escaping from backslah commands and refactorised code.
I hope so this code is more verbose and clean than previous versions.
Regards
Pavel
>
>> Your version is acceptable only when we don't enable escape syntax for
>> commands. Then we don't need check it. On your version - I am not sure
>> if it is fully compatible, and using a global variables isn't nice.
>
> I'm not adding any new global variables - I'm just using the ones that
> are already there to avoid duplicating the same code four times.
> Referencing them from within the bodies of the lexer rules doesn't
> make the variables not global.
>
> ...Robert
>
*** ./doc/src/sgml/ref/psql-ref.sgml.orig 2009-12-25 00:36:39.000000000 +0100
--- ./doc/src/sgml/ref/psql-ref.sgml 2010-01-28 16:57:15.016331154 +0100
***************
*** 658,664 ****
<para>
If an unquoted argument begins with a colon (<literal>:</literal>),
it is taken as a <application>psql</> variable and the value of the
! variable is used as the argument instead.
</para>
<para>
--- 658,669 ----
<para>
If an unquoted argument begins with a colon (<literal>:</literal>),
it is taken as a <application>psql</> variable and the value of the
! variable is used as the argument instead. If the variable name is
! surrounded by single quotes (e.g. <literal>:'var'</literal>), it
! will be escaped as an SQL literal and the result will be used as
! the argument. If the variable name is surrounded by double quotes,
! it will be escaped as an SQL identifier and the result will be used
! as the argument.
</para>
<para>
***************
*** 2711,2728 ****
<para>
An additional useful feature of <application>psql</application>
variables is that you can substitute (<quote>interpolate</quote>)
! them into regular <acronym>SQL</acronym> statements. The syntax for
! this is again to prepend the variable name with a colon
(<literal>:</literal>):
<programlisting>
testdb=> <userinput>\set foo 'my_table'</userinput>
testdb=> <userinput>SELECT * FROM :foo;</userinput>
</programlisting>
! would then query the table <literal>my_table</literal>. The value of
! the variable is copied literally, so it can even contain unbalanced
! quotes or backslash commands. You must make sure that it makes sense
! where you put it. Variable interpolation will not be performed into
! quoted <acronym>SQL</acronym> entities.
</para>
<para>
--- 2716,2750 ----
<para>
An additional useful feature of <application>psql</application>
variables is that you can substitute (<quote>interpolate</quote>)
! them into regular <acronym>SQL</acronym> statements.
! <application>psql</application> provides special facilities for
! ensuring that values used as SQL literals and identifiers are
! properly escaped. The syntax for interpolating a value without
! any special escaping is again to prepend the variable name with a colon
(<literal>:</literal>):
<programlisting>
testdb=> <userinput>\set foo 'my_table'</userinput>
testdb=> <userinput>SELECT * FROM :foo;</userinput>
</programlisting>
! would then query the table <literal>my_table</literal>. Note that this
! may be unsafe: the value of the variable is copied literally, so it can
! even contain unbalanced quotes or backslash commands. You must make sure
! that it makes sense where you put it.
! </para>
!
! <para>
! When a value is to be used as an SQL literal or identifier, it is
! safest to arrange for it to be escaped. To escape the value of
! a variable as an SQL literal, write a colon followed by the variable
! name in single quotes. To escape the value an SQL identifier, write
! a colon followed by the variable name in double quotes. The previous
! example would be more safely written this way:
! <programlisting>
! testdb=> <userinput>\set foo 'my_table'</userinput>
! testdb=> <userinput>SELECT * FROM :"foo";</userinput>
! </programlisting>
! Variable interpolation will not be performed into quoted
! <acronym>SQL</acronym> entities.
</para>
<para>
***************
*** 2730,2769 ****
copy the contents of a file into a table column. First load the file into a
variable and then proceed as above:
<programlisting>
! testdb=> <userinput>\set content '''' `cat my_file.txt` ''''</userinput>
! testdb=> <userinput>INSERT INTO my_table VALUES (:content);</userinput>
! </programlisting>
! One problem with this approach is that <filename>my_file.txt</filename>
! might contain single quotes. These need to be escaped so that
! they don't cause a syntax error when the second line is processed. This
! could be done with the program <command>sed</command>:
! <programlisting>
! testdb=> <userinput>\set content '''' `sed -e "s/'/''/g" < my_file.txt` ''''</userinput>
! </programlisting>
! If you are using non-standard-conforming strings then you'll also need
! to double backslashes. This is a bit tricky:
! <programlisting>
! testdb=> <userinput>\set content '''' `sed -e "s/'/''/g" -e 's/\\/\\\\/g' < my_file.txt` ''''</userinput>
</programlisting>
! Note the use of different shell quoting conventions so that neither
! the single quote marks nor the backslashes are special to the shell.
! Backslashes are still special to <command>sed</command>, however, so
! we need to double them. (Perhaps
! at one point you thought it was great that all Unix commands use the
! same escape character.)
</para>
<para>
! Since colons can legally appear in SQL commands, the following rule
! applies: the character sequence
! <quote>:name</quote> is not changed unless <quote>name</> is the name
! of a variable that is currently set. In any case you can escape
! a colon with a backslash to protect it from substitution. (The
! colon syntax for variables is standard <acronym>SQL</acronym> for
embedded query languages, such as <application>ECPG</application>.
The colon syntax for array slices and type casts are
<productname>PostgreSQL</productname> extensions, hence the
! conflict.)
</para>
</refsect3>
--- 2752,2777 ----
copy the contents of a file into a table column. First load the file into a
variable and then proceed as above:
<programlisting>
! testdb=> <userinput>\set content `cat my_file.txt`</userinput>
! testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
</programlisting>
! (Note that this still won't work if my_file.txt contains NUL bytes.
! psql does not support embedded NUL bytes in variable values.)
</para>
<para>
! Since colons can legally appear in SQL commands, an apparent attempt
! at interpolation (such as <literal>:name</literal>,
! <literal>:'name'</literal>, or <literal>:"name"</literal>) is not
! changed unless the named variable is currently set. In any case you
! can escape a colon with a backslash to protect it from substitution.
! (The colon syntax for variables is standard <acronym>SQL</acronym> for
embedded query languages, such as <application>ECPG</application>.
The colon syntax for array slices and type casts are
<productname>PostgreSQL</productname> extensions, hence the
! conflict. The colon syntax for escaping a variable's value as an
! SQL literal or identifier is a <application>psql</application>
! extension.)
</para>
</refsect3>
*** ./src/bin/psql/psqlscan.l.orig 2010-01-02 17:57:59.000000000 +0100
--- ./src/bin/psql/psqlscan.l 2010-01-28 17:51:40.674834578 +0100
***************
*** 119,124 ****
--- 119,126 ----
static void emit(const char *txt, int len);
static bool is_utf16_surrogate_first(uint32 c);
+ static char *take_variable_name(const char *src, int len);
+
#define ECHO emit(yytext, yyleng)
%}
***************
*** 707,712 ****
--- 709,803 ----
}
}
+ :'[A-Za-z0-9_]+' {
+ /* Possible psql variable substitution */
+ const char *value;
+ char *varname = take_variable_name(yytext, yyleng);
+
+ value = GetVariable(pset.vars, varname);
+
+ if (value)
+ {
+ /* It is a variable, perform substitution */
+ char *escaped_value;
+
+ escaped_value = PQescapeLiteral(pset.db, value, strlen(value));
+ if (escaped_value != NULL)
+ {
+ push_new_buffer(escaped_value);
+ free(escaped_value);
+ }
+ else
+ {
+ const char *error_message = PQerrorMessage(pset.db);
+
+ if (strlen(error_message))
+ psql_error("%s", error_message);
+
+ /*
+ * when we cannot copy escaped string, then copy
+ * the string as as
+ */
+ ECHO;
+ }
+ /* yy_scan_string already made buffer active */
+ }
+ else
+ {
+ /*
+ * if the variable doesn't exist we'll copy the
+ * string as is
+ */
+ ECHO;
+ }
+
+ free(varname);
+ }
+
+ :\"[A-Za-z0-9_]+\" {
+ /* Possible psql variable substitution */
+ const char *value;
+ char *varname = take_variable_name(yytext, yyleng);
+
+ value = GetVariable(pset.vars, varname);
+
+ if (value)
+ {
+ /* It is a variable, perform substitution */
+ char *escaped_value;
+
+ escaped_value = PQescapeIdentifier(pset.db, value, strlen(value));
+ if (escaped_value != NULL)
+ {
+ push_new_buffer(escaped_value);
+ free(escaped_value);
+ }
+ else
+ {
+ const char *error_message = PQerrorMessage(pset.db);
+
+ if (strlen(error_message))
+ psql_error("%s", error_message);
+ /*
+ * when we cannot copy escaped string, then copy
+ * the string as as
+ */
+ ECHO;
+ }
+ /* yy_scan_string already made buffer active */
+ }
+ else
+ {
+ /*
+ * if the variable doesn't exist we'll copy the
+ * string as is
+ */
+ ECHO;
+ }
+
+ free(varname);
+ }
+
/*
* Back to backend-compatible rules.
*/
***************
*** 1740,1742 ****
--- 1831,1851 ----
{
return (c >= 0xD800 && c <= 0xDBFF);
}
+
+ /*
+ * remove colon and outer qoutes from text
+ *
+ */
+ static char *
+ take_variable_name(const char *src, int len)
+ {
+ char *result;
+
+ result = pg_malloc(len - 2);
+
+ /* skip first two chars (colon, quote) and stop before last char (quote) */
+ memcpy(result, src + 2, len - 3);
+ result[len - 3] = '\0';
+
+ return result;
+ }
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
