Обсуждение: \crosstabview fixes

Поиск
Список
Период
Сортировка

\crosstabview fixes

От
Tom Lane
Дата:
I noticed that the \crosstabview documentation asserts that column name
arguments are handled per standard SQL semantics.  In point of fact,
though, the patch expends a couple hundred lines to implement what is
NOT standard SQL semantics: matching unquoted names case-insensitively
is anything but that.  I think we should rip all that out and do it as
per attached.  (I also took the trouble to make the error messages conform
to project style.)

Comments/objections?

            regards, tom lane

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 227d180..b88f706 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 369,379 ****
      else if (strcmp(cmd, "crosstabview") == 0)
      {
          pset.ctv_col_V = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);
          pset.ctv_col_H = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);
          pset.ctv_col_D = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);

          pset.crosstab_flag = true;
          status = PSQL_CMD_SEND;
--- 369,379 ----
      else if (strcmp(cmd, "crosstabview") == 0)
      {
          pset.ctv_col_V = psql_scan_slash_option(scan_state,
!                                                 OT_SQLID, NULL, true);
          pset.ctv_col_H = psql_scan_slash_option(scan_state,
!                                                 OT_SQLID, NULL, true);
          pset.ctv_col_D = psql_scan_slash_option(scan_state,
!                                                 OT_SQLID, NULL, true);

          pset.crosstab_flag = true;
          status = PSQL_CMD_SEND;
diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index 3cc15ed..7889f63 100644
*** a/src/bin/psql/crosstabview.c
--- b/src/bin/psql/crosstabview.c
*************** static bool printCrosstab(const PGresult
*** 82,97 ****
                int num_columns, pivot_field *piv_columns, int field_for_columns,
                int num_rows, pivot_field *piv_rows, int field_for_rows,
                int field_for_data);
- static int parseColumnRefs(const char *arg, const PGresult *res,
-                 int **col_numbers,
-                 int max_columns, char separator);
  static void avlInit(avl_tree *tree);
  static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
  static int avlCollectFields(avl_tree *tree, avl_node *node,
                   pivot_field *fields, int idx);
  static void avlFree(avl_tree *tree, avl_node *node);
  static void rankSort(int num_columns, pivot_field *piv_columns);
! static int    indexOfColumn(const char *arg, const PGresult *res);
  static int    pivotFieldCompare(const void *a, const void *b);
  static int    rankCompare(const void *a, const void *b);

--- 82,95 ----
                int num_columns, pivot_field *piv_columns, int field_for_columns,
                int num_rows, pivot_field *piv_rows, int field_for_rows,
                int field_for_data);
  static void avlInit(avl_tree *tree);
  static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
  static int avlCollectFields(avl_tree *tree, avl_node *node,
                   pivot_field *fields, int idx);
  static void avlFree(avl_tree *tree, avl_node *node);
  static void rankSort(int num_columns, pivot_field *piv_columns);
! static int indexOfColumn(const char *arg, const PGresult *res,
!               bool printError);
  static int    pivotFieldCompare(const void *a, const void *b);
  static int    rankCompare(const void *a, const void *b);

