From a8f7675fba0f7cb16c2197427a1656741940a092 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Thu, 30 Jan 2025 16:53:26 +0100 Subject: [PATCH] Allow EXPLAIN in read-only mode Using `EXPLAIN` when `transaction_read_only` is set to `on` fails with an error. This is fixed by explicitly setting the `check_read_only` flag. (cherry picked from commit 39049feba6a01efd0a9075705ef44443bd62b4d7) --- .unreleased/pr_7637 | 2 + src/process_utility.c | 1 + tsl/test/expected/read_only.out | 77 ++++++++++++++++++++++++++ tsl/test/shared/expected/extension.out | 4 +- tsl/test/sql/read_only.sql | 52 +++++++++++++++++ 5 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 .unreleased/pr_7637 diff --git a/.unreleased/pr_7637 b/.unreleased/pr_7637 new file mode 100644 index 00000000000..d1c0dd47d07 --- /dev/null +++ b/.unreleased/pr_7637 @@ -0,0 +1,2 @@ +Fixes: #7637 Allow EXPLAIN in read-only mode +Thanks: @ikalafat for reporting the error diff --git a/src/process_utility.c b/src/process_utility.c index 239feebbf49..9dc1fb521d8 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -4859,6 +4859,7 @@ process_ddl_command_start(ProcessUtilityArgs *args) handler = preprocess_execute; break; case T_ExplainStmt: + check_read_only = false; handler = process_explain_start; break; diff --git a/tsl/test/expected/read_only.out b/tsl/test/expected/read_only.out index f9057559143..d478f4b3775 100644 --- a/tsl/test/expected/read_only.out +++ b/tsl/test/expected/read_only.out @@ -8,7 +8,84 @@ -- create_hypertable() -- CREATE TABLE test_table(time bigint NOT NULL, device int); +-- EXPLAIN should work in read-only mode, when enabling in transaction. +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (COSTS OFF) SELECT * FROM test_table; + QUERY PLAN +------------------------ + Seq Scan on test_table +(1 row) + +ROLLBACK; +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) SELECT * FROM test_table; + QUERY PLAN +------------------------------------------------ + Seq Scan on test_table (actual rows=0 loops=1) +(1 row) + +ROLLBACK; +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (COSTS OFF) INSERT INTO test_table VALUES (1, 1); + QUERY PLAN +---------------------- + Insert on test_table + -> Result +(2 rows) + +ROLLBACK; +-- This should give an error since we are using ANALYZE and a DML. The +-- read-only is checked when executing the actual INSERT statement, +-- not inside our code. +\set ON_ERROR_STOP 0 +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) INSERT INTO test_table VALUES (1, 1); +ERROR: cannot execute INSERT in a read-only transaction +ROLLBACK; +\set ON_ERROR_STOP 1 SET default_transaction_read_only TO on; +-- EXPLAIN should work in read-only mode, even when using the default. +START TRANSACTION; +EXPLAIN (COSTS OFF) SELECT * FROM test_table; + QUERY PLAN +------------------------ + Seq Scan on test_table +(1 row) + +ROLLBACK; +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) SELECT * FROM test_table; + QUERY PLAN +------------------------------------------------ + Seq Scan on test_table (actual rows=0 loops=1) +(1 row) + +ROLLBACK; +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (COSTS OFF) INSERT INTO test_table VALUES (1, 1); + QUERY PLAN +---------------------- + Insert on test_table + -> Result +(2 rows) + +ROLLBACK; +-- This should give an error since we are using ANALYZE and a DML. The +-- read-only is checked when executing the actual INSERT statement, +-- not inside our code. +\set ON_ERROR_STOP 0 +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) INSERT INTO test_table VALUES (1, 1); +ERROR: cannot execute INSERT in a read-only transaction +ROLLBACK; +\set ON_ERROR_STOP 1 \set ON_ERROR_STOP 0 SELECT * FROM create_hypertable('test_table', 'time'); ERROR: cannot execute create_hypertable() in a read-only transaction diff --git a/tsl/test/shared/expected/extension.out b/tsl/test/shared/expected/extension.out index f3fd30c1c84..310d4ba836c 100644 --- a/tsl/test/shared/expected/extension.out +++ b/tsl/test/shared/expected/extension.out @@ -208,6 +208,8 @@ ORDER BY pronamespace::regnamespace::text COLLATE "C", p.oid::regprocedure::text debug_waitpoint_enable(text) debug_waitpoint_id(text) debug_waitpoint_release(text) + ts_hypercore_handler(internal) + ts_hypercore_proxy_handler(internal) ts_now_mock() add_columnstore_policy(regclass,"any",boolean,interval,timestamp with time zone,text,interval,boolean) add_compression_policy(regclass,"any",boolean,interval,timestamp with time zone,text,interval,boolean) @@ -298,8 +300,6 @@ ORDER BY pronamespace::regnamespace::text COLLATE "C", p.oid::regprocedure::text time_bucket_gapfill(smallint,smallint,smallint,smallint) timescaledb_post_restore() timescaledb_pre_restore() - ts_hypercore_handler(internal) - ts_hypercore_proxy_handler(internal) timescaledb_experimental.add_policies(regclass,boolean,"any","any","any","any",boolean) timescaledb_experimental.alter_policies(regclass,boolean,"any","any","any","any") timescaledb_experimental.remove_all_policies(regclass,boolean) diff --git a/tsl/test/sql/read_only.sql b/tsl/test/sql/read_only.sql index 4c0eec46629..cfcd3fce89e 100644 --- a/tsl/test/sql/read_only.sql +++ b/tsl/test/sql/read_only.sql @@ -12,8 +12,59 @@ -- CREATE TABLE test_table(time bigint NOT NULL, device int); +-- EXPLAIN should work in read-only mode, when enabling in transaction. +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (COSTS OFF) SELECT * FROM test_table; +ROLLBACK; + +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) SELECT * FROM test_table; +ROLLBACK; + +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (COSTS OFF) INSERT INTO test_table VALUES (1, 1); +ROLLBACK; + +-- This should give an error since we are using ANALYZE and a DML. The +-- read-only is checked when executing the actual INSERT statement, +-- not inside our code. +\set ON_ERROR_STOP 0 +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) INSERT INTO test_table VALUES (1, 1); +ROLLBACK; +\set ON_ERROR_STOP 1 + SET default_transaction_read_only TO on; +-- EXPLAIN should work in read-only mode, even when using the default. +START TRANSACTION; +EXPLAIN (COSTS OFF) SELECT * FROM test_table; +ROLLBACK; + +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) SELECT * FROM test_table; +ROLLBACK; + +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (COSTS OFF) INSERT INTO test_table VALUES (1, 1); +ROLLBACK; + +-- This should give an error since we are using ANALYZE and a DML. The +-- read-only is checked when executing the actual INSERT statement, +-- not inside our code. +\set ON_ERROR_STOP 0 +START TRANSACTION; +SET transaction_read_only TO on; +EXPLAIN (ANALYZE ON, COSTS OFF, TIMING OFF, SUMMARY OFF) INSERT INTO test_table VALUES (1, 1); +ROLLBACK; +\set ON_ERROR_STOP 1 + \set ON_ERROR_STOP 0 SELECT * FROM create_hypertable('test_table', 'time'); \set ON_ERROR_STOP 1 @@ -213,3 +264,4 @@ SELECT remove_retention_policy('test_table'); SELECT add_job('now','12h'); SELECT alter_job(1,scheduled:=false); SELECT delete_job(1); +