Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commitd76652e

Browse files
committed
Fix concurrent indexing operations with temporary tables
Attempting to use CREATE INDEX, DROP INDEX or REINDEX with CONCURRENTLYon a temporary relation with ON COMMIT actions triggered unexpectederrors because those operations use multiple transactions internally tocomplete their work. Here is for example one confusing error when usingON COMMIT DELETE ROWS:ERROR: index "foo" already contains dataIssues related to temporary relations and concurrent indexing are fixedin this commit by enforcing the non-concurrent path to be taken fortemporary relations even if using CONCURRENTLY, transparently to theuser. Using a non-concurrent path does not matter in practice as lockscannot be taken on a temporary relation by a session different than theone owning the relation, and the non-concurrent operation is moreeffective.The problem exists with REINDEX since v12 with the introduction ofCONCURRENTLY, and with CREATE/DROP INDEX since CONCURRENTLY exists forthose commands. In all supported versions, this caused only confusingerror messages to be generated. Note that with REINDEX, it was alsopossible to issue a REINDEX CONCURRENTLY for a temporary relation ownedby a different session, leading to a server crash.The idea to enforce transparently the non-concurrent code path fortemporary relations comes originally from Andres Freund.Reported-by: Manuel RiggerAuthor: Michael Paquier, Heikki LinnakangasReviewed-by: Andres Freund, Álvaro Herrera, Heikki LinnakangasDiscussion:https://postgr.es/m/CA+u7OA6gP7YAeCguyseusYcc=uR8+ypjCcgDDCTzjQ+k6S9ksQ@mail.gmail.comBackpatch-through: 9.4
1 parentba1dfbe commitd76652e

File tree

9 files changed

+129
-8
lines changed

9 files changed

+129
-8
lines changed

‎doc/src/sgml/ref/create_index.sgml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable class="parameter">name</
123123
&mdash; see <xref linkend="SQL-CREATEINDEX-CONCURRENTLY"
124124
endterm="SQL-CREATEINDEX-CONCURRENTLY-title">.
125125
</para>
126+
<para>
127+
For temporary tables, <command>CREATE INDEX</command> is always
128+
non-concurrent, as no other session can access them, and
129+
non-concurrent index creation is cheaper.
130+
</para>
126131
</listitem>
127132
</varlistentry>
128133

‎doc/src/sgml/ref/drop_index.sgml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ DROP INDEX [ CONCURRENTLY ] [ IF EXISTS ] <replaceable class="PARAMETER">name</r
5858
performed within a transaction block, but
5959
<command>DROP INDEX CONCURRENTLY</> cannot.
6060
</para>
61+
<para>
62+
For temporary tables, <command>DROP INDEX</command> is always
63+
non-concurrent, as no other session can access them, and
64+
non-concurrent index drop is cheaper.
65+
</para>
6166
</listitem>
6267
</varlistentry>
6368

‎src/backend/catalog/index.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,15 @@ index_drop(Oid indexId, bool concurrent)
13361336
LOCKTAGheaplocktag;
13371337
LOCKMODElockmode;
13381338

