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

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
index de3695c7e05f3981119afdf9c996856b198ac33e..2e1fef03504e57a0d88ef84ead740d8576571742 100644 (file)
--- 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
index 486179938c3e8537dc2cf631f4693a488168a53d..c0902794e98f14b5e7800135c31b6f15fb62ceaa 100644 (file)
--- 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
index 7959120f53eb3a17210a55a124f2f301982f3eee..b61fda9909d5d6dae53d4cce114f2c8c0b6a0171 100644 (file)
--- 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
index 1e941fbd600276b9c99b81540b369f9ef058e642..a22fd5397e59d0fb57aead07252930ca246ec4ea 100644 (file)
--- 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
index 79f3be36e496dcadc8007d556042562b1e45ae39..5dd14b43d3b1e886a571e71ff65b2c7de52449f1 100644 (file)
--- 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
index 0d560fb3eed3519f09fbe77d00b20faabefa9c2d..29b8adf1e23cc85554f2acc2cbf6d346280a43bb 100644 (file)
--- 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
index 5581fcb16485ac7ccee7fd79a936e5695841b15e..9f2ed88f20981c3f55da31c38da7d7c1d0c7bfe2 100644 (file)
--- 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