* alter index/table rename weird syntax supported,

* correct the wrong level of lock if the weird syntax is used
2022-07-04 12:17:44 +03:00 · 2022-07-04 12:17:44 +03:00 · d53a7760b0
parent 4e9ea834b6
commit d53a7760b0
3 changed files with 156 additions and 4 deletions
--- a/src/backend/distributed/commands/rename.c
+++ b/src/backend/distributed/commands/rename.c
@ -48,11 +48,12 @@ PreprocessRenameStmt(Node *node, const char *renameCommand,

 	/*
 	 * The lock levels here should be same as the ones taken in
-	 * RenameRelation(), renameatt() and RenameConstraint(). However, since all
-	 * four statements have identical lock levels, we just use a single statement.
+	 * RenameRelation(), renameatt() and RenameConstraint(). All statements
+	 * have identical lock levels except alter index rename.
 	 */
-	objectRelationId = RangeVarGetRelid(renameStmt->relation,
-										AccessExclusiveLock,
+	LOCKMODE lockmode = (IsIndexRenameStmt(renameStmt)) ?
+						ShareUpdateExclusiveLock : AccessExclusiveLock;
+	objectRelationId = RangeVarGetRelid(renameStmt->relation, lockmode,
 										renameStmt->missing_ok);

 	/*
@ -99,6 +100,18 @@ PreprocessRenameStmt(Node *node, const char *renameCommand,
 		case OBJECT_TABCONSTRAINT:
 		case OBJECT_POLICY:
 		{
+			if (relKind == RELKIND_INDEX ||
+				relKind == RELKIND_PARTITIONED_INDEX)
+			{
+				/*
+				 * Although weird, postgres allows ALTER TABLE .. RENAME command
+				 * on indexes. We don't want to break non-distributed tables,
+				 * so allow.
+				 */
+				tableRelationId = IndexGetRelation(objectRelationId, false);
+				break;
+			}
+
 			/* the target object is our tableRelationId. */
 			tableRelationId = objectRelationId;
 			break;
@ -106,6 +119,25 @@ PreprocessRenameStmt(Node *node, const char *renameCommand,

 		case OBJECT_INDEX:
 		{
+			if (relKind == RELKIND_RELATION ||
+				relKind == RELKIND_PARTITIONED_TABLE)
+			{
+				/*
+				 * Although weird, postgres allows ALTER INDEX .. RENAME command
+				 * on tables. We don't want to break non-distributed tables,
+				 * so allow.
+				 * Because of the weird syntax, we locked with wrong level, so relock
+				 * the relation to acquire true level of lock. Same logic
+				 * can be found in the function RenameRelation(RenameStmt) at tablecmds.c
+				 */
+				UnlockRelationOid(objectRelationId, lockmode);
+				objectRelationId = RangeVarGetRelid(renameStmt->relation,
+													AccessExclusiveLock,
+													renameStmt->missing_ok);
+				tableRelationId = objectRelationId;
+				break;
+			}
+
 			/*
 			 * here, objRelationId points to the index relation entry, and we
 			 * are interested into the entry of the table on which the index is
--- a/src/test/regress/expected/index_create.out
+++ b/src/test/regress/expected/index_create.out
@ -38,3 +38,73 @@ ALTER INDEX idx2 ALTER COLUMN 1 SET STATISTICS 1000;
 -- test reindex
 REINDEX INDEX idx1;
 ALTER TABLE test_tbl REPLICA IDENTITY USING INDEX a_index;
+-- postgres allows ALTER INDEX rename on tables, and so Citus..
+-- and, also ALTER TABLE rename on indexes..
+CREATE TABLE alter_idx_rename_test (a INT);
+CREATE INDEX alter_idx_rename_test_idx ON alter_idx_rename_test (a);
+CREATE TABLE alter_idx_rename_test_parted (a INT) PARTITION BY LIST (a);
+CREATE INDEX alter_idx_rename_test_parted_idx ON alter_idx_rename_test_parted (a);
+BEGIN;
+-- rename index/table with weird syntax
+ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2;
+ALTER TABLE alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2;
+ALTER INDEX alter_idx_rename_test_parted RENAME TO alter_idx_rename_test_parted_2;
+ALTER TABLE alter_idx_rename_test_parted_idx RENAME TO alter_idx_rename_test_parted_idx_2;
+-- also, rename index/table with proper syntax
+ALTER INDEX alter_idx_rename_test_idx_2 RENAME TO alter_idx_rename_test_idx_3;
+ALTER TABLE alter_idx_rename_test_2 RENAME TO alter_idx_rename_test_3;
+ALTER INDEX alter_idx_rename_test_parted_idx_2 RENAME TO alter_idx_rename_test_parted_idx_3;
+ALTER TABLE alter_idx_rename_test_parted_2 RENAME TO alter_idx_rename_test_parted_3;
+SELECT 'alter_idx_rename_test_3'::regclass, 'alter_idx_rename_test_idx_3'::regclass;
+        regclass         |          regclass
+---------------------------------------------------------------------
+ alter_idx_rename_test_3 | alter_idx_rename_test_idx_3
+(1 row)
+
+SELECT 'alter_idx_rename_test_parted_3'::regclass, 'alter_idx_rename_test_parted_idx_3'::regclass;
+        regclass         |          regclass
+---------------------------------------------------------------------
+ alter_idx_rename_test_parted_3 | alter_idx_rename_test_parted_idx_3
+(1 row)
+
+ROLLBACK;
+-- now, on distributed tables
+SELECT create_distributed_table('alter_idx_rename_test', 'a');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+SELECT create_distributed_table('alter_idx_rename_test_parted', 'a');
+ create_distributed_table
+---------------------------------------------------------------------
+
+(1 row)
+
+-- rename index/table with weird syntax
+ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2;
+ALTER TABLE alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2;
+ALTER INDEX alter_idx_rename_test_parted RENAME TO alter_idx_rename_test_parted_2;
+ALTER TABLE alter_idx_rename_test_parted_idx RENAME TO alter_idx_rename_test_parted_idx_2;
+-- also, rename index/table with proper syntax
+ALTER INDEX alter_idx_rename_test_idx_2 RENAME TO alter_idx_rename_test_idx_3;
+ALTER TABLE alter_idx_rename_test_2 RENAME TO alter_idx_rename_test_3;
+ALTER INDEX alter_idx_rename_test_parted_idx_2 RENAME TO alter_idx_rename_test_parted_idx_3;
+ALTER TABLE alter_idx_rename_test_parted_2 RENAME TO alter_idx_rename_test_parted_3;
+SELECT 'alter_idx_rename_test_3'::regclass, 'alter_idx_rename_test_idx_3'::regclass;
+        regclass         |          regclass
+---------------------------------------------------------------------
+ alter_idx_rename_test_3 | alter_idx_rename_test_idx_3
+(1 row)
+
+SELECT 'alter_idx_rename_test_parted_3'::regclass, 'alter_idx_rename_test_parted_idx_3'::regclass;
+        regclass         |          regclass
+---------------------------------------------------------------------
+ alter_idx_rename_test_parted_3 | alter_idx_rename_test_parted_idx_3
+(1 row)
+
+ALTER INDEX alter_idx_rename_test_idx_3 RENAME TO alter_idx_rename_test_idx_4;
+DROP INDEX alter_idx_rename_test_idx_4;
+DROP TABLE alter_idx_rename_test_3;
+DROP INDEX alter_idx_rename_test_parted_idx_3;
+DROP TABLE alter_idx_rename_test_parted_3;
--- a/src/test/regress/sql/index_create.sql
+++ b/src/test/regress/sql/index_create.sql
@ -39,3 +39,53 @@ ALTER INDEX idx2 ALTER COLUMN 1 SET STATISTICS 1000;
 REINDEX INDEX idx1;

 ALTER TABLE test_tbl REPLICA IDENTITY USING INDEX a_index;
+
+-- postgres allows ALTER INDEX rename on tables, and so Citus..
+-- and, also ALTER TABLE rename on indexes..
+CREATE TABLE alter_idx_rename_test (a INT);
+CREATE INDEX alter_idx_rename_test_idx ON alter_idx_rename_test (a);
+CREATE TABLE alter_idx_rename_test_parted (a INT) PARTITION BY LIST (a);
+CREATE INDEX alter_idx_rename_test_parted_idx ON alter_idx_rename_test_parted (a);
+BEGIN;
+-- rename index/table with weird syntax
+ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2;
+ALTER TABLE alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2;
+ALTER INDEX alter_idx_rename_test_parted RENAME TO alter_idx_rename_test_parted_2;
+ALTER TABLE alter_idx_rename_test_parted_idx RENAME TO alter_idx_rename_test_parted_idx_2;
+
+-- also, rename index/table with proper syntax
+ALTER INDEX alter_idx_rename_test_idx_2 RENAME TO alter_idx_rename_test_idx_3;
+ALTER TABLE alter_idx_rename_test_2 RENAME TO alter_idx_rename_test_3;
+ALTER INDEX alter_idx_rename_test_parted_idx_2 RENAME TO alter_idx_rename_test_parted_idx_3;
+ALTER TABLE alter_idx_rename_test_parted_2 RENAME TO alter_idx_rename_test_parted_3;
+
+SELECT 'alter_idx_rename_test_3'::regclass, 'alter_idx_rename_test_idx_3'::regclass;
+SELECT 'alter_idx_rename_test_parted_3'::regclass, 'alter_idx_rename_test_parted_idx_3'::regclass;
+
+ROLLBACK;
+
+-- now, on distributed tables
+SELECT create_distributed_table('alter_idx_rename_test', 'a');
+SELECT create_distributed_table('alter_idx_rename_test_parted', 'a');
+
+-- rename index/table with weird syntax
+ALTER INDEX alter_idx_rename_test RENAME TO alter_idx_rename_test_2;
+ALTER TABLE alter_idx_rename_test_idx RENAME TO alter_idx_rename_test_idx_2;
+ALTER INDEX alter_idx_rename_test_parted RENAME TO alter_idx_rename_test_parted_2;
+ALTER TABLE alter_idx_rename_test_parted_idx RENAME TO alter_idx_rename_test_parted_idx_2;
+
+-- also, rename index/table with proper syntax
+ALTER INDEX alter_idx_rename_test_idx_2 RENAME TO alter_idx_rename_test_idx_3;
+ALTER TABLE alter_idx_rename_test_2 RENAME TO alter_idx_rename_test_3;
+ALTER INDEX alter_idx_rename_test_parted_idx_2 RENAME TO alter_idx_rename_test_parted_idx_3;
+ALTER TABLE alter_idx_rename_test_parted_2 RENAME TO alter_idx_rename_test_parted_3;
+
+SELECT 'alter_idx_rename_test_3'::regclass, 'alter_idx_rename_test_idx_3'::regclass;
+SELECT 'alter_idx_rename_test_parted_3'::regclass, 'alter_idx_rename_test_parted_idx_3'::regclass;
+
+ALTER INDEX alter_idx_rename_test_idx_3 RENAME TO alter_idx_rename_test_idx_4;
+DROP INDEX alter_idx_rename_test_idx_4;
+DROP TABLE alter_idx_rename_test_3;
+
+DROP INDEX alter_idx_rename_test_parted_idx_3;
+DROP TABLE alter_idx_rename_test_parted_3;