Thread: Variable substitution in psql backtick expansion

Variable substitution in psql backtick expansion

From
Tom Lane
Date:
Awhile back in the discussion about the \if feature for psql,
I'd pointed out that you shouldn't really need very much in
the way of boolean-expression evaluation smarts, because you
ought to be able to use a backtick shell escape:
\if `expr :foo \> :bar`    \echo :foo is greater than :bar\endif

Now that the basic feature is in, I went to play around with this,
and was disappointed to find out that it doesn't work.  The reason
is not far to seek: we do not do variable substitution within the
text between backticks.  psqlscanslash.l already foresaw that some
day we'd want to do that:
/* * backticked text: copy everything until next backquote, then evaluate. * * XXX Possible future behavioral change:
substitutefor :VARIABLE? */
 

I think today is that day, because it's going to make a material
difference to the usability of this feature.

I propose extending backtick processing so that

1. :VARIABLE is replaced by the contents of the variable

2. :'VARIABLE' is replaced by the contents of the variable,
single-quoted according to Unix shell conventions.  (So the
processing would be a bit different from what it is for the
same notation in SQL contexts.)

This doesn't look like it would take very much new code to do.

Thoughts?
        regards, tom lane



Re: Variable substitution in psql backtick expansion

From
Michael Paquier
Date:
On Fri, Mar 31, 2017 at 2:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Awhile back in the discussion about the \if feature for psql,
> I'd pointed out that you shouldn't really need very much in
> the way of boolean-expression evaluation smarts, because you
> ought to be able to use a backtick shell escape:
>
>         \if `expr :foo \> :bar`
>                 \echo :foo is greater than :bar
>         \endif
>
> Now that the basic feature is in, I went to play around with this,
> and was disappointed to find out that it doesn't work.  The reason
> is not far to seek: we do not do variable substitution within the
> text between backticks.  psqlscanslash.l already foresaw that some
> day we'd want to do that:
>
>         /*
>          * backticked text: copy everything until next backquote, then evaluate.
>          *
>          * XXX Possible future behavioral change: substitute for :VARIABLE?
>          */
>
> I think today is that day, because it's going to make a material
> difference to the usability of this feature.
>
> I propose extending backtick processing so that
>
> 1. :VARIABLE is replaced by the contents of the variable
>
> 2. :'VARIABLE' is replaced by the contents of the variable,
> single-quoted according to Unix shell conventions.  (So the
> processing would be a bit different from what it is for the
> same notation in SQL contexts.)
>
> This doesn't look like it would take very much new code to do.
>
> Thoughts?

In short, +1.
-- 
Michael



Re: Variable substitution in psql backtick expansion

From
Corey Huinker
Date:

On Thu, Mar 30, 2017 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
single-quoted according to Unix shell conventions.  (So the
processing would be a bit different from what it is for the
same notation in SQL contexts.)

+1
Having been bit by format '%L' prepending an 'E' to any string that happens to have a backslash in it, I'm in favor of this difference.

Any reason we wouldn't do :"VARIABLE" as well? People might expect it given its use elsewhere, and it would make possible things like

SELECT '$HOME/lamentable application name dir/bin/myprog' as myprog \gset
`:"myprog" arg1 arg2`

both for expanding $HOME and keeping the lamentable dir path as one arg.

Re: Variable substitution in psql backtick expansion

From
"Daniel Verite"
Date:
    Tom Lane wrote:

> Thoughts?

ISTM that expr is too painful to use to be seen as the
idiomatic way of achieving comparison in psql.

Among its disadvantages, it won't work on windows, and its
interface is hard to work with due to the necessary
quoting of half its operators, and the mandatory spacing
between arguments.

Also the quoting rules and command line syntax
depend on the underlying shell.
Isn't it going to be tricky to produce code that works
across different families of shells, like bourne and csh?

I think that users would rather have the option to just put
an SQL expression behind \if. That implies a working connection
to evaluate, which expr doesn't, but that's no
different from the other backslash commands that read
the database.


Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite



Re: Variable substitution in psql backtick expansion

From
Tom Lane
Date:
Corey Huinker <corey.huinker@gmail.com> writes:
> On Thu, Mar 30, 2017 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> single-quoted according to Unix shell conventions.  (So the
>> processing would be a bit different from what it is for the
>> same notation in SQL contexts.)

> Any reason we wouldn't do :"VARIABLE" as well?

Well, what would that mean exactly?  The charter of :'FOO', I think,
is to get the string value through shell parsing unscathed.  I'm a
lot less clear on what :"FOO" ought to do.  If it has any use then
I'd imagine that that includes allowing $shell_variable references in
the string to become expanded.  But then you have a situation where some
shell special characters in the string are supposed to take effect but
others presumably still aren't.  Figuring out the exact semantics would
take some specific use-cases, and more time than I really have available
right now.

In short, that's something I thought was best left as a later
refinement, rather than doing a rush job we might regret.

> People might expect it given
> its use elsewhere, and it would make possible things like

> SELECT '$HOME/lamentable application name dir/bin/myprog' as myprog \gset
> `:"myprog" arg1 arg2`

You could get about 80% of the way there with `":myprog" arg1 arg2`,
since backtick processing doesn't have any rule that would prevent
:myprog from being expanded inside shell double quotes.
        regards, tom lane



Re: Variable substitution in psql backtick expansion

From
Tom Lane
Date:
"Daniel Verite" <daniel@manitou-mail.org> writes:
> ISTM that expr is too painful to use to be seen as the
> idiomatic way of achieving comparison in psql.

I'm not proposing it as the best permanent solution, just saying
that having this in v10 is a lot better than having nothing in v10.
And we certainly aren't going to get any more-capable boolean
expression syntax into v10 at this point.

> Among its disadvantages, it won't work on windows, and its
> interface is hard to work with due to the necessary
> quoting of half its operators, and the mandatory spacing
> between arguments.
> Also the quoting rules and command line syntax
> depend on the underlying shell.

All true, but that's true of just about any use of backtick
or \! commands.  Shall we remove those features because they
are hard to use 100% portably?

> Isn't it going to be tricky to produce code that works
> across different families of shells, like bourne and csh?

Probably 95% of our users do not really care about that,
because they're only trying to produce scripts that will
work in their own environment.

> I think that users would rather have the option to just put
> an SQL expression behind \if. That implies a working connection
> to evaluate, which expr doesn't, but that's no
> different from the other backslash commands that read
> the database.

Well, it also implies being in a non-aborted transaction,
which seems like more of a problem to me for \if scripting.
Certainly there would be value in an option like that, but
it's not any more of a 100% solution than the `expr` one is.

Also, I didn't think I'd have to point it out, but expr(1)
is hardly the only command you can call from a backtick
substitution.  There are *lots* of potential use-cases
here ... but not so many if you can't plug any variable
material into the shell command.
        regards, tom lane



Re: Variable substitution in psql backtick expansion

From
Tom Lane
Date:
Corey Huinker <corey.huinker@gmail.com> writes:
> On Thu, Mar 30, 2017 at 1:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> single-quoted according to Unix shell conventions.  (So the
>> processing would be a bit different from what it is for the
>> same notation in SQL contexts.)

> +1

Here's a proposed patch for this.  I used the existing appendShellString()
logic, which already knows how to quote stuff safely on both Unix and
Windows.  A small problem is that appendShellString() rejects LF and CR
characters.  As written, it just printed an error and did exit(1), which
is more or less OK for existing callers but seemed like a pretty bad idea
for psql.  I refactored it to get the behavior proposed in the patch,
which is that we print an error and decline to substitute the variable's
value, leading to executing the backtick command with the :'variable'
text still in place.  This is more or less the same thing that happens
for encoding-check failures in the other variable-substitution cases,
so it didn't seem too unreasonable.

Perhaps it would be preferable to prevent execution of the backtick
command and/or fail the calling metacommand, but that seems to require
some very invasive refactoring (or else magic global variables), which
I didn't have the appetite for.

            regards, tom lane

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b51b11b..ad463e7 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 770,786 ****
      </para>

      <para>
-     Within an argument, text that is enclosed in backquotes
-     (<literal>`</literal>) is taken as a command line that is passed to the
-     shell. The output of the command (with any trailing newline removed)
-     replaces the backquoted text.
-     </para>
-
-     <para>
      If an unquoted colon (<literal>:</literal>) followed by a
      <application>psql</> variable name appears within an argument, it is
      replaced by the variable's value, as described in <xref
      linkend="APP-PSQL-interpolation" endterm="APP-PSQL-interpolation-title">.
      </para>

      <para>
--- 770,801 ----
      </para>

      <para>
      If an unquoted colon (<literal>:</literal>) followed by a
      <application>psql</> variable name appears within an argument, it is
      replaced by the variable's value, as described in <xref
      linkend="APP-PSQL-interpolation" endterm="APP-PSQL-interpolation-title">.
+     The forms <literal>:'<replaceable>variable_name</>'</literal> and
+     <literal>:"<replaceable>variable_name</>"</literal> described there
+     work as well.
+     </para>
+
+     <para>
+     Within an argument, text that is enclosed in backquotes
+     (<literal>`</literal>) is taken as a command line that is passed to the
+     shell.  The output of the command (with any trailing newline removed)
+     replaces the backquoted text.  Within the text enclosed in backquotes,
+     no special quoting or other processing occurs, except that appearances
+     of <literal>:<replaceable>variable_name</></literal> where
+     <replaceable>variable_name</> is a <application>psql</> variable name
+     are replaced by the variable's value.  Also, appearances of
+     <literal>:'<replaceable>variable_name</>'</literal> are replaced by the
+     variable's value suitably quoted to become a single shell command
+     argument.  (The latter form is almost always preferable, unless you are
+     very sure of what is in the variable.)  Because carriage return and line
+     feed characters cannot be safely quoted on all platforms, the
+     <literal>:'<replaceable>variable_name</>'</literal> form prints an
+     error message and does not substitute the variable value when such
+     characters appear in the value.
      </para>

      <para>
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index b06ae97..a2f1259 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** setQFout(const char *fname)
*** 116,134 ****
   * If the specified variable exists, return its value as a string (malloc'd
   * and expected to be freed by the caller); else return NULL.
   *
!  * If "escape" is true, return the value suitably quoted and escaped,
!  * as an identifier or string literal depending on "as_ident".
!  * (Failure in escaping should lead to returning NULL.)
   *
   * "passthrough" is the pointer previously given to psql_scan_set_passthrough.
   * In psql, passthrough points to a ConditionalStack, which we check to
   * determine whether variable expansion is allowed.
   */
  char *
! psql_get_variable(const char *varname, bool escape, bool as_ident,
                    void *passthrough)
  {
!     char       *result;
      const char *value;

      /* In an inactive \if branch, suppress all variable substitutions */
--- 116,134 ----
   * If the specified variable exists, return its value as a string (malloc'd
   * and expected to be freed by the caller); else return NULL.
   *
!  * If "quote" isn't PQUOTE_PLAIN, then return the value suitably quoted and
!  * escaped for the specified quoting requirement.  (Failure in escaping
!  * should lead to printing an error and returning NULL.)
   *
   * "passthrough" is the pointer previously given to psql_scan_set_passthrough.
   * In psql, passthrough points to a ConditionalStack, which we check to
   * determine whether variable expansion is allowed.
   */
  char *
! psql_get_variable(const char *varname, PsqlScanQuoteType quote,
                    void *passthrough)
  {
!     char       *result = NULL;
      const char *value;

      /* In an inactive \if branch, suppress all variable substitutions */
*************** psql_get_variable(const char *varname, b
*** 139,178 ****
      if (!value)
          return NULL;

!     if (escape)
      {
!         char       *escaped_value;

!         if (!pset.db)
!         {
!             psql_error("cannot escape without active connection\n");
!             return NULL;
!         }

!         if (as_ident)
!             escaped_value =
!                 PQescapeIdentifier(pset.db, value, strlen(value));
!         else
!             escaped_value =
!                 PQescapeLiteral(pset.db, value, strlen(value));

!         if (escaped_value == NULL)
!         {
!             const char *error = PQerrorMessage(pset.db);

!             psql_error("%s", error);
!             return NULL;
!         }

!         /*
!          * Rather than complicate the lexer's API with a notion of which
!          * free() routine to use, just pay the price of an extra strdup().
!          */
!         result = pg_strdup(escaped_value);
!         PQfreemem(escaped_value);
      }
-     else
-         result = pg_strdup(value);

      return result;
  }
--- 139,212 ----
      if (!value)
          return NULL;

!     switch (quote)
      {
!         case PQUOTE_PLAIN:
!             result = pg_strdup(value);
!             break;
!         case PQUOTE_SQL_LITERAL:
!         case PQUOTE_SQL_IDENT:
!             {
!                 /*
!                  * For these cases, we use libpq's quoting functions, which
!                  * assume the string is in the connection's client encoding.
!                  */
!                 char       *escaped_value;

!                 if (!pset.db)
!                 {
!                     psql_error("cannot escape without active connection\n");
!                     return NULL;
!                 }

!                 if (quote == PQUOTE_SQL_LITERAL)
!                     escaped_value =
!                         PQescapeLiteral(pset.db, value, strlen(value));
!                 else
!                     escaped_value =
!                         PQescapeIdentifier(pset.db, value, strlen(value));

!                 if (escaped_value == NULL)
!                 {
!                     const char *error = PQerrorMessage(pset.db);

!                     psql_error("%s", error);
!                     return NULL;
!                 }

!                 /*
!                  * Rather than complicate the lexer's API with a notion of
!                  * which free() routine to use, just pay the price of an extra
!                  * strdup().
!                  */
!                 result = pg_strdup(escaped_value);
!                 PQfreemem(escaped_value);
!                 break;
!             }
!         case PQUOTE_SHELL_ARG:
!             {
!                 /*
!                  * For this we use appendShellStringNoError, which is
!                  * encoding-agnostic, which is fine since the shell probably
!                  * is too.  In any case, the only special character is "'",
!                  * which is not known to appear in valid multibyte characters.
!                  */
!                 PQExpBufferData buf;
!
!                 initPQExpBuffer(&buf);
!                 if (!appendShellStringNoError(&buf, value))
!                 {
!                     psql_error("shell command argument contains a newline or carriage return: \"%s\"\n",
!                                value);
!                     free(buf.data);
!                     return NULL;
!                 }
!                 result = buf.data;
!                 break;
!             }
!
!             /* No default: we want a compiler warning for missing cases */
      }

      return result;
  }
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 3d8b8da..1ceb8ae 100644
*** a/src/bin/psql/common.h
--- b/src/bin/psql/common.h
***************
*** 12,22 ****

  #include "libpq-fe.h"
  #include "fe_utils/print.h"

  extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
  extern bool setQFout(const char *fname);

! extern char *psql_get_variable(const char *varname, bool escape, bool as_ident,
                    void *passthrough);

  extern void psql_error(const char *fmt,...) pg_attribute_printf(1, 2);
--- 12,23 ----

  #include "libpq-fe.h"
  #include "fe_utils/print.h"
+ #include "fe_utils/psqlscan.h"

  extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
  extern bool setQFout(const char *fname);

! extern char *psql_get_variable(const char *varname, PsqlScanQuoteType quote,
                    void *passthrough);

  extern void psql_error(const char *fmt,...) pg_attribute_printf(1, 2);
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 319afdc..db7a1b9 100644
*** a/src/bin/psql/psqlscanslash.l
--- b/src/bin/psql/psqlscanslash.l
*************** other            .
*** 242,249 ****
                                                               yytext + 1,
                                                               yyleng - 1);
                          value = cur_state->callbacks->get_variable(varname,
!                                                                    false,
!                                                                    false,
                                                                     cur_state->cb_passthrough);
                          free(varname);

--- 242,248 ----
                                                               yytext + 1,
                                                               yyleng - 1);
                          value = cur_state->callbacks->get_variable(varname,
!                                                                    PQUOTE_PLAIN,
                                                                     cur_state->cb_passthrough);
                          free(varname);

*************** other            .
*** 268,281 ****
                  }

  :'{variable_char}+'    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng, false);
                      *option_quote = ':';
                      unquoted_option_chars = 0;
                  }


  :\"{variable_char}+\"    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng, true);
                      *option_quote = ':';
                      unquoted_option_chars = 0;
                  }
--- 267,282 ----
                  }

  :'{variable_char}+'    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng,
!                                              PQUOTE_SQL_LITERAL);
                      *option_quote = ':';
                      unquoted_option_chars = 0;
                  }


  :\"{variable_char}+\"    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng,
!                                              PQUOTE_SQL_IDENT);
                      *option_quote = ':';
                      unquoted_option_chars = 0;
                  }
*************** other            .
*** 337,345 ****

  <xslashbackquote>{
      /*
!      * backticked text: copy everything until next backquote, then evaluate.
!      *
!      * XXX Possible future behavioral change: substitute for :VARIABLE?
       */

  "`"                {
--- 338,345 ----

  <xslashbackquote>{
      /*
!      * backticked text: copy everything until next backquote (expanding
!      * variable references, but doing nought else), then evaluate.
       */

  "`"                {
*************** other            .
*** 350,355 ****
--- 350,393 ----
                      BEGIN(xslasharg);
                  }

+ :{variable_char}+    {
+                     /* Possible psql variable substitution */
+                     if (cur_state->callbacks->get_variable == NULL)
+                         ECHO;
+                     else
+                     {
+                         char       *varname;
+                         char       *value;
+
+                         varname = psqlscan_extract_substring(cur_state,
+                                                              yytext + 1,
+                                                              yyleng - 1);
+                         value = cur_state->callbacks->get_variable(varname,
+                                                                    PQUOTE_PLAIN,
+                                                                    cur_state->cb_passthrough);
+                         free(varname);
+
+                         if (value)
+                         {
+                             appendPQExpBufferStr(output_buf, value);
+                             free(value);
+                         }
+                         else
+                             ECHO;
+                     }
+                 }
+
+ :'{variable_char}+'    {
+                     psqlscan_escape_variable(cur_state, yytext, yyleng,
+                                              PQUOTE_SHELL_ARG);
+                 }
+
+ :'{variable_char}*    {
+                     /* Throw back everything but the colon */
+                     yyless(1);
+                     ECHO;
+                 }
+
  {other}|\n        { ECHO; }

  }
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 19b3e57..27689d7 100644
*** a/src/fe_utils/psqlscan.l
--- b/src/fe_utils/psqlscan.l
*************** other            .
*** 699,706 ****
                                                           yyleng - 1);
                      if (cur_state->callbacks->get_variable)
                          value = cur_state->callbacks->get_variable(varname,
!                                                                    false,
!                                                                    false,
                                                                     cur_state->cb_passthrough);
                      else
                          value = NULL;
--- 699,705 ----
                                                           yyleng - 1);
                      if (cur_state->callbacks->get_variable)
                          value = cur_state->callbacks->get_variable(varname,
!                                                                    PQUOTE_PLAIN,
                                                                     cur_state->cb_passthrough);
                      else
                          value = NULL;
*************** other            .
*** 737,747 ****
                  }

  :'{variable_char}+'    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng, false);
                  }

  :\"{variable_char}+\"    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng, true);
                  }

      /*
--- 736,748 ----
                  }

  :'{variable_char}+'    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng,
!                                              PQUOTE_SQL_LITERAL);
                  }

  :\"{variable_char}+\"    {
!                     psqlscan_escape_variable(cur_state, yytext, yyleng,
!                                              PQUOTE_SQL_IDENT);
                  }

      /*
*************** psqlscan_extract_substring(PsqlScanState
*** 1415,1421 ****
   */
  void
  psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
!                          bool as_ident)
  {
      char       *varname;
      char       *value;
--- 1416,1422 ----
   */
  void
  psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
!                          PsqlScanQuoteType quote)
  {
      char       *varname;
      char       *value;
*************** psqlscan_escape_variable(PsqlScanState s
*** 1423,1429 ****
      /* Variable lookup. */
      varname = psqlscan_extract_substring(state, txt + 2, len - 3);
      if (state->callbacks->get_variable)
!         value = state->callbacks->get_variable(varname, true, as_ident,
                                                 state->cb_passthrough);
      else
          value = NULL;
--- 1424,1430 ----
      /* Variable lookup. */
      varname = psqlscan_extract_substring(state, txt + 2, len - 3);
      if (state->callbacks->get_variable)
!         value = state->callbacks->get_variable(varname, quote,
                                                 state->cb_passthrough);
      else
          value = NULL;
diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c
index d1a9ddc..dc84d32 100644
*** a/src/fe_utils/string_utils.c
--- b/src/fe_utils/string_utils.c
*************** appendByteaLiteral(PQExpBuffer buf, cons
*** 425,437 ****
--- 425,454 ----
   * arguments containing LF or CR characters.  A future major release should
   * reject those characters in CREATE ROLE and CREATE DATABASE, because use
   * there eventually leads to errors here.
+  *
+  * appendShellString() simply prints an error and dies if LF or CR appears.
+  * appendShellStringNoError() omits those characters from the result, and
+  * returns false if there were any.
   */
  void
  appendShellString(PQExpBuffer buf, const char *str)
  {
+     if (!appendShellStringNoError(buf, str))
+     {
+         fprintf(stderr,
+                 _("shell command argument contains a newline or carriage return: \"%s\"\n"),
+                 str);
+         exit(EXIT_FAILURE);
+     }
+ }
+
+ bool
+ appendShellStringNoError(PQExpBuffer buf, const char *str)
+ {
  #ifdef WIN32
      int            backslash_run_length = 0;
  #endif
+     bool        ok = true;
      const char *p;

      /*
*************** appendShellString(PQExpBuffer buf, const
*** 442,448 ****
          strspn(str, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_./:") == strlen(str))
      {
          appendPQExpBufferStr(buf, str);
!         return;
      }

  #ifndef WIN32
--- 459,465 ----
          strspn(str, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_./:") == strlen(str))
      {
          appendPQExpBufferStr(buf, str);
!         return ok;
      }

  #ifndef WIN32
*************** appendShellString(PQExpBuffer buf, const
*** 451,460 ****
      {
          if (*p == '\n' || *p == '\r')
          {
!             fprintf(stderr,
!                     _("shell command argument contains a newline or carriage return: \"%s\"\n"),
!                     str);
!             exit(EXIT_FAILURE);
          }

          if (*p == '\'')
--- 468,475 ----
      {
          if (*p == '\n' || *p == '\r')
          {
!             ok = false;
!             continue;
          }

          if (*p == '\'')
*************** appendShellString(PQExpBuffer buf, const
*** 481,490 ****
      {
          if (*p == '\n' || *p == '\r')
          {
!             fprintf(stderr,
!                     _("shell command argument contains a newline or carriage return: \"%s\"\n"),
!                     str);
!             exit(EXIT_FAILURE);
          }

          /* Change N backslashes before a double quote to 2N+1 backslashes. */
--- 496,503 ----
      {
          if (*p == '\n' || *p == '\r')
          {
!             ok = false;
!             continue;
          }

          /* Change N backslashes before a double quote to 2N+1 backslashes. */
*************** appendShellString(PQExpBuffer buf, const
*** 524,529 ****
--- 537,544 ----
      }
      appendPQExpBufferStr(buf, "^\"");
  #endif   /* WIN32 */
+
+     return ok;
  }


diff --git a/src/include/fe_utils/psqlscan.h b/src/include/fe_utils/psqlscan.h
index 0cc632b..e9c8143 100644
*** a/src/include/fe_utils/psqlscan.h
--- b/src/include/fe_utils/psqlscan.h
*************** typedef enum _promptStatus
*** 48,60 ****
      PROMPT_COPY
  } promptStatus_t;

  /* Callback functions to be used by the lexer */
  typedef struct PsqlScanCallbacks
  {
!     /* Fetch value of a variable, as a pfree'able string; NULL if unknown */
      /* This pointer can be NULL if no variable substitution is wanted */
!     char       *(*get_variable) (const char *varname, bool escape,
!                                            bool as_ident, void *passthrough);
      /* Print an error message someplace appropriate */
      /* (very old gcc versions don't support attributes on function pointers) */
  #if defined(__GNUC__) && __GNUC__ < 4
--- 48,69 ----
      PROMPT_COPY
  } promptStatus_t;

+ /* Quoting request types for get_variable() callback */
+ typedef enum
+ {
+     PQUOTE_PLAIN,                /* just return the actual value */
+     PQUOTE_SQL_LITERAL,            /* add quotes to make a valid SQL literal */
+     PQUOTE_SQL_IDENT,            /* quote if needed to make a SQL identifier */
+     PQUOTE_SHELL_ARG            /* quote if needed to be safe in a shell cmd */
+ } PsqlScanQuoteType;
+
  /* Callback functions to be used by the lexer */
  typedef struct PsqlScanCallbacks
  {
!     /* Fetch value of a variable, as a free'able string; NULL if unknown */
      /* This pointer can be NULL if no variable substitution is wanted */
!     char       *(*get_variable) (const char *varname, PsqlScanQuoteType quote,
!                                              void *passthrough);
      /* Print an error message someplace appropriate */
      /* (very old gcc versions don't support attributes on function pointers) */
  #if defined(__GNUC__) && __GNUC__ < 4
diff --git a/src/include/fe_utils/psqlscan_int.h b/src/include/fe_utils/psqlscan_int.h
index b4044e8..af62f5e 100644
*** a/src/include/fe_utils/psqlscan_int.h
--- b/src/include/fe_utils/psqlscan_int.h
*************** extern char *psqlscan_extract_substring(
*** 141,146 ****
                             const char *txt, int len);
  extern void psqlscan_escape_variable(PsqlScanState state,
                           const char *txt, int len,
!                          bool as_ident);

  #endif   /* PSQLSCAN_INT_H */
--- 141,146 ----
                             const char *txt, int len);
  extern void psqlscan_escape_variable(PsqlScanState state,
                           const char *txt, int len,
!                          PsqlScanQuoteType quote);

  #endif   /* PSQLSCAN_INT_H */
diff --git a/src/include/fe_utils/string_utils.h b/src/include/fe_utils/string_utils.h
index 6fb7f5e..c682343 100644
*** a/src/include/fe_utils/string_utils.h
--- b/src/include/fe_utils/string_utils.h
*************** extern void appendByteaLiteral(PQExpBuff
*** 42,47 ****
--- 42,48 ----
                     bool std_strings);

  extern void appendShellString(PQExpBuffer buf, const char *str);
+ extern bool appendShellStringNoError(PQExpBuffer buf, const char *str);
  extern void appendConnStrVal(PQExpBuffer buf, const char *str);
  extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname);


Re: Variable substitution in psql backtick expansion

From
"Daniel Verite"
Date:
    Fabien COELHO wrote:

> Now it could be decided that \set in psql stays simplistic because it is
> not needed as much as it is with pgbench. Fine with me.

It's not just that. It's that currently, if we do in psql:

\set d sqrt(1 + random(1000) * 17)

then we get that:

\echo :d
sqrt(1+random(1000)*17)

I assume we want to keep that pre-existing behavior of \set in
psql, that is, making a copy of that string and associating a
name to it, whereas I guess pgbench computes a value from it and
stores that value.

Certainly if we want the same sort of evaluator in pgbench and psql
we'd better share the code between them, but I don't think it will be
exposed by the same backslash commands in both programs,
if only for that backward compatibility concern.


Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite



Re: Variable substitution in psql backtick expansion

From
Fabien COELHO
Date:
> \set d sqrt(1 + random(1000) * 17)
> \echo :d
> sqrt(1+random(1000)*17)
>
> I assume we want to keep that pre-existing behavior of \set in
> psql,

Ok. So no interpreted expression ever in psql's \set for backward 
compatibility.

> that is, making a copy of that string and associating a name to it, 
> whereas I guess pgbench computes a value from it and stores that value.

Actually it stores the parsed tree, ready to be executed.

-- 
Fabien.