From: Tom Lane Date: Sun, 4 Jun 2017 16:02:31 +0000 (-0400) Subject: Disallow CREATE INDEX if table is already in use in current session. X-Git-Tag: REL_10_BETA2~238 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0d1885266630eee1de5c43af463fe2b921451932;p=postgresql 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 de3695c7e0..2e1fef0350 100644 --- 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 486179938c..c0902794e9 100644 --- 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 7959120f53..b61fda9909 100644 --- 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 1e941fbd60..a22fd5397e 100644 --- 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 79f3be36e4..5dd14b43d3 100644 --- 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 0d560fb3ee..29b8adf1e2 100644 --- 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 5581fcb164..9f2ed88f20 100644 --- 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