From 24585a8c04280c59c420fe2e0365a9b86ae8a5bd Mon Sep 17 00:00:00 2001 From: Mehmet YILMAZ Date: Fri, 27 Dec 2024 15:07:38 +0300 Subject: [PATCH] Error out for ALTER TABLE ... SET ACCESS METHOD DEFAULT (#7803) PG17 introduced ALTER TABLE ... SET ACCESS METHOD DEFAULT This PR introduces and enforces an error check preventing ALTER TABLE ... SET ACCESS METHOD DEFAULT on both Citus local tables (added via citus_add_local_table_to_metadata) and distributed/partitioned distributed tables. The regression tests now demonstrate that each table type raises an error advising users to explicitly specify an access method, rather than relying on DEFAULT. This ensures consistent behavior across local and distributed environments in Citus. The reason why we currently don't support this is that we can't simply propagate the command as it is, because the default table access method may be different across Citus cluster nodes. Relevant PG commit: https://github.com/postgres/postgres/commit/d61a6cad6 --- src/backend/distributed/commands/table.c | 18 ++++++++++ src/test/regress/expected/pg17.out | 43 ++++++++++++++++++++++++ src/test/regress/sql/pg17.sql | 30 +++++++++++++++++ 3 files changed, 91 insertions(+) diff --git a/src/backend/distributed/commands/table.c b/src/backend/distributed/commands/table.c index e65f57961..c395892b5 100644 --- a/src/backend/distributed/commands/table.c +++ b/src/backend/distributed/commands/table.c @@ -3666,6 +3666,24 @@ ErrorIfUnsupportedAlterTableStmt(AlterTableStmt *alterTableStatement) #if PG_VERSION_NUM >= PG_VERSION_15 case AT_SetAccessMethod: + { + /* + * If command->name == NULL, that means the user is trying to use + * ALTER TABLE ... SET ACCESS METHOD DEFAULT + * which we don't support currently. + */ + if (command->name == NULL) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg( + "DEFAULT option in ALTER TABLE ... SET ACCESS METHOD " + "is currently unsupported."), + errhint( + "You can rerun the command by explicitly writing the access method name."))); + } + break; + } + #endif case AT_SetNotNull: case AT_ReplicaIdentity: diff --git a/src/test/regress/expected/pg17.out b/src/test/regress/expected/pg17.out index 23efee4bd..fe925516b 100644 --- a/src/test/regress/expected/pg17.out +++ b/src/test/regress/expected/pg17.out @@ -1333,6 +1333,49 @@ SELECT b, c FROM forcetest WHERE a = 6; \pset null '' -- End of Testing FORCE_NOT_NULL and FORCE_NULL options +-- Test for ALTER TABLE SET ACCESS METHOD DEFAULT +-- Step 1: Local table setup (non-distributed) +CREATE TABLE test_local_table (id int); +SELECT citus_add_local_table_to_metadata('test_local_table'); + citus_add_local_table_to_metadata +--------------------------------------------------------------------- + +(1 row) + +-- Step 2: Attempt to set access method to DEFAULT on a Citus local table (should fail) +ALTER TABLE test_local_table SET ACCESS METHOD DEFAULT; +ERROR: DEFAULT option in ALTER TABLE ... SET ACCESS METHOD is currently unsupported. +HINT: You can rerun the command by explicitly writing the access method name. +-- Step 3: Setup: create and distribute a table +CREATE TABLE test_alter_access_method (id int); +SELECT create_distributed_table('test_alter_access_method', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Step 4: Attempt to set access method to DEFAULT on a distributed table (should fail with your custom error) +ALTER TABLE test_alter_access_method SET ACCESS METHOD DEFAULT; +ERROR: DEFAULT option in ALTER TABLE ... SET ACCESS METHOD is currently unsupported. +HINT: You can rerun the command by explicitly writing the access method name. +-- Step 5: Create and distribute a partitioned table +CREATE TABLE test_partitioned_alter (id int, val text) PARTITION BY RANGE (id); +CREATE TABLE test_partitioned_alter_part1 PARTITION OF test_partitioned_alter FOR VALUES FROM (1) TO (100); +SELECT create_distributed_table('test_partitioned_alter', 'id'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- Step 6: Attempt to set access method to DEFAULT on a partitioned, distributed table (should fail) +ALTER TABLE test_partitioned_alter SET ACCESS METHOD DEFAULT; +ERROR: DEFAULT option in ALTER TABLE ... SET ACCESS METHOD is currently unsupported. +HINT: You can rerun the command by explicitly writing the access method name. +-- Cleanup +DROP TABLE test_local_table CASCADE; +DROP TABLE test_alter_access_method CASCADE; +DROP TABLE test_partitioned_alter CASCADE; +-- End of Test for ALTER TABLE SET ACCESS METHOD DEFAULT \set VERBOSITY terse SET client_min_messages TO WARNING; DROP SCHEMA pg17 CASCADE; diff --git a/src/test/regress/sql/pg17.sql b/src/test/regress/sql/pg17.sql index ab8528f4b..88d1a475f 100644 --- a/src/test/regress/sql/pg17.sql +++ b/src/test/regress/sql/pg17.sql @@ -711,6 +711,36 @@ SELECT b, c FROM forcetest WHERE a = 6; -- End of Testing FORCE_NOT_NULL and FORCE_NULL options +-- Test for ALTER TABLE SET ACCESS METHOD DEFAULT +-- Step 1: Local table setup (non-distributed) +CREATE TABLE test_local_table (id int); + +SELECT citus_add_local_table_to_metadata('test_local_table'); + +-- Step 2: Attempt to set access method to DEFAULT on a Citus local table (should fail) +ALTER TABLE test_local_table SET ACCESS METHOD DEFAULT; + +-- Step 3: Setup: create and distribute a table +CREATE TABLE test_alter_access_method (id int); +SELECT create_distributed_table('test_alter_access_method', 'id'); + +-- Step 4: Attempt to set access method to DEFAULT on a distributed table (should fail with your custom error) +ALTER TABLE test_alter_access_method SET ACCESS METHOD DEFAULT; + +-- Step 5: Create and distribute a partitioned table +CREATE TABLE test_partitioned_alter (id int, val text) PARTITION BY RANGE (id); +CREATE TABLE test_partitioned_alter_part1 PARTITION OF test_partitioned_alter FOR VALUES FROM (1) TO (100); +SELECT create_distributed_table('test_partitioned_alter', 'id'); + +-- Step 6: Attempt to set access method to DEFAULT on a partitioned, distributed table (should fail) +ALTER TABLE test_partitioned_alter SET ACCESS METHOD DEFAULT; + +-- Cleanup +DROP TABLE test_local_table CASCADE; +DROP TABLE test_alter_access_method CASCADE; +DROP TABLE test_partitioned_alter CASCADE; +-- End of Test for ALTER TABLE SET ACCESS METHOD DEFAULT + \set VERBOSITY terse SET client_min_messages TO WARNING; DROP SCHEMA pg17 CASCADE;