*************** PrintResultsInCrosstab(const PGresult *r
*** 116,125 ****
      pivot_field *array_rows = NULL;
      int            num_columns = 0;
      int            num_rows = 0;
-     int           *colsV = NULL,
-                *colsH = NULL,
-                *colsD = NULL;
-     int            n;
      int            field_for_columns;
      int            sort_field_for_columns = -1;
      int            field_for_rows;
--- 114,119 ----
*************** PrintResultsInCrosstab(const PGresult *r
*** 129,155 ****
      avlInit(&piv_rows);
      avlInit(&piv_columns);

-     if (res == NULL)
-     {
-         psql_error(_("No result\n"));
-         goto error_return;
-     }
-
      if (PQresultStatus(res) != PGRES_TUPLES_OK)
      {
!         psql_error(_("The query must return results to be shown in crosstab\n"));
!         goto error_return;
!     }
!
!     if (opt_field_for_rows && !opt_field_for_columns)
!     {
!         psql_error(_("A second column must be specified for the horizontal header\n"));
          goto error_return;
      }

!     if (PQnfields(res) <= 2)
      {
!         psql_error(_("The query must return at least two columns to be shown in crosstab\n"));
          goto error_return;
      }

--- 123,137 ----
      avlInit(&piv_rows);
      avlInit(&piv_columns);

      if (PQresultStatus(res) != PGRES_TUPLES_OK)
      {
!         psql_error(_("query must return results to be shown in crosstab\n"));
          goto error_return;
      }

!     if (PQnfields(res) < 3)
      {
!         psql_error(_("query must return at least three columns to be shown in crosstab\n"));
          goto error_return;
      }

*************** PrintResultsInCrosstab(const PGresult *r
*** 158,209 ****
       * left-most column. Only a reference to a field is accepted (no sort
       * column).
       */
-
      if (opt_field_for_rows == NULL)
      {
          field_for_rows = 0;
      }
      else
      {
!         n = parseColumnRefs(opt_field_for_rows, res, &colsV, 1, ':');
!         if (n != 1)
              goto error_return;
-         field_for_rows = colsV[0];
      }

-     if (field_for_rows < 0)
-         goto error_return;
-
      /*----------
       * Arguments processing for the horizontal header (2nd arg)
       * (pivoted column that gets displayed as the first row).
       * Determine:
       * - the field number for the horizontal header column
       * - the field number of the associated sort column, if any
       */
-
      if (opt_field_for_columns == NULL)
          field_for_columns = 1;
      else
      {
!         n = parseColumnRefs(opt_field_for_columns, res, &colsH, 2, ':');
!         if (n <= 0)
!             goto error_return;
!         if (n == 1)
!             field_for_columns = colsH[0];
          else
          {
!             field_for_columns = colsH[0];
!             sort_field_for_columns = colsH[1];
          }
-
-         if (field_for_columns < 0)
-             goto error_return;
      }

      if (field_for_columns == field_for_rows)
      {
!         psql_error(_("The same column cannot be used for both vertical and horizontal headers\n"));
          goto error_return;
      }

--- 140,200 ----
       * left-most column. Only a reference to a field is accepted (no sort
       * column).
       */
      if (opt_field_for_rows == NULL)
      {
          field_for_rows = 0;
      }
      else
      {
!         field_for_rows = indexOfColumn(opt_field_for_rows, res, true);
!         if (field_for_rows < 0)
              goto error_return;
      }

      /*----------
       * Arguments processing for the horizontal header (2nd arg)
       * (pivoted column that gets displayed as the first row).
       * Determine:
       * - the field number for the horizontal header column
       * - the field number of the associated sort column, if any
+      *----------
       */
      if (opt_field_for_columns == NULL)
          field_for_columns = 1;
      else
      {
!         char       *colonpos = strchr(opt_field_for_columns, ':');
!
!         if (colonpos == NULL)
!         {
!             /* No colon, so must be a simple column ID */
!             field_for_columns = indexOfColumn(opt_field_for_columns, res, true);
!             if (field_for_columns < 0)
!                 goto error_return;
!         }
          else
          {
!             /* Try it as simple column ID, but don't demand a match */
!             field_for_columns = indexOfColumn(opt_field_for_columns, res, false);
!             if (field_for_columns < 0)
!             {
!                 /* Split at the colon, and demand matches */
!                 /* It's okay to trash pset.ctv_col_H here */
!                 *colonpos++ = '\0';
!                 field_for_columns = indexOfColumn(opt_field_for_columns, res, true);
!                 if (field_for_columns < 0)
!                     goto error_return;
!                 sort_field_for_columns = indexOfColumn(colonpos, res, true);
!                 if (sort_field_for_columns < 0)
!                     goto error_return;
!             }
          }
      }

+     /* Insist that header columns be distinct */
      if (field_for_columns == field_for_rows)
      {
!         psql_error(_("vertical and horizontal headers must be different columns\n"));
          goto error_return;
      }

*************** PrintResultsInCrosstab(const PGresult *r
*** 217,228 ****

          /*
           * If the data column was not specified, we search for the one not
!          * used as either vertical or horizontal headers.  If the result has
!          * more than three columns, raise an error.
           */
!         if (PQnfields(res) > 3)
          {
!             psql_error(_("Data column must be specified when the result set has more than three columns\n"));
              goto error_return;
          }

--- 208,219 ----

          /*
           * If the data column was not specified, we search for the one not
!          * used as either vertical or horizontal headers.  Must be exactly
!          * three columns, or this won't be unique.
           */
!         if (PQnfields(res) != 3)
          {
!             psql_error(_("data column must be specified when result set has more than three columns\n"));
              goto error_return;
          }

*************** PrintResultsInCrosstab(const PGresult *r
*** 238,250 ****
      }
      else
      {
!         int        num_fields;
!
!         /* If a field was given, find out what it is.  Only one is allowed. */
!         num_fields = parseColumnRefs(opt_field_for_data, res, &colsD, 1, ',');
!         if (num_fields < 1)
              goto error_return;
-         field_for_data = colsD[0];
      }

      /*
--- 229,237 ----
      }
      else
      {
!         field_for_data = indexOfColumn(opt_field_for_data, res, true);
!         if (field_for_data < 0)
              goto error_return;
      }

      /*
*************** PrintResultsInCrosstab(const PGresult *r
*** 271,277 ****

          if (piv_columns.count > CROSSTABVIEW_MAX_COLUMNS)
          {
!             psql_error(_("Maximum number of columns (%d) exceeded\n"),
                         CROSSTABVIEW_MAX_COLUMNS);
              goto error_return;
          }
--- 258,264 ----

          if (piv_columns.count > CROSSTABVIEW_MAX_COLUMNS)
          {
!             psql_error(_("maximum number of crosstab columns (%d) exceeded\n"),
                         CROSSTABVIEW_MAX_COLUMNS);
              goto error_return;
          }
*************** error_return:
*** 319,327 ****
      avlFree(&piv_rows, piv_rows.root);
      pg_free(array_columns);
      pg_free(array_rows);
-     pg_free(colsV);
-     pg_free(colsH);
-     pg_free(colsD);

      return retval;
  }
--- 306,311 ----
*************** printCrosstab(const PGresult *results,
*** 442,448 ****
               */
              if (cont.cells[idx] != NULL)
              {
!                 psql_error(_("data cell already contains a value: (row: \"%s\", column: \"%s\")\n"),
                               piv_rows[row_number].name ? piv_rows[row_number].name :
                               popt.nullPrint ? popt.nullPrint : "(null)",
                               piv_columns[col_number].name ? piv_columns[col_number].name :
--- 426,432 ----
               */
              if (cont.cells[idx] != NULL)
              {
!                 psql_error(_("query contains multiple data values for row \"%s\", column \"%s\"\n"),
                               piv_rows[row_number].name ? piv_rows[row_number].name :
                               popt.nullPrint ? popt.nullPrint : "(null)",
                               piv_columns[col_number].name ? piv_columns[col_number].name :
*************** error:
*** 476,583 ****
  }

  /*
-  * Parse "arg", which is a string of column IDs separated by "separator".
-  *
-  * Each column ID can be:
-  * - a number from 1 to PQnfields(res)
-  * - an unquoted column name matching (case insensitively) one of PQfname(res,...)
-  * - a quoted column name matching (case sensitively) one of PQfname(res,...)
-  *
-  * If max_columns > 0, it is the max number of column IDs allowed.
-  *
-  * On success, return number of column IDs found (possibly 0), and return a
-  * malloc'd array of the matching column numbers of "res" into *col_numbers.
-  *
-  * On failure, return -1 and set *col_numbers to NULL.
-  */
- static int
- parseColumnRefs(const char *arg,
-                 const PGresult *res,
-                 int **col_numbers,
-                 int max_columns,
-                 char separator)
- {
-     const char *p = arg;
-     char        c;
-     int            num_cols = 0;
-
-     *col_numbers = NULL;
-     while ((c = *p) != '\0')
-     {
-         const char *field_start = p;
-         bool        quoted_field = false;
-
-         /* first char */
-         if (c == '"')
-         {
-             quoted_field = true;
-             p++;
-         }
-
-         while ((c = *p) != '\0')
-         {
-             if (c == separator && !quoted_field)
-                 break;
-             if (c == '"')        /* end of field or embedded double quote */
-             {
-                 p++;
-                 if (*p == '"')
-                 {
-                     if (quoted_field)
-                     {
-                         p++;
-                         continue;
-                     }
-                 }
-                 else if (quoted_field && *p == separator)
-                     break;
-             }
-             if (*p)
-                 p += PQmblen(p, pset.encoding);
-         }
-
-         if (p != field_start)
-         {
-             char   *col_name;
-             int        col_num;
-
-             /* enforce max_columns limit */
-             if (max_columns > 0 && num_cols == max_columns)
-             {
-                 psql_error(_("No more than %d column references expected\n"),
-                            max_columns);
-                 goto errfail;
-             }
-             /* look up the column and add its index into *col_numbers */
-             col_name = pg_malloc(p - field_start + 1);
-             memcpy(col_name, field_start, p - field_start);
-             col_name[p - field_start] = '\0';
-             col_num = indexOfColumn(col_name, res);
-             pg_free(col_name);
-             if (col_num < 0)
-                 goto errfail;
-             *col_numbers = (int *) pg_realloc(*col_numbers,
-                                               (num_cols + 1) * sizeof(int));
-             (*col_numbers)[num_cols++] = col_num;
-         }
-         else
-         {
-             psql_error(_("Empty column reference\n"));
-             goto errfail;
-         }
-
-         if (*p)
-             p += PQmblen(p, pset.encoding);
-     }
-     return num_cols;
-
- errfail:
-     pg_free(*col_numbers);
-     *col_numbers = NULL;
-     return -1;
- }
-
- /*
   * The avl* functions below provide a minimalistic implementation of AVL binary
   * trees, to efficiently collect the distinct values that will form the horizontal
   * and vertical headers. It only supports adding new values, no removal or even
--- 460,465 ----
*************** rankSort(int num_columns, pivot_field *p
*** 773,833 ****
  }

  /*
!  * Compare a user-supplied argument against a field name obtained by PQfname(),
!  * which is already case-folded.
!  * If arg is not enclosed in double quotes, pg_strcasecmp applies, otherwise
!  * do a case-sensitive comparison with these rules:
!  * - double quotes enclosing 'arg' are filtered out
!  * - double quotes inside 'arg' are expected to be doubled
!  */
! static bool
! fieldNameEquals(const char *arg, const char *fieldname)
! {
!     const char *p = arg;
!     const char *f = fieldname;
!     char        c;
!
!     if (*p++ != '"')
!         return (pg_strcasecmp(arg, fieldname) == 0);
!
!     while ((c = *p++))
!     {
!         if (c == '"')
!         {
!             if (*p == '"')
!                 p++;            /* skip second quote and continue */
!             else if (*p == '\0')
!                 return (*f == '\0');    /* p is shorter than f, or is
!                                          * identical */
!         }
!         if (*f == '\0')
!             return false;        /* f is shorter than p */
!         if (c != *f)            /* found one byte that differs */
!             return false;
!         f++;
!     }
!     return (*f == '\0');
! }
!
! /*
!  * arg can be a number or a column name, possibly quoted (like in an ORDER BY clause)
!  * Returns:
!  *    on success, the 0-based index of the column
!  *    or -1 if the column number or name is not found in the result's structure,
!  *          or if it's ambiguous (arg corresponding to several columns)
   */
  static int
! indexOfColumn(const char *arg, const PGresult *res)
  {
      int            idx;

!     if (strspn(arg, "0123456789") == strlen(arg))
      {
          /* if arg contains only digits, it's a column number */
          idx = atoi(arg) - 1;
          if (idx < 0 || idx >= PQnfields(res))
          {
!             psql_error(_("Invalid column number: %s\n"), arg);
              return -1;
          }
      }
--- 655,680 ----
  }

  /*
!  * Look up a column reference, which can be either:
!  * - a number from 1 to PQnfields(res)
!  * - a column name matching one of PQfname(res,...)
!  *
!  * Returns zero-based column number, or -1 if not found or ambiguous.
!  * On failure return, a suitable error is printed if printError is true.
   */
  static int
! indexOfColumn(const char *arg, const PGresult *res, bool printError)
  {
      int            idx;

!     if (arg[0] && strspn(arg, "0123456789") == strlen(arg))
      {
          /* if arg contains only digits, it's a column number */
          idx = atoi(arg) - 1;
          if (idx < 0 || idx >= PQnfields(res))
          {
!             if (printError)
!                 psql_error(_("invalid column number: \"%s\"\n"), arg);
              return -1;
          }
      }
*************** indexOfColumn(const char *arg, const PGr
*** 838,849 ****
          idx = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
!             if (fieldNameEquals(arg, PQfname(res, i)))
              {
                  if (idx >= 0)
                  {
!                     /* if another idx was already found for the same name */
!                     psql_error(_("Ambiguous column name: %s\n"), arg);
                      return -1;
                  }
                  idx = i;
--- 685,697 ----
          idx = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
!             if (strcmp(arg, PQfname(res, i)) == 0)
              {
                  if (idx >= 0)
                  {
!                     /* another idx was already found for the same name */
!                     if (printError)
!                         psql_error(_("ambiguous column name: \"%s\"\n"), arg);
                      return -1;
                  }
                  idx = i;
*************** indexOfColumn(const char *arg, const PGr
*** 851,857 ****
          }
          if (idx == -1)
          {
!             psql_error(_("Invalid column name: %s\n"), arg);
              return -1;
          }
      }
--- 699,706 ----
          }
          if (idx == -1)
          {
!             if (printError)
!                 psql_error(_("column name not found: \"%s\"\n"), arg);
              return -1;
          }
      }
diff --git a/src/test/regress/expected/psql_crosstab.out b/src/test/regress/expected/psql_crosstab.out
index c87c2fc..6e7fd08 100644
*** a/src/test/regress/expected/psql_crosstab.out
--- b/src/test/regress/expected/psql_crosstab.out
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 112,118 ****

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
! The query must return at least two columns to be shown in crosstab
  -- only null, no column name, 3 columns: works
  SELECT null,null,null \crosstabview
   ?column? |
--- 112,118 ----

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
! query must return at least three columns to be shown in crosstab
  -- only null, no column name, 3 columns: works
  SELECT null,null,null \crosstabview
   ?column? |
*************** FROM ctv_data GROUP BY v, h ORDER BY h,v
*** 166,185 ****
  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j
! Invalid column name: j
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5
! Invalid column number: 5
  -- error: same H and V columns
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 h 4
! The same column cannot be used for both vertical and horizontal headers
  -- error: too many columns
  SELECT a,a,1 FROM generate_series(1,3000) AS a
   \crosstabview
! Maximum number of columns (1600) exceeded
  -- error: only one column
  SELECT 1 \crosstabview
! The query must return at least two columns to be shown in crosstab
  DROP TABLE ctv_data;
--- 166,185 ----
  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j
! column name not found: "j"
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5
! invalid column number: "5"
  -- error: same H and V columns
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 h 4
! vertical and horizontal headers must be different columns
  -- error: too many columns
  SELECT a,a,1 FROM generate_series(1,3000) AS a
   \crosstabview
! maximum number of crosstab columns (1600) exceeded
  -- error: only one column
  SELECT 1 \crosstabview
! query must return at least three columns to be shown in crosstab
  DROP TABLE ctv_data;

Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Tom Lane wrote:

> I noticed that the \crosstabview documentation asserts that column name
> arguments are handled per standard SQL semantics.  In point of fact,
> though, the patch expends a couple hundred lines to implement what is
> NOT standard SQL semantics: matching unquoted names case-insensitively
> is anything but that.  I think we should rip all that out and do it as
> per attached.  (I also took the trouble to make the error messages conform
> to project style.)
>
> Comments/objections?

+1 for ripping it out in favor of the option parser with OT_SQLID,
but then there's a problem with the colH:scolH syntax.
For instance when refering to columns by positions, with the
patch applied, it accepts this invocation without error: \crosstabview 1 "2:4" 3
whereas before it produced this error:Invalid column name: "2:4"

To avoid the confusion between "2:4" and "2":"4" or 2:4,
and the ambiguity with a possibly existing "2:4" column,
maybe we should abandon this syntax and require the optional
scolH to be on its own at the end of the command.

The simplified invocation of the command would be \crosstabview colV colH colD [scolH]
(without any default, just scolH being optional):

Or if it's preferrable to have colH just near scolH: \crosstabview colD colV colH [scolH]


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



Re: \crosstabview fixes

От
Alvaro Herrera
Дата:
Tom Lane wrote:
> I noticed that the \crosstabview documentation asserts that column name
> arguments are handled per standard SQL semantics.  In point of fact,
> though, the patch expends a couple hundred lines to implement what is
> NOT standard SQL semantics: matching unquoted names case-insensitively
> is anything but that.  I think we should rip all that out and do it as
> per attached.

Ah, yeah, I hadn't realized this bogosity.  Haven't verified the patch in
detail.

> (I also took the trouble to make the error messages conform
> to project style.)

Not sure about this part.  Many psql error messages are full sentences (start
with uppercase, end in period); others start with the \ command being
complained about.  Compare

alvherre=# \foobar
Invalid command \foobar. Try \? for help.
alvherre=# \copy foobar
\copy: parse error at end of line

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: \crosstabview fixes

От
Tom Lane
Дата:
"Daniel Verite" <daniel@manitou-mail.org> writes:
> To avoid the confusion between "2:4" and "2":"4" or 2:4,
> and the ambiguity with a possibly existing "2:4" column,
> maybe we should abandon this syntax and require the optional
> scolH to be on its own at the end of the command.

That would be OK with me; it's certainly less of a hack than what's
there now.  (I went back and forth about how much effort to put into
dealing with the colon syntax; I think the version I have in my patch
would be all right, but it's not perfect.)

> The simplified invocation of the command would be
>   \crosstabview colV colH colD [scolH]
> (without any default, just scolH being optional):

> Or if it's preferrable to have colH just near scolH:
>   \crosstabview colD colV colH [scolH]

Uh, why not

\crosstabview [ colV colH [ colD [ scolH ]]]

I see no particular reason that the existing abbreviation styles aren't
good.  In any case, forcing colD to be specified is kind of annoying ...
        regards, tom lane



Re: \crosstabview fixes

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Tom Lane wrote:
>> (I also took the trouble to make the error messages conform
>> to project style.)

> Not sure about this part.  Many psql error messages are full sentences (start
> with uppercase, end in period); others start with the \ command being
> complained about.  Compare

> alvherre=# \foobar
> Invalid command \foobar. Try \? for help.
> alvherre=# \copy foobar
> \copy: parse error at end of line

Well, the first of those is meant to be two full sentences, though they
are a bit abbreviated.

I think putting "\crosstabview: " in front of all the error messages
in this patch would be a fine idea, though.  Will do that unless there
are further objections.
        regards, tom lane



Re: \crosstabview fixes

От
Tom Lane
Дата:
I wrote:
> "Daniel Verite" <daniel@manitou-mail.org> writes:
>> To avoid the confusion between "2:4" and "2":"4" or 2:4,
>> and the ambiguity with a possibly existing "2:4" column,
>> maybe we should abandon this syntax and require the optional
>> scolH to be on its own at the end of the command.

> That would be OK with me; it's certainly less of a hack than what's
> there now.  (I went back and forth about how much effort to put into
> dealing with the colon syntax; I think the version I have in my patch
> would be all right, but it's not perfect.)

Here's a patch along those lines.  Any objections?

            regards, tom lane

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b2b2adc..9eeb1ca 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 993,1001 ****
        <varlistentry id="APP-PSQL-meta-commands-crosstabview">
          <term><literal>\crosstabview [
              <replaceable class="parameter">colV</replaceable>
!             <replaceable class="parameter">colH</replaceable>[:<replaceable class="parameter">scolH</replaceable>]
!             [<replaceable class="parameter">colD</replaceable>]
!             ] </literal></term>
          <listitem>
          <para>
          Executes the current query buffer (like <literal>\g</literal>) and
--- 993,1002 ----
        <varlistentry id="APP-PSQL-meta-commands-crosstabview">
          <term><literal>\crosstabview [
              <replaceable class="parameter">colV</replaceable>
!             [ <replaceable class="parameter">colH</replaceable>
!             [ <replaceable class="parameter">colD</replaceable>
!             [ <replaceable class="parameter">scolH</replaceable>
!             ] ] ] ] </literal></term>
          <listitem>
          <para>
          Executes the current query buffer (like <literal>\g</literal>) and
*************** testdb=>
*** 1004,1019 ****
          The output column identified by <replaceable class="parameter">colV</>
          becomes a vertical header and the output column identified by
          <replaceable class="parameter">colH</replaceable>
!         becomes a horizontal header, optionally sorted by ranking data obtained
!         from column <replaceable class="parameter">scolH</replaceable>.
          <replaceable class="parameter">colD</replaceable> identifies
          the output column to display within the grid.
!         If <replaceable class="parameter">colD</replaceable> is not
!         specified and there are exactly three columns in the result set,
!         the column that is neither
!         <replaceable class="parameter">colV</replaceable> nor
!         <replaceable class="parameter">colH</replaceable>
!         is displayed; if there are more columns, an error is reported.
          </para>

          <para>
--- 1005,1015 ----
          The output column identified by <replaceable class="parameter">colV</>
          becomes a vertical header and the output column identified by
          <replaceable class="parameter">colH</replaceable>
!         becomes a horizontal header.
          <replaceable class="parameter">colD</replaceable> identifies
          the output column to display within the grid.
!         <replaceable class="parameter">scolH</replaceable> identifies
!         an optional sort column for the horizontal header.
          </para>

          <para>
*************** testdb=>
*** 1024,1029 ****
--- 1020,1031 ----
          and <replaceable class="parameter">colH</replaceable> as column 2.
          <replaceable class="parameter">colH</replaceable> must differ from
          <replaceable class="parameter">colV</replaceable>.
+         If <replaceable class="parameter">colD</replaceable> is not
+         specified and there are exactly three columns in the result set,
+         the column that is neither
+         <replaceable class="parameter">colV</replaceable> nor
+         <replaceable class="parameter">colH</replaceable>
+         is displayed; if there are more columns, an error is reported.
          </para>

          <para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 227d180..e1f5805 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 368,380 ****
      /* \crosstabview -- execute a query and display results in crosstab */
      else if (strcmp(cmd, "crosstabview") == 0)
      {
!         pset.ctv_col_V = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);
!         pset.ctv_col_H = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);
!         pset.ctv_col_D = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);

          pset.crosstab_flag = true;
          status = PSQL_CMD_SEND;
      }
--- 368,378 ----
      /* \crosstabview -- execute a query and display results in crosstab */
      else if (strcmp(cmd, "crosstabview") == 0)
      {
!         int            i;

+         for (i = 0; i < lengthof(pset.ctv_args); i++)
+             pset.ctv_args[i] = psql_scan_slash_option(scan_state,
+                                                       OT_SQLID, NULL, true);
          pset.crosstab_flag = true;
          status = PSQL_CMD_SEND;
      }
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 437cb56..2c0d781 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** SendQuery(const char *query)
*** 1130,1135 ****
--- 1130,1136 ----
      PGTransactionStatusType transaction_status;
      double        elapsed_msec = 0;
      bool        OK = false;
+     int            i;
      bool        on_error_rollback_savepoint = false;
      static bool on_error_rollback_warning = false;

*************** sendquery_cleanup:
*** 1362,1381 ****

      /* reset \crosstabview trigger */
      pset.crosstab_flag = false;
!     if (pset.ctv_col_V)
!     {
!         free(pset.ctv_col_V);
!         pset.ctv_col_V = NULL;
!     }
!     if (pset.ctv_col_H)
!     {
!         free(pset.ctv_col_H);
!         pset.ctv_col_H = NULL;
!     }
!     if (pset.ctv_col_D)
      {
!         free(pset.ctv_col_D);
!         pset.ctv_col_D = NULL;
      }

      return OK;
--- 1363,1372 ----

      /* reset \crosstabview trigger */
      pset.crosstab_flag = false;
!     for (i = 0; i < lengthof(pset.ctv_args); i++)
      {
!         pg_free(pset.ctv_args[i]);
!         pset.ctv_args[i] = NULL;
      }

      return OK;
diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index 3cc15ed..56cae7a 100644
*** a/src/bin/psql/crosstabview.c
--- b/src/bin/psql/crosstabview.c
*************** static bool printCrosstab(const PGresult
*** 82,90 ****
                int num_columns, pivot_field *piv_columns, int field_for_columns,
                int num_rows, pivot_field *piv_rows, int field_for_rows,
                int field_for_data);
- static int parseColumnRefs(const char *arg, const PGresult *res,
-                 int **col_numbers,
-                 int max_columns, char separator);
  static void avlInit(avl_tree *tree);
  static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
  static int avlCollectFields(avl_tree *tree, avl_node *node,
--- 82,87 ----
*************** static int    rankCompare(const void *a, co
*** 99,231 ****
  /*
   * Main entry point to this module.
   *
!  * Process the data from *res according the display options in pset (global),
   * to generate the horizontal and vertical headers contents,
   * then call printCrosstab() for the actual output.
   */
  bool
  PrintResultsInCrosstab(const PGresult *res)
  {
!     char       *opt_field_for_rows = pset.ctv_col_V;
!     char       *opt_field_for_columns = pset.ctv_col_H;
!     char       *opt_field_for_data = pset.ctv_col_D;
!     int            rn;
      avl_tree    piv_columns;
      avl_tree    piv_rows;
      pivot_field *array_columns = NULL;
      pivot_field *array_rows = NULL;
      int            num_columns = 0;
      int            num_rows = 0;
-     int           *colsV = NULL,
-                *colsH = NULL,
-                *colsD = NULL;
-     int            n;
-     int            field_for_columns;
-     int            sort_field_for_columns = -1;
      int            field_for_rows;
!     int            field_for_data = -1;
!     bool        retval = false;

      avlInit(&piv_rows);
      avlInit(&piv_columns);

-     if (res == NULL)
-     {
-         psql_error(_("No result\n"));
-         goto error_return;
-     }
-
      if (PQresultStatus(res) != PGRES_TUPLES_OK)
      {
!         psql_error(_("The query must return results to be shown in crosstab\n"));
!         goto error_return;
!     }
!
!     if (opt_field_for_rows && !opt_field_for_columns)
!     {
!         psql_error(_("A second column must be specified for the horizontal header\n"));
          goto error_return;
      }

!     if (PQnfields(res) <= 2)
      {
!         psql_error(_("The query must return at least two columns to be shown in crosstab\n"));
          goto error_return;
      }

!     /*
!      * Arguments processing for the vertical header (1st arg) displayed in the
!      * left-most column. Only a reference to a field is accepted (no sort
!      * column).
!      */
!
!     if (opt_field_for_rows == NULL)
!     {
          field_for_rows = 0;
-     }
      else
      {
!         n = parseColumnRefs(opt_field_for_rows, res, &colsV, 1, ':');
!         if (n != 1)
              goto error_return;
-         field_for_rows = colsV[0];
      }

!     if (field_for_rows < 0)
!         goto error_return;
!
!     /*----------
!      * Arguments processing for the horizontal header (2nd arg)
!      * (pivoted column that gets displayed as the first row).
!      * Determine:
!      * - the field number for the horizontal header column
!      * - the field number of the associated sort column, if any
!      */
!
!     if (opt_field_for_columns == NULL)
          field_for_columns = 1;
      else
      {
!         n = parseColumnRefs(opt_field_for_columns, res, &colsH, 2, ':');
!         if (n <= 0)
!             goto error_return;
!         if (n == 1)
!             field_for_columns = colsH[0];
!         else
!         {
!             field_for_columns = colsH[0];
!             sort_field_for_columns = colsH[1];
!         }
!
          if (field_for_columns < 0)
              goto error_return;
      }

      if (field_for_columns == field_for_rows)
      {
!         psql_error(_("The same column cannot be used for both vertical and horizontal headers\n"));
          goto error_return;
      }

!     /*
!      * Arguments processing for the data columns (3rd arg).  Determine the
!      * column to display in the grid.
!      */
!     if (opt_field_for_data == NULL)
      {
!         int        i;

          /*
           * If the data column was not specified, we search for the one not
!          * used as either vertical or horizontal headers.  If the result has
!          * more than three columns, raise an error.
           */
!         if (PQnfields(res) > 3)
          {
!             psql_error(_("Data column must be specified when the result set has more than three columns\n"));
              goto error_return;
          }

          for (i = 0; i < PQnfields(res); i++)
          {
              if (i != field_for_rows && i != field_for_columns)
--- 96,180 ----
  /*
   * Main entry point to this module.
   *
!  * Process the data from *res according to the options in pset (global),
   * to generate the horizontal and vertical headers contents,
   * then call printCrosstab() for the actual output.
   */
  bool
  PrintResultsInCrosstab(const PGresult *res)
  {
!     bool        retval = false;
      avl_tree    piv_columns;
      avl_tree    piv_rows;
      pivot_field *array_columns = NULL;
      pivot_field *array_rows = NULL;
      int            num_columns = 0;
      int            num_rows = 0;
      int            field_for_rows;
!     int            field_for_columns;
!     int            field_for_data;
!     int            sort_field_for_columns;
!     int            rn;

      avlInit(&piv_rows);
      avlInit(&piv_columns);

      if (PQresultStatus(res) != PGRES_TUPLES_OK)
      {
!         psql_error(_("\\crosstabview: query must return results to be shown in crosstab\n"));
          goto error_return;
      }

!     if (PQnfields(res) < 3)
      {
!         psql_error(_("\\crosstabview: query must return at least three columns\n"));
          goto error_return;
      }

!     /* Process first optional arg (vertical header column) */
!     if (pset.ctv_args[0] == NULL)
          field_for_rows = 0;
      else
      {
!         field_for_rows = indexOfColumn(pset.ctv_args[0], res);
!         if (field_for_rows < 0)
              goto error_return;
      }

!     /* Process second optional arg (horizontal header column) */
!     if (pset.ctv_args[1] == NULL)
          field_for_columns = 1;
      else
      {
!         field_for_columns = indexOfColumn(pset.ctv_args[1], res);
          if (field_for_columns < 0)
              goto error_return;
      }

+     /* Insist that header columns be distinct */
      if (field_for_columns == field_for_rows)
      {
!         psql_error(_("\\crosstabview: vertical and horizontal headers must be different columns\n"));
          goto error_return;
      }

!     /* Process third optional arg (data column) */
!     if (pset.ctv_args[2] == NULL)
      {
!         int            i;

          /*
           * If the data column was not specified, we search for the one not
!          * used as either vertical or horizontal headers.  Must be exactly
!          * three columns, or this won't be unique.
           */
!         if (PQnfields(res) != 3)
          {
!             psql_error(_("\\crosstabview: data column must be specified when query returns more than three
columns\n"));
              goto error_return;
          }

+         field_for_data = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
              if (i != field_for_rows && i != field_for_columns)
*************** PrintResultsInCrosstab(const PGresult *r
*** 238,250 ****
      }
      else
      {
!         int        num_fields;

!         /* If a field was given, find out what it is.  Only one is allowed. */
!         num_fields = parseColumnRefs(opt_field_for_data, res, &colsD, 1, ',');
!         if (num_fields < 1)
              goto error_return;
-         field_for_data = colsD[0];
      }

      /*
--- 187,205 ----
      }
      else
      {
!         field_for_data = indexOfColumn(pset.ctv_args[2], res);
!         if (field_for_data < 0)
!             goto error_return;
!     }

!     /* Process fourth optional arg (horizontal header sort column) */
!     if (pset.ctv_args[3] == NULL)
!         sort_field_for_columns = -1;    /* no sort column */
!     else
!     {
!         sort_field_for_columns = indexOfColumn(pset.ctv_args[3], res);
!         if (sort_field_for_columns < 0)
              goto error_return;
      }

      /*
*************** PrintResultsInCrosstab(const PGresult *r
*** 271,277 ****

          if (piv_columns.count > CROSSTABVIEW_MAX_COLUMNS)
          {
!             psql_error(_("Maximum number of columns (%d) exceeded\n"),
                         CROSSTABVIEW_MAX_COLUMNS);
              goto error_return;
          }
--- 226,232 ----

          if (piv_columns.count > CROSSTABVIEW_MAX_COLUMNS)
          {
!             psql_error(_("\\crosstabview: maximum number of columns (%d) exceeded\n"),
                         CROSSTABVIEW_MAX_COLUMNS);
              goto error_return;
          }
*************** error_return:
*** 319,327 ****
      avlFree(&piv_rows, piv_rows.root);
      pg_free(array_columns);
      pg_free(array_rows);
-     pg_free(colsV);
-     pg_free(colsH);
-     pg_free(colsD);

      return retval;
  }
--- 274,279 ----
*************** printCrosstab(const PGresult *results,
*** 442,448 ****
               */
              if (cont.cells[idx] != NULL)
              {
!                 psql_error(_("data cell already contains a value: (row: \"%s\", column: \"%s\")\n"),
                               piv_rows[row_number].name ? piv_rows[row_number].name :
                               popt.nullPrint ? popt.nullPrint : "(null)",
                               piv_columns[col_number].name ? piv_columns[col_number].name :
--- 394,400 ----
               */
              if (cont.cells[idx] != NULL)
              {
!                 psql_error(_("\\crosstabview: query result contains multiple data values for row \"%s\", column
\"%s\"\n"),
                               piv_rows[row_number].name ? piv_rows[row_number].name :
                               popt.nullPrint ? popt.nullPrint : "(null)",
                               piv_columns[col_number].name ? piv_columns[col_number].name :
*************** error:
*** 476,583 ****
  }

  /*
-  * Parse "arg", which is a string of column IDs separated by "separator".
-  *
-  * Each column ID can be:
-  * - a number from 1 to PQnfields(res)
-  * - an unquoted column name matching (case insensitively) one of PQfname(res,...)
-  * - a quoted column name matching (case sensitively) one of PQfname(res,...)
-  *
-  * If max_columns > 0, it is the max number of column IDs allowed.
-  *
-  * On success, return number of column IDs found (possibly 0), and return a
-  * malloc'd array of the matching column numbers of "res" into *col_numbers.
-  *
-  * On failure, return -1 and set *col_numbers to NULL.
-  */
- static int
- parseColumnRefs(const char *arg,
-                 const PGresult *res,
-                 int **col_numbers,
-                 int max_columns,
-                 char separator)
- {
-     const char *p = arg;
-     char        c;
-     int            num_cols = 0;
-
-     *col_numbers = NULL;
-     while ((c = *p) != '\0')
-     {
-         const char *field_start = p;
-         bool        quoted_field = false;
-
-         /* first char */
-         if (c == '"')
-         {
-             quoted_field = true;
-             p++;
-         }
-
-         while ((c = *p) != '\0')
-         {
-             if (c == separator && !quoted_field)
-                 break;
-             if (c == '"')        /* end of field or embedded double quote */
-             {
-                 p++;
-                 if (*p == '"')
-                 {
-                     if (quoted_field)
-                     {
-                         p++;
-                         continue;
-                     }
-                 }
-                 else if (quoted_field && *p == separator)
-                     break;
-             }
-             if (*p)
-                 p += PQmblen(p, pset.encoding);
-         }
-
-         if (p != field_start)
-         {
-             char   *col_name;
-             int        col_num;
-
-             /* enforce max_columns limit */
-             if (max_columns > 0 && num_cols == max_columns)
-             {
-                 psql_error(_("No more than %d column references expected\n"),
-                            max_columns);
-                 goto errfail;
-             }
-             /* look up the column and add its index into *col_numbers */
-             col_name = pg_malloc(p - field_start + 1);
-             memcpy(col_name, field_start, p - field_start);
-             col_name[p - field_start] = '\0';
-             col_num = indexOfColumn(col_name, res);
-             pg_free(col_name);
-             if (col_num < 0)
-                 goto errfail;
-             *col_numbers = (int *) pg_realloc(*col_numbers,
-                                               (num_cols + 1) * sizeof(int));
-             (*col_numbers)[num_cols++] = col_num;
-         }
-         else
-         {
-             psql_error(_("Empty column reference\n"));
-             goto errfail;
-         }
-
-         if (*p)
-             p += PQmblen(p, pset.encoding);
-     }
-     return num_cols;
-
- errfail:
-     pg_free(*col_numbers);
-     *col_numbers = NULL;
-     return -1;
- }
-
- /*
   * The avl* functions below provide a minimalistic implementation of AVL binary
   * trees, to efficiently collect the distinct values that will form the horizontal
   * and vertical headers. It only supports adding new values, no removal or even
--- 428,433 ----
*************** rankSort(int num_columns, pivot_field *p
*** 773,833 ****
  }

  /*
!  * Compare a user-supplied argument against a field name obtained by PQfname(),
!  * which is already case-folded.
!  * If arg is not enclosed in double quotes, pg_strcasecmp applies, otherwise
!  * do a case-sensitive comparison with these rules:
!  * - double quotes enclosing 'arg' are filtered out
!  * - double quotes inside 'arg' are expected to be doubled
!  */
! static bool
! fieldNameEquals(const char *arg, const char *fieldname)
! {
!     const char *p = arg;
!     const char *f = fieldname;
!     char        c;
!
!     if (*p++ != '"')
!         return (pg_strcasecmp(arg, fieldname) == 0);
!
!     while ((c = *p++))
!     {
!         if (c == '"')
!         {
!             if (*p == '"')
!                 p++;            /* skip second quote and continue */
!             else if (*p == '\0')
!                 return (*f == '\0');    /* p is shorter than f, or is
!                                          * identical */
!         }
!         if (*f == '\0')
!             return false;        /* f is shorter than p */
!         if (c != *f)            /* found one byte that differs */
!             return false;
!         f++;
!     }
!     return (*f == '\0');
! }
!
! /*
!  * arg can be a number or a column name, possibly quoted (like in an ORDER BY clause)
!  * Returns:
!  *    on success, the 0-based index of the column
!  *    or -1 if the column number or name is not found in the result's structure,
!  *          or if it's ambiguous (arg corresponding to several columns)
   */
  static int
  indexOfColumn(const char *arg, const PGresult *res)
  {
      int            idx;

!     if (strspn(arg, "0123456789") == strlen(arg))
      {
          /* if arg contains only digits, it's a column number */
          idx = atoi(arg) - 1;
          if (idx < 0 || idx >= PQnfields(res))
          {
!             psql_error(_("Invalid column number: %s\n"), arg);
              return -1;
          }
      }
--- 623,646 ----
  }

  /*
!  * Look up a column reference, which can be either:
!  * - a number from 1 to PQnfields(res)
!  * - a column name matching one of PQfname(res,...)
!  *
!  * Returns zero-based column number, or -1 if not found or ambiguous.
   */
  static int
  indexOfColumn(const char *arg, const PGresult *res)
  {
      int            idx;

!     if (arg[0] && strspn(arg, "0123456789") == strlen(arg))
      {
          /* if arg contains only digits, it's a column number */
          idx = atoi(arg) - 1;
          if (idx < 0 || idx >= PQnfields(res))
          {
!             psql_error(_("\\crosstabview: invalid column number: \"%s\"\n"), arg);
              return -1;
          }
      }
*************** indexOfColumn(const char *arg, const PGr
*** 838,849 ****
          idx = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
!             if (fieldNameEquals(arg, PQfname(res, i)))
              {
                  if (idx >= 0)
                  {
!                     /* if another idx was already found for the same name */
!                     psql_error(_("Ambiguous column name: %s\n"), arg);
                      return -1;
                  }
                  idx = i;
--- 651,662 ----
          idx = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
!             if (strcmp(arg, PQfname(res, i)) == 0)
              {
                  if (idx >= 0)
                  {
!                     /* another idx was already found for the same name */
!                     psql_error(_("\\crosstabview: ambiguous column name: \"%s\"\n"), arg);
                      return -1;
                  }
                  idx = i;
*************** indexOfColumn(const char *arg, const PGr
*** 851,857 ****
          }
          if (idx == -1)
          {
!             psql_error(_("Invalid column name: %s\n"), arg);
              return -1;
          }
      }
--- 664,670 ----
          }
          if (idx == -1)
          {
!             psql_error(_("\\crosstabview: column name not found: \"%s\"\n"), arg);
              return -1;
          }
      }
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 643ff8c..8cfe9d2 100644
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*************** typedef struct _psqlSettings
*** 94,102 ****
      char       *gset_prefix;    /* one-shot prefix argument for \gset */
      bool        gexec_flag;        /* one-shot flag to execute query's results */
      bool        crosstab_flag;    /* one-shot request to crosstab results */
!     char       *ctv_col_V;        /* \crosstabview 1st argument */
!     char       *ctv_col_H;        /* \crosstabview 2nd argument */
!     char       *ctv_col_D;        /* \crosstabview 3nd argument */

      bool        notty;            /* stdin or stdout is not a tty (as determined
                                   * on startup) */
--- 94,100 ----
      char       *gset_prefix;    /* one-shot prefix argument for \gset */
      bool        gexec_flag;        /* one-shot flag to execute query's results */
      bool        crosstab_flag;    /* one-shot request to crosstab results */
!     char       *ctv_args[4];    /* \crosstabview arguments */

      bool        notty;            /* stdin or stdout is not a tty (as determined
                                   * on startup) */
diff --git a/src/test/regress/expected/psql_crosstab.out b/src/test/regress/expected/psql_crosstab.out
index c87c2fc..c508f87 100644
*** a/src/test/regress/expected/psql_crosstab.out
--- b/src/test/regress/expected/psql_crosstab.out
*************** SELECT v, EXTRACT(year FROM d), count(*)
*** 35,41 ****
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name":num 4
   v  | Jan | Apr | Jul | Dec
  ----+-----+-----+-----+-----
   v0 |     |     |   2 |   1
--- 35,41 ----
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name" 4 num
   v  | Jan | Apr | Jul | Dec
  ----+-----+-----+-----+-----
   v0 |     |     |   2 |   1
*************** SELECT EXTRACT(year FROM d) AS year, to_
*** 50,56 ****
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year:year format
   month name |      2014       |      2015
  ------------+-----------------+----------------
   Jan        |                 | sum=3 avg=3.0
--- 50,56 ----
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year format year
   month name |      2014       |      2015
  ------------+-----------------+----------------
   Jan        |                 | sum=3 avg=3.0
*************** SELECT v, h, string_agg(c, E'\n') FROM c
*** 74,80 ****
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
--- 74,80 ----
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 87,93 ****
  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c
   v  |     | h4  |  h2  | h1  | h0
  ----+-----+-----+------+-----+-----
   v0 | qux | qux+|      |     |
--- 87,93 ----
  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r
   v  |     | h4  |  h2  | h1  | h0
  ----+-----+-----+------+-----+-----
   v0 | qux | qux+|      |     |
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 100,106 ****
  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
--- 100,106 ----
  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 112,118 ****

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
! The query must return at least two columns to be shown in crosstab
  -- only null, no column name, 3 columns: works
  SELECT null,null,null \crosstabview
   ?column? |
--- 112,118 ----

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
! \crosstabview: query must return at least three columns
  -- only null, no column name, 3 columns: works
  SELECT null,null,null \crosstabview
   ?column? |
*************** FROM ctv_data GROUP BY v, h ORDER BY h,v
*** 166,185 ****
  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j
! Invalid column name: j
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5
! Invalid column number: 5
  -- error: same H and V columns
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 h 4
! The same column cannot be used for both vertical and horizontal headers
  -- error: too many columns
  SELECT a,a,1 FROM generate_series(1,3000) AS a
   \crosstabview
! Maximum number of columns (1600) exceeded
  -- error: only one column
  SELECT 1 \crosstabview
! The query must return at least two columns to be shown in crosstab
  DROP TABLE ctv_data;
--- 166,185 ----
  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j
! \crosstabview: column name not found: "j"
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5
! \crosstabview: invalid column number: "5"
  -- error: same H and V columns
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 h 4
! \crosstabview: vertical and horizontal headers must be different columns
  -- error: too many columns
  SELECT a,a,1 FROM generate_series(1,3000) AS a
   \crosstabview
! \crosstabview: maximum number of columns (1600) exceeded
  -- error: only one column
  SELECT 1 \crosstabview
! \crosstabview: query must return at least three columns
  DROP TABLE ctv_data;
diff --git a/src/test/regress/sql/psql_crosstab.sql b/src/test/regress/sql/psql_crosstab.sql
index e602676..d47555f 100644
*** a/src/test/regress/sql/psql_crosstab.sql
--- b/src/test/regress/sql/psql_crosstab.sql
*************** SELECT v, EXTRACT(year FROM d), count(*)
*** 23,29 ****
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name":num 4

  -- ordered months in vertical header, ordered years in horizontal header
  SELECT EXTRACT(year FROM d) AS year, to_char(d,'Mon') AS "month name",
--- 23,29 ----
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name" 4 num

  -- ordered months in vertical header, ordered years in horizontal header
  SELECT EXTRACT(year FROM d) AS year, to_char(d,'Mon') AS "month name",
*************** SELECT EXTRACT(year FROM d) AS year, to_
*** 32,38 ****
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year:year format

  -- combine contents vertically into the same cell (V/H duplicates)
  SELECT v, h, string_agg(c, E'\n') FROM ctv_data GROUP BY v, h ORDER BY 1,2,3
--- 32,38 ----
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year format year

  -- combine contents vertically into the same cell (V/H duplicates)
  SELECT v, h, string_agg(c, E'\n') FROM ctv_data GROUP BY v, h ORDER BY 1,2,3
*************** SELECT v, h, string_agg(c, E'\n') FROM c
*** 41,57 ****
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c

  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c

  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
--- 41,57 ----
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r

  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r

  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview

Re: \crosstabview fixes

От
Christoph Berg
Дата:
Re: Tom Lane 2016-04-14 <15673.1460592362@sss.pgh.pa.us>
> Here's a patch along those lines.  Any objections?

>           <term><literal>\crosstabview [
>               <replaceable class="parameter">colV</replaceable>
> !             [ <replaceable class="parameter">colH</replaceable>
> !             [ <replaceable class="parameter">colD</replaceable>
> !             [ <replaceable class="parameter">scolH</replaceable>
> !             ] ] ] ] </literal></term>

Maybe use "sortcolH" to make it immediately clear what it does?

When I first read the docs it seemed to me that this scolH thing would
introduce a lot of magic until I finished the text, sortcolH would
have made me more comfortable with it.

Christoph



Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Tom Lane wrote:

> > That would be OK with me; it's certainly less of a hack than what's
> > there now.  (I went back and forth about how much effort to put into
> > dealing with the colon syntax; I think the version I have in my patch
> > would be all right, but it's not perfect.)
>
> Here's a patch along those lines.  Any objections?

There's the issue that it can no longer distinguish between numbers as
column positions and column names that consist entirely of numbers.

For example, before the patch:
=# SELECT 'a' as "4", 'b' as "5", 'x'  \crosstabview "4" "5"4 | b
---+---a | x

After the patch:
=# SELECT 'a' as "4", 'b' as "5", 'x' \crosstabview "4" "5"
\crosstabview: invalid column number: "4"

crosstabview's parseColumnRefs() knows that "4" is a column
name because of the quotes, but once it's replaced by the psql
ident parser, I guess the quotes are removed and the argument
becomes a bare number.

I don't quite see how to work around that, short of simply
removing the possibility of addressing columns by their
numbers. Which maybe is a bit sad for the end user, I'm not
sure, but ISTM that's a logical consequence of abandoning
the dedicated parser for columns.


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



Re: \crosstabview fixes

От
Christoph Berg
Дата:
Re: Daniel Verite 2016-04-14 <bc3a2e99-5030-4466-a248-98c5e9841205@mm>
> I don't quite see how to work around that, short of simply
> removing the possibility of addressing columns by their
> numbers. Which maybe is a bit sad for the end user, I'm not
> sure, but ISTM that's a logical consequence of abandoning
> the dedicated parser for columns.

That would be bad news, given that \crosstabview is meant for
interactive use where these number shortcuts are much more likely to
be used than in proper production SQL code. Be it only for ease of
typing, or for the case where the columns are just called ?column?.

If there's no way out, what about changing it the other way, i.e.
breaking the case where the column is named by a number? That seems
much less of a problem in practice.

Christoph



Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Christoph Berg wrote:

> > I don't quite see how to work around that, short of simply
> > removing the possibility of addressing columns by their
> > numbers. [...]

> That would be bad news, given that \crosstabview is meant for
> interactive use where these number shortcuts are much more likely to
> be used than in proper production SQL code. Be it only for ease of
> typing, or for the case where the columns are just called ?column?.

Ah, that reminds me that there's another corner case.
In a resultset, two columns can share the exact same name.
I think the only way to distinguish them in that case is
to refer to them by position, so that's another argument
to not discard that possibility.

=# select 'X' as "a", 'Y' as "a", 'Z' \crosstabview a a
Ambiguous column name: a

versus

=# select 'X' as "a", 'Y' as "a", 'x' \crosstabview 1 2a | Y
---+---X | x


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



Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Christoph Berg wrote:

> If there's no way out, what about changing it the other way, i.e.
> breaking the case where the column is named by a number? That seems
> much less of a problem in practice.

I don't think it would be acceptable.
But there's still the option of keeping the dedicated parser.

crosstabview doc says:
"The usual SQL case folding and quoting rules apply to column names"

Tom objected to that, upthread:
> I noticed that the \crosstabview documentation asserts that column
> name arguments are handled per standard SQL semantics.  In point of
> fact, though, the patch expends a couple hundred lines to implement
> what is NOT standard SQL semantics: matching unquoted names
> case-insensitively is anything but that.

Indeed it differs, but that's not necessarily bad. If there's a FOO
column and it's refered to as \crosstabview foo... it will complain about
an ambiguity only if there's another column that's named foo, or Foo, or
any case variant. Quoting becomes mandatory only in that case,
or of course if the column referred to contains spaces or double
quotes.

For example, both these invocations work:
current=# SELECT 'X' as "FOO", 'Y', 'z' \crosstabview foo 2FOO | Y
-----+---X   | z

current=# SELECT 'X' as "FOO", 'Y', 'z' \crosstabview FOO 2FOO | Y
-----+---X   | z

OTOH a detected ambiguity would be:
current=# SELECT 'X' as "FOO", 'Y' as "foo", 'z' \crosstabview FOO 2
Ambiguous column name: FOO

which is solved by quoting the argument:
current=# SELECT 'X' as "FOO", 'Y' as "foo", 'z' \crosstabview "FOO" 2FOO | Y
-----+---X   | z


Whereas using the generic column parser with Tom's patch, the only
accepted invocation out of the 4 examples above is the last one:

new=# SELECT 'X' as "FOO", 'Y', 'z' \crosstabview foo 2
\crosstabview: column name not found: "foo"

new # SELECT 'X' as "FOO", 'Y', 'z' \crosstabview FOO 2
\crosstabview: column name not found: "foo"

new=# SELECT 'X' as "FOO", 'Y' as "foo", 'z' \crosstabview FOO 2
\crosstabview: vertical and horizontal headers must be different columns

new=# SELECT 'X' as "FOO", 'Y' as "foo", 'z' \crosstabview "FOO" 2FOO | Y
-----+---X   | z

Personally I prefer the current behavior, but I can also understand
the appeal from a maintainer's point of view of getting rid of it in
favor of already existing, well-tested generic code.
In which case, what the dedicated parser does is a moot point.

However, because of the aforementioned problem of the interpretation
of column names as numbers, maybe the balance comes back to the
dedicated parser? In which case, there's the question of whether how
it handles case folding as shown above is OK, or whether it should
just downcase unquoted identifiers to strictly match SQL rules.


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



Re: \crosstabview fixes

От
Tom Lane
Дата:
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Christoph Berg wrote:
>> If there's no way out, what about changing it the other way, i.e.
>> breaking the case where the column is named by a number? That seems
>> much less of a problem in practice.

> I don't think it would be acceptable.

I had figured it would be ;-) ... but if you object, let's see what
it would take to preserve that.

My feeling is that what we should do is undo the change to use OT_SQLID,
and in indexOfColumn() perform a downcasing/dequoting conversion that
duplicates what OT_SQLID does in psqlscanslash.l.  That only adds a couple
dozen lines of code, so it will still be way smaller than the existing
logic in crosstabview.c, and it will match the behavior of the rest of
psql.  By postponing that till after the "is it a number" check, we
restore the current behavior that a quoted number won't be taken as a
column number.

(Another approach would be to somehow modify psqlscanslash.l's API so that
we could tell externally whether an argument had been quoted or not.  But
that would be much more invasive, and I doubt it's worth it for this one
use-case.)

I'll have a patch after I finish catching up the rest of my mail.
        regards, tom lane



Re: \crosstabview fixes

От
Tom Lane
Дата:
I wrote:
> My feeling is that what we should do is undo the change to use OT_SQLID,
> and in indexOfColumn() perform a downcasing/dequoting conversion that
> duplicates what OT_SQLID does in psqlscanslash.l.

Here's an updated patch that does it that way, and also adopts Christoph's
documentation suggestion about "sortcolH".  Any further comments?

            regards, tom lane

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b2b2adc..4160665 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=>
*** 993,1001 ****
        <varlistentry id="APP-PSQL-meta-commands-crosstabview">
          <term><literal>\crosstabview [
              <replaceable class="parameter">colV</replaceable>
!             <replaceable class="parameter">colH</replaceable>[:<replaceable class="parameter">scolH</replaceable>]
!             [<replaceable class="parameter">colD</replaceable>]
!             ] </literal></term>
          <listitem>
          <para>
          Executes the current query buffer (like <literal>\g</literal>) and
--- 993,1002 ----
        <varlistentry id="APP-PSQL-meta-commands-crosstabview">
          <term><literal>\crosstabview [
              <replaceable class="parameter">colV</replaceable>
!             [ <replaceable class="parameter">colH</replaceable>
!             [ <replaceable class="parameter">colD</replaceable>
!             [ <replaceable class="parameter">sortcolH</replaceable>
!             ] ] ] ] </literal></term>
          <listitem>
          <para>
          Executes the current query buffer (like <literal>\g</literal>) and
*************** testdb=>
*** 1004,1019 ****
          The output column identified by <replaceable class="parameter">colV</>
          becomes a vertical header and the output column identified by
          <replaceable class="parameter">colH</replaceable>
!         becomes a horizontal header, optionally sorted by ranking data obtained
!         from column <replaceable class="parameter">scolH</replaceable>.
          <replaceable class="parameter">colD</replaceable> identifies
          the output column to display within the grid.
!         If <replaceable class="parameter">colD</replaceable> is not
!         specified and there are exactly three columns in the result set,
!         the column that is neither
!         <replaceable class="parameter">colV</replaceable> nor
!         <replaceable class="parameter">colH</replaceable>
!         is displayed; if there are more columns, an error is reported.
          </para>

          <para>
--- 1005,1015 ----
          The output column identified by <replaceable class="parameter">colV</>
          becomes a vertical header and the output column identified by
          <replaceable class="parameter">colH</replaceable>
!         becomes a horizontal header.
          <replaceable class="parameter">colD</replaceable> identifies
          the output column to display within the grid.
!         <replaceable class="parameter">sortcolH</replaceable> identifies
!         an optional sort column for the horizontal header.
          </para>

          <para>
*************** testdb=>
*** 1024,1029 ****
--- 1020,1031 ----
          and <replaceable class="parameter">colH</replaceable> as column 2.
          <replaceable class="parameter">colH</replaceable> must differ from
          <replaceable class="parameter">colV</replaceable>.
+         If <replaceable class="parameter">colD</replaceable> is not
+         specified, then there must be exactly three columns in the query
+         result, and the column that is neither
+         <replaceable class="parameter">colV</replaceable> nor
+         <replaceable class="parameter">colH</replaceable>
+         is taken to be <replaceable class="parameter">colD</replaceable>.
          </para>

          <para>
*************** testdb=>
*** 1037,1047 ****
          found in column <replaceable class="parameter">colH</replaceable>,
          with duplicates removed.  By default, these appear in the same order
          as in the query results.  But if the
!         optional <replaceable class="parameter">scolH</> argument is given, it
!         identifies a column whose values must be integer numbers, and the
          values from <replaceable class="parameter">colH</replaceable> will
          appear in the horizontal header sorted according to the
!         corresponding <replaceable class="parameter">scolH</> values.
          </para>

          <para>
--- 1039,1049 ----
          found in column <replaceable class="parameter">colH</replaceable>,
          with duplicates removed.  By default, these appear in the same order
          as in the query results.  But if the
!         optional <replaceable class="parameter">sortcolH</> argument is given,
!         it identifies a column whose values must be integer numbers, and the
          values from <replaceable class="parameter">colH</replaceable> will
          appear in the horizontal header sorted according to the
!         corresponding <replaceable class="parameter">sortcolH</> values.
          </para>

          <para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 227d180..4fa7760 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 368,380 ****
      /* \crosstabview -- execute a query and display results in crosstab */
      else if (strcmp(cmd, "crosstabview") == 0)
      {
!         pset.ctv_col_V = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);
!         pset.ctv_col_H = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);
!         pset.ctv_col_D = psql_scan_slash_option(scan_state,
!                                                 OT_NORMAL, NULL, false);

          pset.crosstab_flag = true;
          status = PSQL_CMD_SEND;
      }
--- 368,378 ----
      /* \crosstabview -- execute a query and display results in crosstab */
      else if (strcmp(cmd, "crosstabview") == 0)
      {
!         int            i;

+         for (i = 0; i < lengthof(pset.ctv_args); i++)
+             pset.ctv_args[i] = psql_scan_slash_option(scan_state,
+                                                       OT_NORMAL, NULL, true);
          pset.crosstab_flag = true;
          status = PSQL_CMD_SEND;
      }
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 437cb56..2c0d781 100644
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*************** SendQuery(const char *query)
*** 1130,1135 ****
--- 1130,1136 ----
      PGTransactionStatusType transaction_status;
      double        elapsed_msec = 0;
      bool        OK = false;
+     int            i;
      bool        on_error_rollback_savepoint = false;
      static bool on_error_rollback_warning = false;

*************** sendquery_cleanup:
*** 1362,1381 ****

      /* reset \crosstabview trigger */
      pset.crosstab_flag = false;
!     if (pset.ctv_col_V)
!     {
!         free(pset.ctv_col_V);
!         pset.ctv_col_V = NULL;
!     }
!     if (pset.ctv_col_H)
!     {
!         free(pset.ctv_col_H);
!         pset.ctv_col_H = NULL;
!     }
!     if (pset.ctv_col_D)
      {
!         free(pset.ctv_col_D);
!         pset.ctv_col_D = NULL;
      }

      return OK;
--- 1363,1372 ----

      /* reset \crosstabview trigger */
      pset.crosstab_flag = false;
!     for (i = 0; i < lengthof(pset.ctv_args); i++)
      {
!         pg_free(pset.ctv_args[i]);
!         pset.ctv_args[i] = NULL;
      }

      return OK;
diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index 3cc15ed..71abaf3 100644
*** a/src/bin/psql/crosstabview.c
--- b/src/bin/psql/crosstabview.c
*************** static bool printCrosstab(const PGresult
*** 82,97 ****
                int num_columns, pivot_field *piv_columns, int field_for_columns,
                int num_rows, pivot_field *piv_rows, int field_for_rows,
                int field_for_data);
- static int parseColumnRefs(const char *arg, const PGresult *res,
-                 int **col_numbers,
-                 int max_columns, char separator);
  static void avlInit(avl_tree *tree);
  static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
  static int avlCollectFields(avl_tree *tree, avl_node *node,
                   pivot_field *fields, int idx);
  static void avlFree(avl_tree *tree, avl_node *node);
  static void rankSort(int num_columns, pivot_field *piv_columns);
! static int    indexOfColumn(const char *arg, const PGresult *res);
  static int    pivotFieldCompare(const void *a, const void *b);
  static int    rankCompare(const void *a, const void *b);

--- 82,94 ----
                int num_columns, pivot_field *piv_columns, int field_for_columns,
                int num_rows, pivot_field *piv_rows, int field_for_rows,
                int field_for_data);
  static void avlInit(avl_tree *tree);
  static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
  static int avlCollectFields(avl_tree *tree, avl_node *node,
                   pivot_field *fields, int idx);
  static void avlFree(avl_tree *tree, avl_node *node);
  static void rankSort(int num_columns, pivot_field *piv_columns);
! static int    indexOfColumn(char *arg, const PGresult *res);
  static int    pivotFieldCompare(const void *a, const void *b);
  static int    rankCompare(const void *a, const void *b);

*************** static int    rankCompare(const void *a, co
*** 99,231 ****
  /*
   * Main entry point to this module.
   *
!  * Process the data from *res according the display options in pset (global),
   * to generate the horizontal and vertical headers contents,
   * then call printCrosstab() for the actual output.
   */
  bool
  PrintResultsInCrosstab(const PGresult *res)
  {
!     char       *opt_field_for_rows = pset.ctv_col_V;
!     char       *opt_field_for_columns = pset.ctv_col_H;
!     char       *opt_field_for_data = pset.ctv_col_D;
!     int            rn;
      avl_tree    piv_columns;
      avl_tree    piv_rows;
      pivot_field *array_columns = NULL;
      pivot_field *array_rows = NULL;
      int            num_columns = 0;
      int            num_rows = 0;
-     int           *colsV = NULL,
-                *colsH = NULL,
-                *colsD = NULL;
-     int            n;
-     int            field_for_columns;
-     int            sort_field_for_columns = -1;
      int            field_for_rows;
!     int            field_for_data = -1;
!     bool        retval = false;

      avlInit(&piv_rows);
      avlInit(&piv_columns);

-     if (res == NULL)
-     {
-         psql_error(_("No result\n"));
-         goto error_return;
-     }
-
      if (PQresultStatus(res) != PGRES_TUPLES_OK)
      {
!         psql_error(_("The query must return results to be shown in crosstab\n"));
!         goto error_return;
!     }
!
!     if (opt_field_for_rows && !opt_field_for_columns)
!     {
!         psql_error(_("A second column must be specified for the horizontal header\n"));
          goto error_return;
      }

!     if (PQnfields(res) <= 2)
      {
!         psql_error(_("The query must return at least two columns to be shown in crosstab\n"));
          goto error_return;
      }

!     /*
!      * Arguments processing for the vertical header (1st arg) displayed in the
!      * left-most column. Only a reference to a field is accepted (no sort
!      * column).
!      */
!
!     if (opt_field_for_rows == NULL)
!     {
          field_for_rows = 0;
-     }
      else
      {
!         n = parseColumnRefs(opt_field_for_rows, res, &colsV, 1, ':');
!         if (n != 1)
              goto error_return;
-         field_for_rows = colsV[0];
      }

!     if (field_for_rows < 0)
!         goto error_return;
!
!     /*----------
!      * Arguments processing for the horizontal header (2nd arg)
!      * (pivoted column that gets displayed as the first row).
!      * Determine:
!      * - the field number for the horizontal header column
!      * - the field number of the associated sort column, if any
!      */
!
!     if (opt_field_for_columns == NULL)
          field_for_columns = 1;
      else
      {
!         n = parseColumnRefs(opt_field_for_columns, res, &colsH, 2, ':');
!         if (n <= 0)
!             goto error_return;
!         if (n == 1)
!             field_for_columns = colsH[0];
!         else
!         {
!             field_for_columns = colsH[0];
!             sort_field_for_columns = colsH[1];
!         }
!
          if (field_for_columns < 0)
              goto error_return;
      }

      if (field_for_columns == field_for_rows)
      {
!         psql_error(_("The same column cannot be used for both vertical and horizontal headers\n"));
          goto error_return;
      }

!     /*
!      * Arguments processing for the data columns (3rd arg).  Determine the
!      * column to display in the grid.
!      */
!     if (opt_field_for_data == NULL)
      {
!         int        i;

          /*
           * If the data column was not specified, we search for the one not
!          * used as either vertical or horizontal headers.  If the result has
!          * more than three columns, raise an error.
           */
!         if (PQnfields(res) > 3)
          {
!             psql_error(_("Data column must be specified when the result set has more than three columns\n"));
              goto error_return;
          }

          for (i = 0; i < PQnfields(res); i++)
          {
              if (i != field_for_rows && i != field_for_columns)
--- 96,180 ----
  /*
   * Main entry point to this module.
   *
!  * Process the data from *res according to the options in pset (global),
   * to generate the horizontal and vertical headers contents,
   * then call printCrosstab() for the actual output.
   */
  bool
  PrintResultsInCrosstab(const PGresult *res)
  {
!     bool        retval = false;
      avl_tree    piv_columns;
      avl_tree    piv_rows;
      pivot_field *array_columns = NULL;
      pivot_field *array_rows = NULL;
      int            num_columns = 0;
      int            num_rows = 0;
      int            field_for_rows;
!     int            field_for_columns;
!     int            field_for_data;
!     int            sort_field_for_columns;
!     int            rn;

      avlInit(&piv_rows);
      avlInit(&piv_columns);

      if (PQresultStatus(res) != PGRES_TUPLES_OK)
      {
!         psql_error(_("\\crosstabview: query must return results to be shown in crosstab\n"));
          goto error_return;
      }

!     if (PQnfields(res) < 3)
      {
!         psql_error(_("\\crosstabview: query must return at least three columns\n"));
          goto error_return;
      }

!     /* Process first optional arg (vertical header column) */
!     if (pset.ctv_args[0] == NULL)
          field_for_rows = 0;
      else
      {
!         field_for_rows = indexOfColumn(pset.ctv_args[0], res);
!         if (field_for_rows < 0)
              goto error_return;
      }

!     /* Process second optional arg (horizontal header column) */
!     if (pset.ctv_args[1] == NULL)
          field_for_columns = 1;
      else
      {
!         field_for_columns = indexOfColumn(pset.ctv_args[1], res);
          if (field_for_columns < 0)
              goto error_return;
      }

+     /* Insist that header columns be distinct */
      if (field_for_columns == field_for_rows)
      {
!         psql_error(_("\\crosstabview: vertical and horizontal headers must be different columns\n"));
          goto error_return;
      }

!     /* Process third optional arg (data column) */
!     if (pset.ctv_args[2] == NULL)
      {
!         int            i;

          /*
           * If the data column was not specified, we search for the one not
!          * used as either vertical or horizontal headers.  Must be exactly
!          * three columns, or this won't be unique.
           */
!         if (PQnfields(res) != 3)
          {
!             psql_error(_("\\crosstabview: data column must be specified when query returns more than three
columns\n"));
              goto error_return;
          }

+         field_for_data = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
              if (i != field_for_rows && i != field_for_columns)
*************** PrintResultsInCrosstab(const PGresult *r
*** 238,250 ****
      }
      else
      {
!         int        num_fields;

!         /* If a field was given, find out what it is.  Only one is allowed. */
!         num_fields = parseColumnRefs(opt_field_for_data, res, &colsD, 1, ',');
!         if (num_fields < 1)
              goto error_return;
-         field_for_data = colsD[0];
      }

      /*
--- 187,205 ----
      }
      else
      {
!         field_for_data = indexOfColumn(pset.ctv_args[2], res);
!         if (field_for_data < 0)
!             goto error_return;
!     }

!     /* Process fourth optional arg (horizontal header sort column) */
!     if (pset.ctv_args[3] == NULL)
!         sort_field_for_columns = -1;    /* no sort column */
!     else
!     {
!         sort_field_for_columns = indexOfColumn(pset.ctv_args[3], res);
!         if (sort_field_for_columns < 0)
              goto error_return;
      }

      /*
*************** PrintResultsInCrosstab(const PGresult *r
*** 271,277 ****

          if (piv_columns.count > CROSSTABVIEW_MAX_COLUMNS)
          {
!             psql_error(_("Maximum number of columns (%d) exceeded\n"),
                         CROSSTABVIEW_MAX_COLUMNS);
              goto error_return;
          }
--- 226,232 ----

          if (piv_columns.count > CROSSTABVIEW_MAX_COLUMNS)
          {
!             psql_error(_("\\crosstabview: maximum number of columns (%d) exceeded\n"),
                         CROSSTABVIEW_MAX_COLUMNS);
              goto error_return;
          }
*************** error_return:
*** 319,327 ****
      avlFree(&piv_rows, piv_rows.root);
      pg_free(array_columns);
      pg_free(array_rows);
-     pg_free(colsV);
-     pg_free(colsH);
-     pg_free(colsD);

      return retval;
  }
--- 274,279 ----
*************** printCrosstab(const PGresult *results,
*** 442,448 ****
               */
              if (cont.cells[idx] != NULL)
              {
!                 psql_error(_("data cell already contains a value: (row: \"%s\", column: \"%s\")\n"),
                               piv_rows[row_number].name ? piv_rows[row_number].name :
                               popt.nullPrint ? popt.nullPrint : "(null)",
                               piv_columns[col_number].name ? piv_columns[col_number].name :
--- 394,400 ----
               */
              if (cont.cells[idx] != NULL)
              {
!                 psql_error(_("\\crosstabview: query result contains multiple data values for row \"%s\", column
\"%s\"\n"),
                               piv_rows[row_number].name ? piv_rows[row_number].name :
                               popt.nullPrint ? popt.nullPrint : "(null)",
                               piv_columns[col_number].name ? piv_columns[col_number].name :
*************** error:
*** 476,583 ****
  }

  /*
-  * Parse "arg", which is a string of column IDs separated by "separator".
-  *
-  * Each column ID can be:
-  * - a number from 1 to PQnfields(res)
-  * - an unquoted column name matching (case insensitively) one of PQfname(res,...)
-  * - a quoted column name matching (case sensitively) one of PQfname(res,...)
-  *
-  * If max_columns > 0, it is the max number of column IDs allowed.
-  *
-  * On success, return number of column IDs found (possibly 0), and return a
-  * malloc'd array of the matching column numbers of "res" into *col_numbers.
-  *
-  * On failure, return -1 and set *col_numbers to NULL.
-  */
- static int
- parseColumnRefs(const char *arg,
-                 const PGresult *res,
-                 int **col_numbers,
-                 int max_columns,
-                 char separator)
- {
-     const char *p = arg;
-     char        c;
-     int            num_cols = 0;
-
-     *col_numbers = NULL;
-     while ((c = *p) != '\0')
-     {
-         const char *field_start = p;
-         bool        quoted_field = false;
-
-         /* first char */
-         if (c == '"')
-         {
-             quoted_field = true;
-             p++;
-         }
-
-         while ((c = *p) != '\0')
-         {
-             if (c == separator && !quoted_field)
-                 break;
-             if (c == '"')        /* end of field or embedded double quote */
-             {
-                 p++;
-                 if (*p == '"')
-                 {
-                     if (quoted_field)
-                     {
-                         p++;
-                         continue;
-                     }
-                 }
-                 else if (quoted_field && *p == separator)
-                     break;
-             }
-             if (*p)
-                 p += PQmblen(p, pset.encoding);
-         }
-
-         if (p != field_start)
-         {
-             char   *col_name;
-             int        col_num;
-
-             /* enforce max_columns limit */
-             if (max_columns > 0 && num_cols == max_columns)
-             {
-                 psql_error(_("No more than %d column references expected\n"),
-                            max_columns);
-                 goto errfail;
-             }
-             /* look up the column and add its index into *col_numbers */
-             col_name = pg_malloc(p - field_start + 1);
-             memcpy(col_name, field_start, p - field_start);
-             col_name[p - field_start] = '\0';
-             col_num = indexOfColumn(col_name, res);
-             pg_free(col_name);
-             if (col_num < 0)
-                 goto errfail;
-             *col_numbers = (int *) pg_realloc(*col_numbers,
-                                               (num_cols + 1) * sizeof(int));
-             (*col_numbers)[num_cols++] = col_num;
-         }
-         else
-         {
-             psql_error(_("Empty column reference\n"));
-             goto errfail;
-         }
-
-         if (*p)
-             p += PQmblen(p, pset.encoding);
-     }
-     return num_cols;
-
- errfail:
-     pg_free(*col_numbers);
-     *col_numbers = NULL;
-     return -1;
- }
-
- /*
   * The avl* functions below provide a minimalistic implementation of AVL binary
   * trees, to efficiently collect the distinct values that will form the horizontal
   * and vertical headers. It only supports adding new values, no removal or even
--- 428,433 ----
*************** rankSort(int num_columns, pivot_field *p
*** 773,849 ****
  }

  /*
!  * Compare a user-supplied argument against a field name obtained by PQfname(),
!  * which is already case-folded.
!  * If arg is not enclosed in double quotes, pg_strcasecmp applies, otherwise
!  * do a case-sensitive comparison with these rules:
!  * - double quotes enclosing 'arg' are filtered out
!  * - double quotes inside 'arg' are expected to be doubled
!  */
! static bool
! fieldNameEquals(const char *arg, const char *fieldname)
! {
!     const char *p = arg;
!     const char *f = fieldname;
!     char        c;
!
!     if (*p++ != '"')
!         return (pg_strcasecmp(arg, fieldname) == 0);
!
!     while ((c = *p++))
!     {
!         if (c == '"')
!         {
!             if (*p == '"')
!                 p++;            /* skip second quote and continue */
!             else if (*p == '\0')
!                 return (*f == '\0');    /* p is shorter than f, or is
!                                          * identical */
!         }
!         if (*f == '\0')
!             return false;        /* f is shorter than p */
!         if (c != *f)            /* found one byte that differs */
!             return false;
!         f++;
!     }
!     return (*f == '\0');
! }
!
! /*
!  * arg can be a number or a column name, possibly quoted (like in an ORDER BY clause)
!  * Returns:
!  *    on success, the 0-based index of the column
!  *    or -1 if the column number or name is not found in the result's structure,
!  *          or if it's ambiguous (arg corresponding to several columns)
   */
  static int
! indexOfColumn(const char *arg, const PGresult *res)
  {
      int            idx;

!     if (strspn(arg, "0123456789") == strlen(arg))
      {
          /* if arg contains only digits, it's a column number */
          idx = atoi(arg) - 1;
          if (idx < 0 || idx >= PQnfields(res))
          {
!             psql_error(_("Invalid column number: %s\n"), arg);
              return -1;
          }
      }
      else
      {
          int            i;

          idx = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
!             if (fieldNameEquals(arg, PQfname(res, i)))
              {
                  if (idx >= 0)
                  {
!                     /* if another idx was already found for the same name */
!                     psql_error(_("Ambiguous column name: %s\n"), arg);
                      return -1;
                  }
                  idx = i;
--- 623,697 ----
  }

  /*
!  * Look up a column reference, which can be either:
!  * - a number from 1 to PQnfields(res)
!  * - a column name matching one of PQfname(res,...)
!  *
!  * Returns zero-based column number, or -1 if not found or ambiguous.
!  *
!  * Note: may modify contents of "arg" string.
   */
  static int
! indexOfColumn(char *arg, const PGresult *res)
  {
      int            idx;

!     if (arg[0] && strspn(arg, "0123456789") == strlen(arg))
      {
          /* if arg contains only digits, it's a column number */
          idx = atoi(arg) - 1;
          if (idx < 0 || idx >= PQnfields(res))
          {
!             psql_error(_("\\crosstabview: invalid column number: \"%s\"\n"), arg);
              return -1;
          }
      }
      else
      {
+         bool        inquotes = false;
+         char       *cp = arg;
          int            i;

+         /*
+          * Dequote and downcase the column name.  By checking for all-digits
+          * before doing this, we can ensure that a quoted name is treated as a
+          * name even if it's all digits.  This transformation should match
+          * what psqlscanslash.l does in OT_SQLID mode.  (XXX ideally we would
+          * let the lexer do this, but then we couldn't tell if the name was
+          * quoted.)
+          */
+         while (*cp)
+         {
+             if (*cp == '"')
+             {
+                 if (inquotes && cp[1] == '"')
+                 {
+                     /* Keep the first quote, remove the second */
+                     cp++;
+                 }
+                 inquotes = !inquotes;
+                 /* Collapse out quote at *cp */
+                 memmove(cp, cp + 1, strlen(cp));
+                 /* do not advance cp */
+             }
+             else
+             {
+                 if (!inquotes)
+                     *cp = pg_tolower((unsigned char) *cp);
+                 cp += PQmblen(cp, pset.encoding);
+             }
+         }
+
+         /* Now look for match(es) among res' column names */
          idx = -1;
          for (i = 0; i < PQnfields(res); i++)
          {
!             if (strcmp(arg, PQfname(res, i)) == 0)
              {
                  if (idx >= 0)
                  {
!                     /* another idx was already found for the same name */
!                     psql_error(_("\\crosstabview: ambiguous column name: \"%s\"\n"), arg);
                      return -1;
                  }
                  idx = i;
*************** indexOfColumn(const char *arg, const PGr
*** 851,857 ****
          }
          if (idx == -1)
          {
!             psql_error(_("Invalid column name: %s\n"), arg);
              return -1;
          }
      }
--- 699,705 ----
          }
          if (idx == -1)
          {
!             psql_error(_("\\crosstabview: column name not found: \"%s\"\n"), arg);
              return -1;
          }
      }
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 643ff8c..8cfe9d2 100644
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*************** typedef struct _psqlSettings
*** 94,102 ****
      char       *gset_prefix;    /* one-shot prefix argument for \gset */
      bool        gexec_flag;        /* one-shot flag to execute query's results */
      bool        crosstab_flag;    /* one-shot request to crosstab results */
!     char       *ctv_col_V;        /* \crosstabview 1st argument */
!     char       *ctv_col_H;        /* \crosstabview 2nd argument */
!     char       *ctv_col_D;        /* \crosstabview 3nd argument */

      bool        notty;            /* stdin or stdout is not a tty (as determined
                                   * on startup) */
--- 94,100 ----
      char       *gset_prefix;    /* one-shot prefix argument for \gset */
      bool        gexec_flag;        /* one-shot flag to execute query's results */
      bool        crosstab_flag;    /* one-shot request to crosstab results */
!     char       *ctv_args[4];    /* \crosstabview arguments */

      bool        notty;            /* stdin or stdout is not a tty (as determined
                                   * on startup) */
diff --git a/src/test/regress/expected/psql_crosstab.out b/src/test/regress/expected/psql_crosstab.out
index c87c2fc..a9c20a1 100644
*** a/src/test/regress/expected/psql_crosstab.out
--- b/src/test/regress/expected/psql_crosstab.out
*************** SELECT v, EXTRACT(year FROM d), count(*)
*** 35,41 ****
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name":num 4
   v  | Jan | Apr | Jul | Dec
  ----+-----+-----+-----+-----
   v0 |     |     |   2 |   1
--- 35,41 ----
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name" 4 num
   v  | Jan | Apr | Jul | Dec
  ----+-----+-----+-----+-----
   v0 |     |     |   2 |   1
*************** SELECT EXTRACT(year FROM d) AS year, to_
*** 50,56 ****
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year:year format
   month name |      2014       |      2015
  ------------+-----------------+----------------
   Jan        |                 | sum=3 avg=3.0
--- 50,56 ----
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year format year
   month name |      2014       |      2015
  ------------+-----------------+----------------
   Jan        |                 | sum=3 avg=3.0
*************** SELECT v, h, string_agg(c, E'\n') FROM c
*** 74,80 ****
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
--- 74,80 ----
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 87,93 ****
  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c
   v  |     | h4  |  h2  | h1  | h0
  ----+-----+-----+------+-----+-----
   v0 | qux | qux+|      |     |
--- 87,93 ----
  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r
   v  |     | h4  |  h2  | h1  | h0
  ----+-----+-----+------+-----+-----
   v0 | qux | qux+|      |     |
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 100,106 ****
  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
--- 100,106 ----
  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r
   v  | h0  | h1  |  h2  | h4  |
  ----+-----+-----+------+-----+-----
   v0 |     |     |      | qux+| qux
*************** FROM ctv_data GROUP BY v, h ORDER BY 1,3
*** 112,118 ****

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
! The query must return at least two columns to be shown in crosstab
  -- only null, no column name, 3 columns: works
  SELECT null,null,null \crosstabview
   ?column? |
--- 112,118 ----

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
! \crosstabview: query must return at least three columns
  -- only null, no column name, 3 columns: works
  SELECT null,null,null \crosstabview
   ?column? |
*************** FROM ctv_data GROUP BY v, h ORDER BY h,v
*** 163,185 ****
      |     |     |      | dbl |
  (3 rows)

  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j
! Invalid column name: j
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5
! Invalid column number: 5
  -- error: same H and V columns
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 h 4
! The same column cannot be used for both vertical and horizontal headers
  -- error: too many columns
  SELECT a,a,1 FROM generate_series(1,3000) AS a
   \crosstabview
! Maximum number of columns (1600) exceeded
  -- error: only one column
  SELECT 1 \crosstabview
! The query must return at least two columns to be shown in crosstab
  DROP TABLE ctv_data;
--- 163,201 ----
      |     |     |      | dbl |
  (3 rows)

+ -- refer to columns by quoted names, check downcasing of unquoted name
+ SELECT 1 as "22", 2 as b, 3 as "Foo"
+  \crosstabview "22" B "Foo"
+  22 | 2
+ ----+---
+   1 | 3
+ (1 row)
+
  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j
! \crosstabview: column name not found: "j"
! -- error: need to quote name
! SELECT 1 as "22", 2 as b, 3 as "Foo"
!  \crosstabview 1 2 Foo
! \crosstabview: column name not found: "foo"
! -- error: need to not quote name
! SELECT 1 as "22", 2 as b, 3 as "Foo"
!  \crosstabview 1 "B" "Foo"
! \crosstabview: column name not found: "B"
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5
! \crosstabview: invalid column number: "5"
  -- error: same H and V columns
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 h 4
! \crosstabview: vertical and horizontal headers must be different columns
  -- error: too many columns
  SELECT a,a,1 FROM generate_series(1,3000) AS a
   \crosstabview
! \crosstabview: maximum number of columns (1600) exceeded
  -- error: only one column
  SELECT 1 \crosstabview
! \crosstabview: query must return at least three columns
  DROP TABLE ctv_data;
diff --git a/src/test/regress/sql/psql_crosstab.sql b/src/test/regress/sql/psql_crosstab.sql
index e602676..43c959b 100644
*** a/src/test/regress/sql/psql_crosstab.sql
--- b/src/test/regress/sql/psql_crosstab.sql
*************** SELECT v, EXTRACT(year FROM d), count(*)
*** 23,29 ****
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name":num 4

  -- ordered months in vertical header, ordered years in horizontal header
  SELECT EXTRACT(year FROM d) AS year, to_char(d,'Mon') AS "month name",
--- 23,29 ----
  -- ordered months in horizontal header, quoted column name
  SELECT v, to_char(d, 'Mon') AS "month name", EXTRACT(month FROM d) AS num,
   count(*) FROM ctv_data  GROUP BY 1,2,3 ORDER BY 1
!  \crosstabview v "month name" 4 num

  -- ordered months in vertical header, ordered years in horizontal header
  SELECT EXTRACT(year FROM d) AS year, to_char(d,'Mon') AS "month name",
*************** SELECT EXTRACT(year FROM d) AS year, to_
*** 32,38 ****
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year:year format

  -- combine contents vertically into the same cell (V/H duplicates)
  SELECT v, h, string_agg(c, E'\n') FROM ctv_data GROUP BY v, h ORDER BY 1,2,3
--- 32,38 ----
    FROM ctv_data
    GROUP BY EXTRACT(year FROM d), to_char(d,'Mon'), EXTRACT(month FROM d)
  ORDER BY month
! \crosstabview "month name" year format year

  -- combine contents vertically into the same cell (V/H duplicates)
  SELECT v, h, string_agg(c, E'\n') FROM ctv_data GROUP BY v, h ORDER BY 1,2,3
*************** SELECT v, h, string_agg(c, E'\n') FROM c
*** 41,57 ****
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c

  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c

  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h:r c

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
--- 41,57 ----
  -- horizontal ASC order from window function
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r

  -- horizontal DESC order from window function
  SELECT v, h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h DESC) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r

  -- horizontal ASC order from window function, NULLs pushed rightmost
  SELECT v,h, string_agg(c, E'\n') AS c, row_number() OVER(ORDER BY h NULLS LAST) AS r
  FROM ctv_data GROUP BY v, h ORDER BY 1,3,2
!  \crosstabview v h c r

  -- only null, no column name, 2 columns: error
  SELECT null,null \crosstabview
*************** SELECT v,h, string_agg(i::text, E'\n') A
*** 76,85 ****
--- 76,97 ----
  FROM ctv_data GROUP BY v, h ORDER BY h,v
   \crosstabview 1 "h" 4

+ -- refer to columns by quoted names, check downcasing of unquoted name
+ SELECT 1 as "22", 2 as b, 3 as "Foo"
+  \crosstabview "22" B "Foo"
+
  -- error: bad column name
  SELECT v,h,c,i FROM ctv_data
   \crosstabview v h j

+ -- error: need to quote name
+ SELECT 1 as "22", 2 as b, 3 as "Foo"
+  \crosstabview 1 2 Foo
+
+ -- error: need to not quote name
+ SELECT 1 as "22", 2 as b, 3 as "Foo"
+  \crosstabview 1 "B" "Foo"
+
  -- error: bad column number
  SELECT v,h,i,c FROM ctv_data
   \crosstabview 2 1 5

Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Tom Lane wrote:

> I wrote:
> > My feeling is that what we should do is undo the change to use OT_SQLID,
> > and in indexOfColumn() perform a downcasing/dequoting conversion that
> > duplicates what OT_SQLID does in psqlscanslash.l.
>
> Here's an updated patch that does it that way, and also adopts Christoph's
> documentation suggestion about "sortcolH".  Any further comments?

For my part, I'm fine with this. Thanks!

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



Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Tom Lane wrote:

> > "Daniel Verite" <daniel@manitou-mail.org> writes:
> >> To avoid the confusion between "2:4" and "2":"4" or 2:4,
> >> and the ambiguity with a possibly existing "2:4" column,
> >> maybe we should abandon this syntax and require the optional
> >> scolH to be on its own at the end of the command.
>
> > That would be OK with me; it's certainly less of a hack than what's
> > there now.  (I went back and forth about how much effort to put into
> > dealing with the colon syntax; I think the version I have in my patch
> > would be all right, but it's not perfect.)
>
> Here's a patch along those lines.  Any objections?

Checking http://www.postgresql.org/docs/devel/static/app-psql.html ,
I notice that the last example is still using the syntax for arguments
that has been deprecated by commit 6f0d6a507, as discussed in this
thread.
A fix to psql-ref.sgml is attached.

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

Вложения

Re: \crosstabview fixes

От
Tom Lane
Дата:
"Daniel Verite" <daniel@manitou-mail.org> writes:
> Checking http://www.postgresql.org/docs/devel/static/app-psql.html ,
> I notice that the last example is still using the syntax for arguments
> that has been deprecated by commit 6f0d6a507, as discussed in this
> thread.

Ooops.

> A fix to psql-ref.sgml is attached.

Pushed, thanks.

BTW, I see we've been spelling your name with an insufficient number
of accents in the commit logs and release notes.  Can't do much about
the logs, but will fix the release notes.
        regards, tom lane



Re: \crosstabview fixes

От
"Daniel Verite"
Дата:
    Tom Lane wrote:

> Pushed, thanks.
> BTW, I see we've been spelling your name with an insufficient number
> of accents in the commit logs and release notes.  Can't do much about
> the logs, but will fix the release notes.

I use myself the nonaccented version of my name in "From" headers, as
a concession to mailers that are not always rfc2047-friendly, so I can't
blame anyone for leaving out one or both of these accents :)
Thanks for taking care of this anyway!


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