Tom Lane wrote: > Keep in mind that a large part of the reason why the \cset patch got > bounced was exactly that its detection of \; was impossibly ugly > and broken. Don't expect another patch using the same logic to > get looked on more favorably.
Looking at the end of the discussion about \cset, it seems what you were against was not much how the detection was done rather than how and why it was used thereafter. In the case of the present bug, we just need to know whether there are any \; query separators in the command string. If yes, then SendQuery() doesn't get to use the cursor technique to avoid any risk with that command string, despite FETCH_COUNT>0. PFA a simple POC patch implementing this. > Having said that, I did like the idea of maybe going over to > PQsetSingleRowMode instead of using an explicit cursor. That > would represent a net decrease of cruftiness here, instead of > layering more cruft on top of what's already a mighty ugly hack. It would also work with queries that start with a CTE, and queries like (UPDATE/INSERT/DELETE.. RETURNING), that the current way with the cursor cannot handle. But that looks like a project for PG13, whereas a fix like the attached could be backpatched. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index bd28444..78641e0 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -1405,7 +1405,8 @@ SendQuery(const char *query) results = NULL; /* PQclear(NULL) does nothing */ } else if (pset.fetch_count <= 0 || pset.gexec_flag || - pset.crosstab_flag || !is_select_command(query)) + pset.crosstab_flag || pset.num_semicolons > 0 || + !is_select_command(query)) { /* Default fetch-it-all-and-print mode */ instr_time before, diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c index 3ae4470..e5af7a2 100644 --- a/src/bin/psql/mainloop.c +++ b/src/bin/psql/mainloop.c @@ -426,6 +426,7 @@ MainLoop(FILE *source) /* execute query unless we're in an inactive \if branch */ if (conditional_active(cond_stack)) { + pset.num_semicolons = psql_scan_get_escaped_semicolons(scan_state); success = SendQuery(query_buf->data); slashCmdStatus = success ? PSQL_CMD_SEND : PSQL_CMD_ERROR; pset.stmt_lineno = 1; @@ -502,6 +503,7 @@ MainLoop(FILE *source) /* should not see this in inactive branch */ Assert(conditional_active(cond_stack)); + pset.num_semicolons = psql_scan_get_escaped_semicolons(scan_state); success = SendQuery(query_buf->data); /* transfer query to previous_buf by pointer-swapping */ diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h index 5be5091..2755927 100644 --- a/src/bin/psql/settings.h +++ b/src/bin/psql/settings.h @@ -110,6 +110,8 @@ typedef struct _psqlSettings char *inputfile; /* file being currently processed, if any */ uint64 lineno; /* also for error reporting */ uint64 stmt_lineno; /* line number inside the current statement */ + int num_semicolons; /* number of query separators (\;) found + inside the current statement */ bool timing; /* enable timing of all queries */ diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l index 850754e..ee32547 100644 --- a/src/fe_utils/psqlscan.l +++ b/src/fe_utils/psqlscan.l @@ -694,8 +694,15 @@ other . * substitution. We want these before {self}, also. */ -"\\"[;:] { - /* Force a semi-colon or colon into the query buffer */ +"\\"; { + /* Count semicolons in compound commands */ + cur_state->escaped_semicolons++; + /* Force a semicolon into the query buffer */ + psqlscan_emit(cur_state, yytext + 1, 1); + } + +"\\": { + /* Force a colon into the query buffer */ psqlscan_emit(cur_state, yytext + 1, 1); } @@ -1066,6 +1073,9 @@ psql_scan(PsqlScanState state, /* Set current output target */ state->output_buf = query_buf; + /* Reset number of escaped semicolons seen */ + state->escaped_semicolons = 0; + /* Set input source */ if (state->buffer_stack != NULL) yy_switch_to_buffer(state->buffer_stack->buf, state->scanner); @@ -1210,6 +1220,16 @@ psql_scan_reset(PsqlScanState state) } /* + * Return the number of escaped semicolons in the lexed string seen by the + * previous psql_scan call. + */ +int +psql_scan_get_escaped_semicolons(PsqlScanState state) +{ + return state->escaped_semicolons; +} + +/* * Reselect this lexer (psqlscan.l) after using another one. * * Currently and for foreseeable uses, it's sufficient to reset to INITIAL diff --git a/src/include/fe_utils/psqlscan.h b/src/include/fe_utils/psqlscan.h index c3a1d58..395ac78 100644 --- a/src/include/fe_utils/psqlscan.h +++ b/src/include/fe_utils/psqlscan.h @@ -83,6 +83,8 @@ extern PsqlScanResult psql_scan(PsqlScanState state, extern void psql_scan_reset(PsqlScanState state); +extern int psql_scan_get_escaped_semicolons(PsqlScanState state); + extern void psql_scan_reselect_sql_lexer(PsqlScanState state); extern bool psql_scan_in_quote(PsqlScanState state); diff --git a/src/include/fe_utils/psqlscan_int.h b/src/include/fe_utils/psqlscan_int.h index 42a738f..752cc94 100644 --- a/src/include/fe_utils/psqlscan_int.h +++ b/src/include/fe_utils/psqlscan_int.h @@ -112,6 +112,7 @@ typedef struct PsqlScanStateData int start_state; /* yylex's starting/finishing state */ int paren_depth; /* depth of nesting in parentheses */ int xcdepth; /* depth of nesting in slash-star comments */ + int escaped_semicolons; /* number of embedded (\;) semicolons */ char *dolqstart; /* current $foo$ quote start string */ /*