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

Commit5b4b07f

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 parentc8e0e56 commit5b4b07f

File tree

7 files changed

+112
-8
lines changed

7 files changed

+112
-8
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
129129
&mdash; see <xref linkend="sql-createindex-concurrently"
130130
endterm="sql-createindex-concurrently-title"/>.
131131
</para>
132+
<para>
133+
For temporary tables, <command>CREATE INDEX</command> is always
134+
non-concurrent, as no other session can access them, and
135+
non-concurrent index creation is cheaper.
136+
</para>
132137
</listitem>
133138
</varlistentry>
134139

‎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</command> 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
@@ -1493,6 +1493,15 @@ index_drop(Oid indexId, bool concurrent)
14931493
LOCKTAGheaplocktag;
14941494
LOCKMODElockmode;
14951495

1496+
/*
1497+
* A temporary relation uses a non-concurrent DROP. Other backends can't
1498+
* access a temporary relation, so there's no harm in grabbing a stronger
1499+
* lock (see comments in RemoveRelations), and a non-concurrent DROP is
1500+
* more efficient.
1501+
*/
1502+
Assert(get_rel_persistence(indexId)!=RELPERSISTENCE_TEMP||
1503+
!concurrent);
1504+
14961505
/*
14971506
* To drop an index safely, we must grab exclusive lock on its parent
14981507
* table. Exclusive lock on the index alone is insufficient because

‎src/backend/commands/indexcmds.c

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ DefineIndex(Oid relationId,
335335
boolskip_build,
336336
boolquiet)
337337
{
338+
boolconcurrent;
338339
char*indexRelationName;
339340
char*accessMethodName;
340341
Oid*typeObjectId;
@@ -371,6 +372,18 @@ DefineIndex(Oid relationId,
371372
Snapshotsnapshot;
372373
inti;
373374

375+
/*
376+
* Force non-concurrent build on temporary relations, even if CONCURRENTLY
377+
* was requested. Other backends can't access a temporary relation, so
378+
* there's no harm in grabbing a stronger lock, and a non-concurrent DROP
379+
* is more efficient. Do this before any use of the concurrent option is
380+
* done.
381+
*/
382+
if (stmt->concurrent&&get_rel_persistence(relationId)!=RELPERSISTENCE_TEMP)
383+
concurrent= true;
384+
else
385+
concurrent= false;
386+
374387
/*
375388
* count key attributes in index
376389
*/
@@ -413,7 +426,7 @@ DefineIndex(Oid relationId,
413426
* parallel workers under the control of certain particular ambuild
414427
* functions will need to be updated, too.
415428
*/
416-
lockmode=stmt->concurrent ?ShareUpdateExclusiveLock :ShareLock;
429+
lockmode=concurrent ?ShareUpdateExclusiveLock :ShareLock;
417430
rel=heap_open(relationId,lockmode);
418431

419432
relationId=RelationGetRelid(rel);
@@ -457,6 +470,12 @@ DefineIndex(Oid relationId,
457470
partitioned=rel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE;
458471
if (partitioned)
459472
{
473+
/*
474+
* Note: we check 'stmt->concurrent' rather than 'concurrent', so that
475+
* the error is thrown also for temporary tables. Seems better to be
476+
* consistent, even though we could do it on temporary table because
477+
* we're not actually doing it concurrently.
478+
*/
460479
if (stmt->concurrent)
461480
ereport(ERROR,
462481
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -645,8 +664,8 @@ DefineIndex(Oid relationId,
645664
indexInfo->ii_ExclusionStrats=NULL;
646665
indexInfo->ii_Unique=stmt->unique;
647666
/* In a concurrent build, mark it not-ready-for-inserts */
648-
indexInfo->ii_ReadyForInserts= !stmt->concurrent;
649-
indexInfo->ii_Concurrent=stmt->concurrent;
667+
indexInfo->ii_ReadyForInserts= !concurrent;
668+
indexInfo->ii_Concurrent=concurrent;
650669
indexInfo->ii_BrokenHotChain= false;
651670
indexInfo->ii_ParallelWorkers=0;
652671
indexInfo->ii_Am=accessMethodId;
@@ -814,7 +833,7 @@ DefineIndex(Oid relationId,
814833
* A valid stmt->oldNode implies that we already have a built form of the
815834
* index. The caller should also decline any index build.
816835
*/
817-
Assert(!OidIsValid(stmt->oldNode)|| (skip_build&& !stmt->concurrent));
836+
Assert(!OidIsValid(stmt->oldNode)|| (skip_build&& !concurrent));
818837

819838
/*
820839
* Make the catalog entries for the index, including constraints. This
@@ -825,11 +844,11 @@ DefineIndex(Oid relationId,
825844
flags=constr_flags=0;
826845
if (stmt->isconstraint)
827846
flags |=INDEX_CREATE_ADD_CONSTRAINT;
828-
if (skip_build||stmt->concurrent||partitioned)
847+
if (skip_build||concurrent||partitioned)
829848
flags |=INDEX_CREATE_SKIP_BUILD;
830849
if (stmt->if_not_exists)
831850
flags |=INDEX_CREATE_IF_NOT_EXISTS;
832-
if (stmt->concurrent)
851+
if (concurrent)
833852
flags |=INDEX_CREATE_CONCURRENT;
834853
if (partitioned)
835854
flags |=INDEX_CREATE_PARTITIONED;
@@ -1107,7 +1126,7 @@ DefineIndex(Oid relationId,
11071126
returnaddress;
11081127
}
11091128

1110-
if (!stmt->concurrent)
1129+
if (!concurrent)
11111130
{
11121131
/* Close the heap and we're done, in the non-concurrent case */
11131132
heap_close(rel,NoLock);

‎src/backend/commands/tablecmds.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,11 @@ RemoveRelations(DropStmt *drop)
11061106
/* DROP CONCURRENTLY uses a weaker lock, and has some restrictions */
11071107
if (drop->concurrent)
11081108
{
1109-
flags |=PERFORM_DELETION_CONCURRENTLY;
1109+
/*
1110+
* Note that for temporary relations this lock may get upgraded
1111+
* later on, but as no other session can access a temporary
1112+
* relation, this is actually fine.
1113+
*/
11101114
lockmode=ShareUpdateExclusiveLock;
11111115
Assert(drop->removeType==OBJECT_INDEX);
11121116
if (list_length(drop->objects)!=1)
@@ -1197,6 +1201,18 @@ RemoveRelations(DropStmt *drop)
11971201
continue;
11981202
}
11991203

1204+
/*
1205+
* Decide if concurrent mode needs to be used here or not. The
1206+
* relation persistence cannot be known without its OID.
1207+
*/
1208+
if (drop->concurrent&&
1209+
get_rel_persistence(relOid)!=RELPERSISTENCE_TEMP)
1210+
{
1211+
Assert(list_length(drop->objects)==1&&
1212+
drop->removeType==OBJECT_INDEX);
1213+
flags |=PERFORM_DELETION_CONCURRENTLY;
1214+
}
1215+
12001216
/* OK, we're ready to delete this one */
12011217
obj.classId=RelationRelationId;
12021218
obj.objectId=relOid;

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

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

2574+
-- Temporary tables with concurrent builds and on-commit actions
2575+
-- CONCURRENTLY used with CREATE INDEX and DROP INDEX is ignored.
2576+
-- PRESERVE ROWS, the default.
2577+
CREATE TEMP TABLE concur_temp (f1 int, f2 text)
2578+
ON COMMIT PRESERVE ROWS;
2579+
INSERT INTO concur_temp VALUES (1, 'foo'), (2, 'bar');
2580+
CREATE INDEX CONCURRENTLY concur_temp_ind ON concur_temp(f1);
2581+
DROP INDEX CONCURRENTLY concur_temp_ind;
2582+
DROP TABLE concur_temp;
2583+
-- ON COMMIT DROP
2584+
BEGIN;
2585+
CREATE TEMP TABLE concur_temp (f1 int, f2 text)
2586+
ON COMMIT DROP;
2587+
INSERT INTO concur_temp VALUES (1, 'foo'), (2, 'bar');
2588+
-- Fails when running in a transaction.
2589+
CREATE INDEX CONCURRENTLY concur_temp_ind ON concur_temp(f1);
2590+
ERROR: CREATE INDEX CONCURRENTLY cannot run inside a transaction block
2591+
COMMIT;
2592+
-- ON COMMIT DELETE ROWS
2593+
CREATE TEMP TABLE concur_temp (f1 int, f2 text)
2594+
ON COMMIT DELETE ROWS;
2595+
INSERT INTO concur_temp VALUES (1, 'foo'), (2, 'bar');
2596+
CREATE INDEX CONCURRENTLY concur_temp_ind ON concur_temp(f1);
2597+
DROP INDEX CONCURRENTLY concur_temp_ind;
2598+
DROP TABLE concur_temp;
25742599
--
25752600
-- Try some concurrent index drops
25762601
--

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,31 @@ VACUUM FULL concur_heap;
825825
REINDEX TABLE concur_heap;
826826
\d concur_heap
827827

828+
-- Temporary tables with concurrent builds and on-commit actions
829+
-- CONCURRENTLY used with CREATE INDEX and DROP INDEX is ignored.
830+
-- PRESERVE ROWS, the default.
831+
CREATE TEMP TABLE concur_temp (f1int, f2text)
832+
ONCOMMIT PRESERVE ROWS;
833+
INSERT INTO concur_tempVALUES (1,'foo'), (2,'bar');
834+
CREATEINDEXCONCURRENTLY concur_temp_indON concur_temp(f1);
835+
DROPINDEX CONCURRENTLY concur_temp_ind;
836+
DROPTABLE concur_temp;
837+
-- ON COMMIT DROP
838+
BEGIN;
839+
CREATE TEMP TABLE concur_temp (f1int, f2text)
840+
ONCOMMIT DROP;
841+
INSERT INTO concur_tempVALUES (1,'foo'), (2,'bar');
842+
-- Fails when running in a transaction.
843+
CREATEINDEXCONCURRENTLY concur_temp_indON concur_temp(f1);
844+
COMMIT;
845+
-- ON COMMIT DELETE ROWS
846+
CREATE TEMP TABLE concur_temp (f1int, f2text)
847+
ONCOMMITDELETE ROWS;
848+
INSERT INTO concur_tempVALUES (1,'foo'), (2,'bar');
849+
CREATEINDEXCONCURRENTLY concur_temp_indON concur_temp(f1);
850+
DROPINDEX CONCURRENTLY concur_temp_ind;
851+
DROPTABLE concur_temp;
852+
828853
--
829854
-- Try some concurrent index drops
830855
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp