Re: proposal \gcsv - Mailing list pgsql-hackers

Here's a WIP patch for the parenthesized-options route.

I realized that if we make the options be single words in the form
name=value, we can easily handle the shortcut forms with no value.
So that's what this does.

What this does *not* do is offer any solution to the question of
how to put a right paren as the last character of a pset option
value.  I don't really see any easy way to handle that, but maybe
we can punt for now.

Also no docs or test cases, but I see no point in putting effort into
that in advance of consensus that this is what we want.

0001 is some save/restore infrastructure that we'd need for pretty
much all of the proposals on the table, and then 0002 improves the
command itself.

            regards, tom lane

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index db31fa8..1055af5 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -160,8 +160,8 @@ static void minimal_error_message(PGresult *res);

 static void printSSLInfo(void);
 static void printGSSInfo(void);
-static bool printPsetInfo(const char *param, struct printQueryOpt *popt);
-static char *pset_value_string(const char *param, struct printQueryOpt *popt);
+static bool printPsetInfo(const char *param, printQueryOpt *popt);
+static char *pset_value_string(const char *param, printQueryOpt *popt);

 #ifdef WIN32
 static void checkWin32Codepage(void);
@@ -1302,7 +1302,11 @@ exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
         }
         free(fname);
         if (strcmp(cmd, "gx") == 0)
-            pset.g_expanded = true;
+        {
+            /* save settings, then force expanded = 1 */
+            pset.gsavepopt = savePsetInfo(&pset.popt);
+            pset.popt.topt.expanded = 1;
+        }
         status = PSQL_CMD_SEND;
     }
     else
@@ -3785,6 +3789,17 @@ _unicode_linestyle2string(int linestyle)
 /*
  * do_pset
  *
+ * Performs the assignment "param = value", where value could be NULL;
+ * for some params that has an effect such as inversion, for others
+ * it does nothing.
+ *
+ * Adjusts the state of the formatting options at *popt.  (In practice that
+ * is always pset.popt, but maybe someday it could be different.)
+ *
+ * If successful and quiet is false, then invokes printPsetInfo() to report
+ * the change.
+ *
+ * Returns true if successful, else false (eg for invalid param or value).
  */
 bool
 do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
@@ -4109,9 +4124,11 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
     return true;
 }

-
+/*
+ * printPsetInfo: print the state of the "param" formatting parameter in popt.
+ */
 static bool
-printPsetInfo(const char *param, struct printQueryOpt *popt)
+printPsetInfo(const char *param, printQueryOpt *popt)
 {
     Assert(param != NULL);

@@ -4292,6 +4309,77 @@ printPsetInfo(const char *param, struct printQueryOpt *popt)
     return true;
 }

+/*
+ * savePsetInfo: make a malloc'd copy of the data in *popt.
+ *
+ * Possibly this should be somewhere else, but it's a bit specific to psql.
+ */
+printQueryOpt *
+savePsetInfo(const printQueryOpt *popt)
+{
+    printQueryOpt *save;
+
+    save = (printQueryOpt *) pg_malloc(sizeof(printQueryOpt));
+
+    /* Flat-copy all the scalar fields, then duplicate sub-structures. */
+    memcpy(save, popt, sizeof(printQueryOpt));
+
+    /* topt.line_style points to const data that need not be duplicated */
+    if (popt->topt.fieldSep.separator)
+        save->topt.fieldSep.separator = pg_strdup(popt->topt.fieldSep.separator);
+    if (popt->topt.recordSep.separator)
+        save->topt.recordSep.separator = pg_strdup(popt->topt.recordSep.separator);
+    if (popt->topt.tableAttr)
+        save->topt.tableAttr = pg_strdup(popt->topt.tableAttr);
+    if (popt->nullPrint)
+        save->nullPrint = pg_strdup(popt->nullPrint);
+    if (popt->title)
+        save->title = pg_strdup(popt->title);
+
+    /*
+     * footers and translate_columns are never set in psql's print settings,
+     * so we needn't write code to duplicate them.
+     */
+    Assert(popt->footers == NULL);
+    Assert(popt->translate_columns == NULL);
+
+    return save;
+}
+
+/*
+ * restorePsetInfo: restore *popt from the previously-saved copy *save,
+ * then free *save.
+ */
+void
+restorePsetInfo(printQueryOpt *popt, printQueryOpt *save)
+{
+    /* Free all the old data we're about to overwrite the pointers to. */
+
+    /* topt.line_style points to const data that need not be duplicated */
+    if (popt->topt.fieldSep.separator)
+        free(popt->topt.fieldSep.separator);
+    if (popt->topt.recordSep.separator)
+        free(popt->topt.recordSep.separator);
+    if (popt->topt.tableAttr)
+        free(popt->topt.tableAttr);
+    if (popt->nullPrint)
+        free(popt->nullPrint);
+    if (popt->title)
+        free(popt->title);
+
+    /*
+     * footers and translate_columns are never set in psql's print settings,
+     * so we needn't write code to duplicate them.
+     */
+    Assert(popt->footers == NULL);
+    Assert(popt->translate_columns == NULL);
+
+    /* Now we may flat-copy all the fields, including pointers. */
+    memcpy(popt, save, sizeof(printQueryOpt));
+
+    /* Lastly, free "save" ... but its sub-structures now belong to popt. */
+    free(save);
+}

 static const char *
 pset_bool_string(bool val)
@@ -4339,7 +4427,7 @@ pset_quoted_string(const char *str)
  * output that produces the correct setting when fed back into \pset.
  */
 static char *
-pset_value_string(const char *param, struct printQueryOpt *popt)
+pset_value_string(const char *param, printQueryOpt *popt)
 {
     Assert(param != NULL);

diff --git a/src/bin/psql/command.h b/src/bin/psql/command.h
index 6113838..006832f 100644
--- a/src/bin/psql/command.h
+++ b/src/bin/psql/command.h
@@ -36,6 +36,10 @@ extern bool do_pset(const char *param,
                     printQueryOpt *popt,
                     bool quiet);

+extern printQueryOpt *savePsetInfo(const printQueryOpt *popt);
+
+extern void restorePsetInfo(printQueryOpt *popt, printQueryOpt *save);
+
 extern void connection_warnings(bool in_startup);

 extern void SyncVariables(void);
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 396a400..621a33f 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -707,13 +707,8 @@ PrintNotifications(void)
 static bool
 PrintQueryTuples(const PGresult *results)
 {
-    printQueryOpt my_popt = pset.popt;
     bool result = true;

-    /* one-shot expanded output requested via \gx */
-    if (pset.g_expanded)
-        my_popt.topt.expanded = 1;
-
     /* write output to \g argument, if any */
     if (pset.gfname)
     {
@@ -725,7 +720,7 @@ PrintQueryTuples(const PGresult *results)
         if (is_pipe)
             disable_sigpipe_trap();

-        printQuery(results, &my_popt, fout, false, pset.logfile);
+        printQuery(results, &pset.popt, fout, false, pset.logfile);
         if (ferror(fout))
         {
             pg_log_error("could not print result table: %m");
@@ -742,7 +737,7 @@ PrintQueryTuples(const PGresult *results)
     }
     else
     {
-        printQuery(results, &my_popt, pset.queryFout, false, pset.logfile);
+        printQuery(results, &pset.popt, pset.queryFout, false, pset.logfile);
         if (ferror(pset.queryFout))
         {
             pg_log_error("could not print result table: %m");
@@ -1418,8 +1413,12 @@ sendquery_cleanup:
         pset.gfname = NULL;
     }

-    /* reset \gx's expanded-mode flag */
-    pset.g_expanded = false;
+    /* restore print settings if \g changed them */
+    if (pset.gsavepopt)
+    {
+        restorePsetInfo(&pset.popt, pset.gsavepopt);
+        pset.gsavepopt = NULL;
+    }

     /* reset \gset trigger */
     if (pset.gset_prefix)
@@ -1646,10 +1645,6 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
              "FETCH FORWARD %d FROM _psql_cursor",
              fetch_count);

-    /* one-shot expanded output requested via \gx */
-    if (pset.g_expanded)
-        my_popt.topt.expanded = 1;
-
     /* prepare to write output to \g argument, if any */
     if (pset.gfname)
     {
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 2b384a3..97941aa 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -88,10 +88,11 @@ typedef struct _psqlSettings

     PGresult   *last_error_result;    /* most recent error result, if any */

-    printQueryOpt popt;
+    printQueryOpt popt;            /* The active print format settings */

     char       *gfname;            /* one-shot file output argument for \g */
-    bool        g_expanded;        /* one-shot expanded output requested via \gx */
+    printQueryOpt *gsavepopt;    /* if not null, saved print format settings */
+
     char       *gset_prefix;    /* one-shot prefix argument for \gset */
     bool        gdesc_flag;        /* one-shot request to describe query results */
     bool        gexec_flag;        /* one-shot request to execute query results */
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 1055af5..933e181 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -86,6 +86,10 @@ static backslashResult exec_command_errverbose(PsqlScanState scan_state, bool ac
 static backslashResult exec_command_f(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_g(PsqlScanState scan_state, bool active_branch,
                                       const char *cmd);
+static backslashResult process_command_g_options(char *first_option,
+                                                 PsqlScanState scan_state,
+                                                 bool active_branch,
+                                                 const char *cmd);
 static backslashResult exec_command_gdesc(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gexec(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gset(PsqlScanState scan_state, bool active_branch);
@@ -1280,19 +1284,40 @@ exec_command_f(PsqlScanState scan_state, bool active_branch)
 }

 /*
- * \g [filename] -- send query, optionally with output to file/pipe
- * \gx [filename] -- same as \g, with expanded mode forced
+ * \g  [(pset-option[=pset-value] ...)] [filename/shell-command]
+ * \gx [(pset-option[=pset-value] ...)] [filename/shell-command]
+ *
+ * Send the current query.  If pset options are specified, they are made
+ * active just for this query.  If a filename or pipe command is given,
+ * the query output goes there.  \gx implicitly forces expanded = 1 along
+ * with any other pset options that are specified.
  */
 static backslashResult
 exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
 {
     backslashResult status = PSQL_CMD_SKIP_LINE;
+    char       *fname;

-    if (active_branch)
+    /*
+     * Because the option processing for this is fairly complicated, we do it
+     * and then decide whether the branch is active.
+     */
+    fname = psql_scan_slash_option(scan_state,
+                                   OT_FILEPIPE, NULL, false);
+
+    if (fname && fname[0] == '(')
     {
-        char       *fname = psql_scan_slash_option(scan_state,
-                                                   OT_FILEPIPE, NULL, false);
+        /* Consume pset options through trailing ')' ... */
+        status = process_command_g_options(fname + 1, scan_state,
+                                           active_branch, cmd);
+        free(fname);
+        /* ... and again attempt to scan the filename. */
+        fname = psql_scan_slash_option(scan_state,
+                                       OT_FILEPIPE, NULL, false);
+    }

+    if (status == PSQL_CMD_SKIP_LINE && active_branch)
+    {
         if (!fname)
             pset.gfname = NULL;
         else
@@ -1300,22 +1325,89 @@ exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
             expand_tilde(&fname);
             pset.gfname = pg_strdup(fname);
         }
-        free(fname);
         if (strcmp(cmd, "gx") == 0)
         {
-            /* save settings, then force expanded = 1 */
-            pset.gsavepopt = savePsetInfo(&pset.popt);
+            /* save settings if not done already, then force expanded = 1 */
+            if (pset.gsavepopt == NULL)
+                pset.gsavepopt = savePsetInfo(&pset.popt);
             pset.popt.topt.expanded = 1;
         }
         status = PSQL_CMD_SEND;
     }
-    else
-        ignore_slash_filepipe(scan_state);
+
+    free(fname);

     return status;
 }

 /*
+ * Process parenthesized pset options for \g
+ */
+static backslashResult
+process_command_g_options(char *first_option, PsqlScanState scan_state,
+                          bool active_branch, const char *cmd)
+{
+    bool        success = true;
+    bool        found_r_paren = false;
+
+    do
+    {
+        char       *option;
+        size_t        optlen;
+
+        /* If not first time through, collect a new option */
+        if (first_option)
+            option = first_option;
+        else
+        {
+            option = psql_scan_slash_option(scan_state,
+                                            OT_NORMAL, NULL, false);
+            if (!option)
+            {
+                if (active_branch)
+                {
+                    pg_log_error("\\%s: missing right parenthesis", cmd);
+                    success = false;
+                }
+                break;
+            }
+        }
+
+        /* Check for terminating right paren, and remove it from string */
+        optlen = strlen(option);
+        if (optlen > 0 && option[optlen - 1] == ')')
+        {
+            option[--optlen] = '\0';
+            found_r_paren = true;
+        }
+
+        /* If there was anything besides right paren, parse/execute it */
+        if (optlen > 0)
+        {
+            char       *valptr = strchr(option, '=');
+
+            if (valptr)
+                *valptr++ = '\0';
+            if (active_branch)
+            {
+                /* save settings if not done already, then apply option */
+                if (pset.gsavepopt == NULL)
+                    pset.gsavepopt = savePsetInfo(&pset.popt);
+                success &= do_pset(option, valptr, &pset.popt, true);
+            }
+        }
+
+        /* Clean up after this option.  We should not free first_option. */
+        if (first_option)
+            first_option = NULL;
+        else
+            free(option);
+    } while (!found_r_paren);
+
+    return success ? PSQL_CMD_SKIP_LINE : PSQL_CMD_ERROR;
+}
+
+/*
  * \gdesc -- describe query result
  */
 static backslashResult