Thread: \crosstabview fixes
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;
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
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
"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
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
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: 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
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: 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
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
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
"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
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
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
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
Attachment
"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
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