1339+
/*
1340+
* A temporary relation uses a non-concurrent DROP. Other backends can't
1341+
* access a temporary relation, so there's no harm in grabbing a stronger
1342+
* lock (see comments in RemoveRelations), and a non-concurrent DROP is
1343+
* more efficient.
1344+
*/
1345+
Assert(get_rel_persistence(indexId)!=RELPERSISTENCE_TEMP||
1346+
!concurrent);
1347+
13391348
/*
13401349
* To drop an index safely, we must grab exclusive lock on its parent
13411350
* table. Exclusive lock on the index alone is insufficient because

‎src/backend/commands/indexcmds.c

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ DefineIndex(Oid relationId,
302302
boolskip_build,
303303
boolquiet)
304304
{
305+
boolconcurrent;
305306
char*indexRelationName;
306307
char*accessMethodName;
307308
Oid*typeObjectId;
@@ -330,6 +331,18 @@ DefineIndex(Oid relationId,
330331
Snapshotsnapshot;
331332
inti;
332333

334+
/*
335+
* Force non-concurrent build on temporary relations, even if CONCURRENTLY
336+
* was requested. Other backends can't access a temporary relation, so
337+
* there's no harm in grabbing a stronger lock, and a non-concurrent DROP
338+
* is more efficient. Do this before any use of the concurrent option is
339+
* done.
340+
*/
341+
if (stmt->concurrent&&get_rel_persistence(relationId)!=RELPERSISTENCE_TEMP)
342+
concurrent= true;
343+
else
344+
concurrent= false;
345+
333346
/*
334347
* count attributes in index
335348
*/
@@ -355,7 +368,7 @@ DefineIndex(Oid relationId,
355368
* relation. To avoid lock upgrade hazards, that lock should be at least
356369
* as strong as the one we take here.
357370
*/
358-
lockmode=stmt->concurrent ?ShareUpdateExclusiveLock :ShareLock;
371+
lockmode=concurrent ?ShareUpdateExclusiveLock :ShareLock;
359372
rel=heap_open(relationId,lockmode);
360373

361374
relationId=RelationGetRelid(rel);
@@ -540,8 +553,8 @@ DefineIndex(Oid relationId,
540553
indexInfo->ii_ExclusionStrats=NULL;
541554
indexInfo->ii_Unique=stmt->unique;
542555
/* In a concurrent build, mark it not-ready-for-inserts */
543-
indexInfo->ii_ReadyForInserts= !stmt->concurrent;
544-
indexInfo->ii_Concurrent=stmt->concurrent;
556+
indexInfo->ii_ReadyForInserts= !concurrent;
557+
indexInfo->ii_Concurrent=concurrent;
545558
indexInfo->ii_BrokenHotChain= false;
546559

547560
typeObjectId= (Oid*)palloc(numberOfAttributes*sizeof(Oid));
@@ -592,7 +605,7 @@ DefineIndex(Oid relationId,
592605
* A valid stmt->oldNode implies that we already have a built form of the
593606
* index. The caller should also decline any index build.
594607
*/
595-
Assert(!OidIsValid(stmt->oldNode)|| (skip_build&& !stmt->concurrent));
608+
Assert(!OidIsValid(stmt->oldNode)|| (skip_build&& !concurrent));
596609

597610
/*
598611
* Make the catalog entries for the index, including constraints. Then, if
@@ -606,15 +619,15 @@ DefineIndex(Oid relationId,
606619
coloptions,reloptions,stmt->primary,
607620
stmt->isconstraint,stmt->deferrable,stmt->initdeferred,
608621
allowSystemTableMods,
609-
skip_build||stmt->concurrent,
610-
stmt->concurrent, !check_rights);
622+
skip_build||concurrent,
623+
concurrent, !check_rights);
611624

612625
/* Add any requested comment */
613626
if (stmt->idxcomment!=NULL)
614627
CreateComments(indexRelationId,RelationRelationId,0,
615628
stmt->idxcomment);
616629

617-
if (!stmt->concurrent)
630+
if (!concurrent)
618631
{
619632
/* Close the heap and we're done, in the non-concurrent case */
620633
heap_close(rel,NoLock);

‎src/backend/commands/tablecmds.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,11 @@ RemoveRelations(DropStmt *drop)
784784
/* DROP CONCURRENTLY uses a weaker lock, and has some restrictions */
785785
if (drop->concurrent)
786786
{
787-
flags |=PERFORM_DELETION_CONCURRENTLY;
787+
/*
788+
* Note that for temporary relations this lock may get upgraded
789+
* later on, but as no other session can access a temporary
790+
* relation, this is actually fine.
791+
*/
788792
lockmode=ShareUpdateExclusiveLock;
789793
Assert(drop->removeType==OBJECT_INDEX);
790794
if (list_length(drop->objects)!=1)
@@ -875,6 +879,18 @@ RemoveRelations(DropStmt *drop)
875879
continue;
876880
}
877881

882+
/*
883+
* Decide if concurrent mode needs to be used here or not. The
884+
* relation persistence cannot be known without its OID.
885+
*/
886+
if (drop->concurrent&&
887+
get_rel_persistence(relOid)!=RELPERSISTENCE_TEMP)
888+
{
889+
Assert(list_length(drop->objects)==1&&
890+
drop->removeType==OBJECT_INDEX);
891+
flags |=PERFORM_DELETION_CONCURRENTLY;
892+
}
893+
878894
/* OK, we're ready to delete this one */
879895
obj.classId=RelationRelationId;
880896
obj.objectId=relOid;

‎src/backend/utils/cache/lsyscache.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,28 @@ get_rel_tablespace(Oid relid)
17971797
returnInvalidOid;
17981798
}
17991799

1800+
/*
1801+
* get_rel_persistence
1802+
*
1803+
*Returns the relpersistence associated with a given relation.
1804+
*/
1805+
char
1806+
get_rel_persistence(Oidrelid)
1807+
{
1808+
HeapTupletp;
1809+
Form_pg_classreltup;
1810+
charresult;
1811+
1812+
tp=SearchSysCache1(RELOID,ObjectIdGetDatum(relid));
1813+
if (!HeapTupleIsValid(tp))
1814+
elog(ERROR,"cache lookup failed for relation %u",relid);
1815+
reltup= (Form_pg_class)GETSTRUCT(tp);
1816+
result=reltup->relpersistence;
1817+
ReleaseSysCache(tp);
1818+
1819+
returnresult;
1820+
}
1821+
18001822

18011823
/*---------- TYPE CACHE ---------- */
18021824

‎src/include/utils/lsyscache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ extern Oidget_rel_namespace(Oid relid);
103103
externOidget_rel_type_id(Oidrelid);
104104
externcharget_rel_relkind(Oidrelid);
105105
externOidget_rel_tablespace(Oidrelid);
106+
externcharget_rel_persistence(Oidrelid);
106107
externboolget_typisdefined(Oidtypid);
107108
externint16get_typlen(Oidtypid);
108109
externboolget_typbyval(Oidtypid);

‎src/test/regress/expected/create_index.out

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2387,6 +2387,31 @@ Indexes:
23872387
"concur_index5" btree (f2) WHERE f1 = 'x'::text
23882388
"std_index" btree (f2)
23892389

2390+
-- Temporary tables with concurrent builds and on-commit actions
2391+
-- CONCURRENTLY used with CREATE INDEX and DROP INDEX is ignored.
2392+
-- PRESERVE ROWS, the default.
2393+
CREATE TEMP TABLE concur_temp (f1 int, f2 text)
2394+
ON COMMIT PRESERVE ROWS;
2395+
INSERT INTO concur_temp VALUES (1, 'foo'), (2, 'bar');
2396+
CREATE INDEX CONCURRENTLY concur_temp_ind ON concur_temp(f1);
2397+
DROP INDEX CONCURRENTLY concur_temp_ind;
2398+
DROP TABLE concur_temp;
2399+
-- ON COMMIT DROP
2400+
BEGIN;
2401+
CREATE TEMP TABLE concur_temp (f1 int, f2 text)
2402+
ON COMMIT DROP;
2403+
INSERT INTO concur_temp VALUES (1, 'foo'), (2, 'bar');
2404+
-- Fails when running in a transaction.
2405+
CREATE INDEX CONCURRENTLY concur_temp_ind ON concur_temp(f1);
2406+
ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block
2407+
COMMIT;
2408+
-- ON COMMIT DELETE ROWS
2409+
CREATE TEMP TABLE concur_temp (f1 int, f2 text)
2410+
ON COMMIT DELETE ROWS;
2411+
INSERT INTO concur_temp VALUES (1, 'foo'), (2, 'bar');
2412+
CREATE INDEX CONCURRENTLY concur_temp_ind ON concur_temp(f1);
2413+
DROP INDEX CONCURRENTLY concur_temp_ind;
2414+
DROP TABLE concur_temp;
23902415
--
23912416
-- Try some concurrent index drops
23922417
--

‎src/test/regress/sql/create_index.sql

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,31 @@ VACUUM FULL concur_heap;
756756
REINDEX TABLE concur_heap;
757757
\d concur_heap
758758

759+
-- Temporary tables with concurrent builds and on-commit actions
760+
-- CONCURRENTLY used with CREATE INDEX and DROP INDEX is ignored.
761+
-- PRESERVE ROWS, the default.
762+
CREATE TEMP TABLE concur_temp (f1int, f2text)
763+
ONCOMMIT PRESERVE ROWS;
764+
INSERT INTO concur_tempVALUES (1,'foo'), (2,'bar');
765+
CREATEINDEXCONCURRENTLY concur_temp_indON concur_temp(f1);
766+
DROPINDEX CONCURRENTLY concur_temp_ind;
767+
DROPTABLE concur_temp;
768+
-- ON COMMIT DROP
769+
BEGIN;
770+
CREATE TEMP TABLE concur_temp (f1int, f2text)
771+
ONCOMMIT DROP;
772+
INSERT INTO concur_tempVALUES (1,'foo'), (2,'bar');
773+
-- Fails when running in a transaction.
774+
CREATEINDEXCONCURRENTLY concur_temp_indON concur_temp(f1);
775+
COMMIT;
776+
-- ON COMMIT DELETE ROWS
777+
CREATE TEMP TABLE concur_temp (f1int, f2text)
778+
ONCOMMITDELETE ROWS;
779+
INSERT INTO concur_tempVALUES (1,'foo'), (2,'bar');
780+
CREATEINDEXCONCURRENTLY concur_temp_indON concur_temp(f1);
781+
DROPINDEX CONCURRENTLY concur_temp_ind;
782+
DROPTABLE concur_temp;
783+
759784
--
760785
-- Try some concurrent index drops
761786
--

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp