Disallow CREATE INDEX if table is already in use in current session.

tglsfdc · tglsfdc · commit 0d1885266630 · 2017-06-04T12:02:41.000-04:00
If we allow this, whatever outer command has the table open will not know about the new index and may fail to update it as needed, as shown in a report from Laurenz Albe. We already had such a prohibition in place for ALTER TABLE, but the CREATE INDEX syntax missed the check. Fixing it requires an API change for DefineIndex(), which conceivably would break third-party extensions if we were to back-patch it. Given how long this problem has existed without being noticed, fixing it in the back branches doesn't seem worth that risk. Discussion: https://postgr.es/m/A737B7A37273E048B164557ADEF4A58B53A4DC9A@ntex2010i.host.magwien.gv.at
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
@@ -323,6 +323,7 @@ Boot_DeclareIndexStmt:
 								$4,
 								false,
 								false,
+								false,
 								true, /* skip_build */
 								false);
 					do_end();
@@ -366,6 +367,7 @@ Boot_DeclareUniqueIndexStmt:
 								$5,
 								false,
 								false,
+								false,
 								true, /* skip_build */
 								false);
 					do_end();
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
@@ -295,6 +295,9 @@ CheckIndexCompatible(Oid oldId,
  * 'is_alter_table': this is due to an ALTER rather than a CREATE operation.
  * 'check_rights': check for CREATE rights in namespace and tablespace.  (This
  *		should be true except when ALTER is deleting/recreating an index.)
+ * 'check_not_in_use': check for table not already in use in current session.
+ *		This should be true unless caller is holding the table open, in which
+ *		case the caller had better have checked it earlier.
  * 'skip_build': make the catalog entries but leave the index file empty;
  *		it will be filled later.
  * 'quiet': suppress the NOTICE chatter ordinarily provided for constraints.
@@ -307,6 +310,7 @@ DefineIndex(Oid relationId,
 			Oid indexRelationId,
 			bool is_alter_table,
 			bool check_rights,
+			bool check_not_in_use,
 			bool skip_build,
 			bool quiet)
 {
@@ -404,6 +408,15 @@ DefineIndex(Oid relationId,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot create indexes on temporary tables of other sessions")));
 
+	/*
+	 * Unless our caller vouches for having checked this already, insist that
+	 * the table not be in use by our own session, either.  Otherwise we might
+	 * fail to make entries in the new index (for instance, if an INSERT or
+	 * UPDATE is in progress and has already made its list of target indexes).
+	 */
+	if (check_not_in_use)
+		CheckTableNotInUse(rel, "CREATE INDEX");
+
 	/*
 	 * Verify we (still) have CREATE rights in the rel's namespace.
 	 * (Presumably we did when the rel was created, but maybe not anymore.)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
@@ -6679,6 +6679,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
 						  InvalidOid,	/* no predefined OID */
 						  true, /* is_alter_table */
 						  check_rights,
+						  false,	/* check_not_in_use - we did it already */
 						  skip_build,
 						  quiet);
 
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
@@ -1329,6 +1329,7 @@ ProcessUtilitySlow(ParseState *pstate,
 									InvalidOid, /* no predefined OID */
 									false,		/* is_alter_table */
 									true,		/* check_rights */
+									true,		/* check_not_in_use */
 									false,		/* skip_build */
 									false);		/* quiet */
 
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
@@ -27,6 +27,7 @@ extern ObjectAddress DefineIndex(Oid relationId,
 			Oid indexRelationId,
 			bool is_alter_table,
 			bool check_rights,
+			bool check_not_in_use,
 			bool skip_build,
 			bool quiet);
 extern Oid	ReindexIndex(RangeVar *indexRelation, int options);
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
@@ -1674,6 +1674,35 @@ drop table self_ref_trigger;
 drop function self_ref_trigger_ins_func();
 drop function self_ref_trigger_del_func();
 --
+-- Check that index creation (or DDL in general) is prohibited in a trigger
+--
+create table trigger_ddl_table (
+   col1 integer,
+   col2 integer
+);
+create function trigger_ddl_func() returns trigger as $$
+begin
+  alter table trigger_ddl_table add primary key (col1);
+  return new;
+end$$ language plpgsql;
+create trigger trigger_ddl_func before insert on trigger_ddl_table for each row
+  execute procedure trigger_ddl_func();
+insert into trigger_ddl_table values (1, 42);  -- fail
+ERROR:  cannot ALTER TABLE "trigger_ddl_table" because it is being used by active queries in this session
+CONTEXT:  SQL statement "alter table trigger_ddl_table add primary key (col1)"
+PL/pgSQL function trigger_ddl_func() line 3 at SQL statement
+create or replace function trigger_ddl_func() returns trigger as $$
+begin
+  create index on trigger_ddl_table (col2);
+  return new;
+end$$ language plpgsql;
+insert into trigger_ddl_table values (1, 42);  -- fail
+ERROR:  cannot CREATE INDEX "trigger_ddl_table" because it is being used by active queries in this session
+CONTEXT:  SQL statement "create index on trigger_ddl_table (col2)"
+PL/pgSQL function trigger_ddl_func() line 3 at SQL statement
+drop table trigger_ddl_table;
+drop function trigger_ddl_func();
+--
 -- Verify behavior of before and after triggers with INSERT...ON CONFLICT
 -- DO UPDATE
 --
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
@@ -1183,6 +1183,37 @@ drop table self_ref_trigger;
 drop function self_ref_trigger_ins_func();
 drop function self_ref_trigger_del_func();
 
+--
+-- Check that index creation (or DDL in general) is prohibited in a trigger
+--
+
+create table trigger_ddl_table (
+   col1 integer,
+   col2 integer
+);
+
+create function trigger_ddl_func() returns trigger as $$
+begin
+  alter table trigger_ddl_table add primary key (col1);
+  return new;
+end$$ language plpgsql;
+
+create trigger trigger_ddl_func before insert on trigger_ddl_table for each row
+  execute procedure trigger_ddl_func();
+
+insert into trigger_ddl_table values (1, 42);  -- fail
+
+create or replace function trigger_ddl_func() returns trigger as $$
+begin
+  create index on trigger_ddl_table (col2);
+  return new;
+end$$ language plpgsql;
+
+insert into trigger_ddl_table values (1, 42);  -- fail
+
+drop table trigger_ddl_table;
+drop function trigger_ddl_func();
+
 --
 -- Verify behavior of before and after triggers with INSERT...ON CONFLICT
 -- DO UPDATE
