From: | Robert Haas <rhaas(at)postgresql(dot)org> |
---|---|
To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
Subject: | pgsql: Fix problems with "read only query" checks, and refactor the cod |
Date: | 2020-01-16 17:13:01 |
Message-ID: | E1is8hZ-0000yL-QR@gemulon.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
Fix problems with "read only query" checks, and refactor the code.
Previously, check_xact_readonly() was responsible for determining
which types of queries could not be run in a read-only transaction,
standard_ProcessUtility() was responsibility for prohibiting things
which were allowed in read only transactions but not in recovery, and
utility commands were basically prohibited in bulk in parallel mode by
calls to CommandIsReadOnly() in functions.c and spi.c. This situation
was confusing and error-prone. Accordingly, move all the checks to a
new function ClassifyUtilityCommandAsReadOnly(), which determines the
degree to which a given statement is read only.
In the old code, check_xact_readonly() inadvertently failed to handle
several statement types that actually should have been prohibited,
specifically T_CreatePolicyStmt, T_AlterPolicyStmt, T_CreateAmStmt,
T_CreateStatsStmt, T_AlterStatsStmt, and T_AlterCollationStmt. As a
result, thes statements were erroneously allowed in read only
transactions, parallel queries, and standby operation. Generally, they
would fail anyway due to some lower-level error check, but we
shouldn't rely on that. In the new code structure, future omissions
of this type should cause ClassifyUtilityCommandAsReadOnly() to
complain about an unrecognized node type.
As a fringe benefit, this means we can allow certain types of utility
commands in parallel mode, where it's safe to do so. This allows
ALTER SYSTEM, CALL, DO, CHECKPOINT, COPY FROM, EXPLAIN, and SHOW.
It might be possible to allow additional commands with more work
and thought.
Along the way, document the thinking process behind the current set
of checks, as per discussion especially with Peter Eisentraut. There
is some interest in revising some of these rules, but that seems
like a job for another patch.
Patch by me, reviewed by Tom Lane, Stephen Frost, and Peter
Eisentraut.
Discussion: http://postgr.es/m/CA+TgmoZ_rLqJt5sYkvh+JpQnfX0Y+B2R+qfi820xNih6x-FQOQ@mail.gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/2eb34ac369741c110b593e2dc2195c57d29ab8e8
Modified Files
--------------
src/backend/commands/copy.c | 1 -
src/backend/commands/lockcmds.c | 11 --
src/backend/executor/functions.c | 3 -
src/backend/executor/spi.c | 31 ++--
src/backend/tcop/utility.c | 350 ++++++++++++++++++++++++++++-----------
src/include/tcop/utility.h | 31 ++++
6 files changed, 292 insertions(+), 135 deletions(-)
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-01-16 20:59:15 | pgsql: Update header comments for wchar.c and encnames.c. |
Previous Message | Tom Lane | 2020-01-16 16:31:34 | pgsql: Minor code beautification in regexp.c. |