pgsql: Fix limitations on what SQL commands can be issued to a walsende

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Fix limitations on what SQL commands can be issued to a walsende
Date: 2022-01-24 20:34:14
Message-ID: E1nC62Y-0001WQ-Ga@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Fix limitations on what SQL commands can be issued to a walsender.

In logical replication mode, a WalSender is supposed to be able
to execute any regular SQL command, as well as the special
replication commands. Poor design of the replication-command
parser caused it to fail in various cases, notably:

* semicolons embedded in a command, or multiple SQL commands
sent in a single message;

* dollar-quoted literals containing odd numbers of single
or double quote marks;

* commands starting with a comment.

The basic problem here is that we're trying to run repl_scanner.l
across the entire input string even when it's not a replication
command. Since repl_scanner.l does not understand all of the
token types known to the core lexer, this is doomed to have
failure modes.

We certainly don't want to make repl_scanner.l as big as scan.l,
so instead rejigger stuff so that we only lex the first token of
a non-replication command. That will usually look like an IDENT
to repl_scanner.l, though a comment would end up getting reported
as a '-' or '/' single-character token. If the token is a replication
command keyword, we push it back and proceed normally with repl_gram.y
parsing. Otherwise, we can drop out of exec_replication_command()
without examining the rest of the string.

(It's still theoretically possible for repl_scanner.l to fail on
the first token; but that could only happen if it's an unterminated
single- or double-quoted string, in which case you'd have gotten
largely the same error from the core lexer too.)

In this way, repl_gram.y isn't involved at all in handling general
SQL commands, so we can get rid of the SQLCmd node type. (In
the back branches, we can't remove it because renumbering enum
NodeTag would be an ABI break; so just leave it sit there unused.)

I failed to resist the temptation to clean up some other sloppy
coding in repl_scanner.l while at it. The only externally-visible
behavior change from that is it now accepts \r and \f as whitespace,
same as the core lexer.

Per bug #17379 from Greg Rychlewski. Back-patch to all supported
branches.

Discussion: https://postgr.es/m/17379-6a5c6cfb3f1f5e77@postgresql.org

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/6aa5186146f15f8b78749b52caee1eabb3a1aa92

Modified Files
--------------
src/backend/replication/repl_gram.y | 26 +--------
src/backend/replication/repl_scanner.l | 87 +++++++++++++++++++++++------
src/backend/replication/walsender.c | 43 ++++++++------
src/include/nodes/nodes.h | 1 -
src/include/nodes/replnodes.h | 9 ---
src/include/replication/walsender_private.h | 1 +
6 files changed, 98 insertions(+), 69 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Andrew Dunstan 2022-01-24 20:34:30 pgsql: Add tests of the CREATEROLE attribute
Previous Message Robert Haas 2022-01-24 20:14:33 pgsql: Server-side gzip compression.