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

Commit212fab9

Browse files
committed
Relax transactional restrictions on ALTER TYPE ... ADD VALUE (redux).
Originally committed as15bc038 (plus some follow-ups), this wasreverted in28e0727 due to a problem discovered in parallelworkers. This new version corrects that problem by sending thelist of uncommitted enum values to parallel workers.Here follows the original commit message describing the change:To prevent possibly breaking indexes on enum columns, we must keepuncommitted enum values from getting stored in tables, unless wecan be sure that any such column is new in the current transaction.Formerly, we enforced this by disallowing ALTER TYPE ... ADD VALUEfrom being executed at all in a transaction block, unless the targetenum type had been created in the current transaction. This patchremoves that restriction, and instead insists that an uncommitted enumvalue can't be referenced unless it belongs to an enum type createdin the same transaction as the value. Per discussion, this should bea bit less onerous. It does require each function that could possiblyreturn a new enum value to SQL operations to check this restriction,but there aren't so many of those that this seems unmaintainable.Author: Andrew Dunstan and Tom Lane, with parallel query fix by Thomas MunroReviewed-by: Tom LaneDiscussion:https://postgr.es/m/CAEepm%3D0Ei7g6PaNTbcmAh9tCRahQrk%3Dr5ZWLD-jr7hXweYX3yg%40mail.gmail.comDiscussion:https://postgr.es/m/4075.1459088427%40sss.pgh.pa.us
1 parent7767aad commit212fab9

File tree

12 files changed

+367
-50
lines changed

12 files changed

+367
-50
lines changed

‎doc/src/sgml/ref/alter_type.sgml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,9 @@ ALTER TYPE <replaceable class="parameter">name</replaceable> RENAME VALUE <repla
290290
<title>Notes</title>
291291

292292
<para>
293-
<command>ALTER TYPE ... ADD VALUE</command> (the form that adds a new value to an
294-
enum type) cannot be executed inside a transaction block.
293+
If <command>ALTER TYPE ... ADD VALUE</command> (the form that adds a new
294+
value to an enum type) is executed inside a transaction block, the new
295+
value cannot be used until after the transaction has been committed.
295296
</para>
296297

297298
<para>

‎src/backend/access/transam/parallel.c

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include"access/session.h"
2020
#include"access/xact.h"
2121
#include"access/xlog.h"
22+
#include"catalog/pg_enum.h"
2223
#include"catalog/index.h"
2324
#include"catalog/namespace.h"
2425
#include"commands/async.h"
@@ -71,6 +72,7 @@
7172
#definePARALLEL_KEY_SESSION_DSMUINT64CONST(0xFFFFFFFFFFFF000A)
7273
#definePARALLEL_KEY_REINDEX_STATEUINT64CONST(0xFFFFFFFFFFFF000B)
7374
#definePARALLEL_KEY_RELMAPPER_STATEUINT64CONST(0xFFFFFFFFFFFF000C)
75+
#definePARALLEL_KEY_ENUMBLACKLISTUINT64CONST(0xFFFFFFFFFFFF000D)
7476

7577
/* Fixed-size parallel state. */
7678
typedefstructFixedParallelState
@@ -210,6 +212,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
210212
Sizetstatelen=0;
211213
Sizereindexlen=0;
212214
Sizerelmapperlen=0;
215+
Sizeenumblacklistlen=0;
213216
Sizesegsize=0;
214217
inti;
215218
FixedParallelState*fps;
@@ -263,8 +266,10 @@ InitializeParallelDSM(ParallelContext *pcxt)
263266
shm_toc_estimate_chunk(&pcxt->estimator,reindexlen);
264267
relmapperlen=EstimateRelationMapSpace();
265268
shm_toc_estimate_chunk(&pcxt->estimator,relmapperlen);
269+
enumblacklistlen=EstimateEnumBlacklistSpace();
270+
shm_toc_estimate_chunk(&pcxt->estimator,enumblacklistlen);
266271
/* If you add more chunks here, you probably need to add keys. */
267-
shm_toc_estimate_keys(&pcxt->estimator,9);
272+
shm_toc_estimate_keys(&pcxt->estimator,10);
268273

269274
/* Estimate space need for error queues. */
270275
StaticAssertStmt(BUFFERALIGN(PARALLEL_ERROR_QUEUE_SIZE)==
@@ -340,6 +345,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
340345
char*error_queue_space;
341346
char*session_dsm_handle_space;
342347
char*entrypointstate;
348+
char*enumblacklistspace;
343349
Sizelnamelen;
344350

345351
/* Serialize shared libraries we have loaded. */
@@ -389,6 +395,12 @@ InitializeParallelDSM(ParallelContext *pcxt)
389395
shm_toc_insert(pcxt->toc,PARALLEL_KEY_RELMAPPER_STATE,
390396
relmapperspace);
391397

398+
/* Serialize enum blacklist state. */
399+
enumblacklistspace=shm_toc_allocate(pcxt->toc,enumblacklistlen);
400+
SerializeEnumBlacklist(enumblacklistspace,enumblacklistlen);
401+
shm_toc_insert(pcxt->toc,PARALLEL_KEY_ENUMBLACKLIST,
402+
enumblacklistspace);
403+
392404
/* Allocate space for worker information. */
393405
pcxt->worker=palloc0(sizeof(ParallelWorkerInfo)*pcxt->nworkers);
394406

@@ -1222,6 +1234,7 @@ ParallelWorkerMain(Datum main_arg)
12221234
char*tstatespace;
12231235
char*reindexspace;
12241236
char*relmapperspace;
1237+
char*enumblacklistspace;
12251238
StringInfoDatamsgbuf;
12261239
char*session_dsm_handle_space;
12271240

@@ -1408,6 +1421,11 @@ ParallelWorkerMain(Datum main_arg)
14081421
relmapperspace=shm_toc_lookup(toc,PARALLEL_KEY_RELMAPPER_STATE, false);
14091422
RestoreRelationMap(relmapperspace);
14101423

1424+
/* Restore enum blacklist. */
1425+
enumblacklistspace=shm_toc_lookup(toc,PARALLEL_KEY_ENUMBLACKLIST,
1426+
false);
1427+
RestoreEnumBlacklist(enumblacklistspace);
1428+
14111429
/*
14121430
* We've initialized all of our state now; nothing should change
14131431
* hereafter.

‎src/backend/access/transam/xact.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include"access/xloginsert.h"
3232
#include"access/xlogutils.h"
3333
#include"catalog/namespace.h"
34+
#include"catalog/pg_enum.h"
3435
#include"catalog/storage.h"
3536
#include"commands/async.h"
3637
#include"commands/tablecmds.h"
@@ -2135,6 +2136,7 @@ CommitTransaction(void)
21352136
AtCommit_Notify();
21362137
AtEOXact_GUC(true,1);
21372138
AtEOXact_SPI(true);
2139+
AtEOXact_Enum();
21382140
AtEOXact_on_commit_actions(true);
21392141
AtEOXact_Namespace(true,is_parallel_worker);
21402142
AtEOXact_SMgr();
@@ -2413,6 +2415,7 @@ PrepareTransaction(void)
24132415
/* PREPARE acts the same as COMMIT as far as GUC is concerned */
24142416
AtEOXact_GUC(true,1);
24152417
AtEOXact_SPI(true);
2418+
AtEOXact_Enum();
24162419
AtEOXact_on_commit_actions(true);
24172420
AtEOXact_Namespace(true, false);
24182421
AtEOXact_SMgr();
@@ -2615,6 +2618,7 @@ AbortTransaction(void)
26152618

26162619
AtEOXact_GUC(false,1);
26172620
AtEOXact_SPI(false);
2621+
AtEOXact_Enum();
26182622
AtEOXact_on_commit_actions(false);
26192623
AtEOXact_Namespace(false,is_parallel_worker);
26202624
AtEOXact_SMgr();

‎src/backend/catalog/pg_enum.c

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,26 @@
2828
#include"utils/builtins.h"
2929
#include"utils/catcache.h"
3030
#include"utils/fmgroids.h"
31+
#include"utils/hsearch.h"
32+
#include"utils/memutils.h"
3133
#include"utils/syscache.h"
3234
#include"utils/tqual.h"
3335

3436

3537
/* Potentially set by pg_upgrade_support functions */
3638
Oidbinary_upgrade_next_pg_enum_oid=InvalidOid;
3739

40+
/*
41+
* Hash table of enum value OIDs created during the current transaction by
42+
* AddEnumLabel. We disallow using these values until the transaction is
43+
* committed; otherwise, they might get into indexes where we can't clean
44+
* them up, and then if the transaction rolls back we have a broken index.
45+
* (See comments for check_safe_enum_use() in enum.c.) Values created by
46+
* EnumValuesCreate are *not* blacklisted; we assume those are created during
47+
* CREATE TYPE, so they can't go away unless the enum type itself does.
48+
*/
49+
staticHTAB*enum_blacklist=NULL;
50+
3851
staticvoidRenumberEnumType(Relationpg_enum,HeapTuple*existing,intnelems);
3952
staticintsort_order_cmp(constvoid*p1,constvoid*p2);
4053

@@ -168,6 +181,23 @@ EnumValuesDelete(Oid enumTypeOid)
168181
heap_close(pg_enum,RowExclusiveLock);
169182
}
170183

184+
/*
185+
* Initialize the enum blacklist for this transaction.
186+
*/
187+
staticvoid
188+
init_enum_blacklist(void)
189+
{
190+
HASHCTLhash_ctl;
191+
192+
memset(&hash_ctl,0,sizeof(hash_ctl));
193+
hash_ctl.keysize=sizeof(Oid);
194+
hash_ctl.entrysize=sizeof(Oid);
195+
hash_ctl.hcxt=TopTransactionContext;
196+
enum_blacklist=hash_create("Enum value blacklist",
197+
32,
198+
&hash_ctl,
199+
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
200+
}
171201

172202
/*
173203
* AddEnumLabel
@@ -460,6 +490,13 @@ AddEnumLabel(Oid enumTypeOid,
460490
heap_freetuple(enum_tup);
461491

462492
heap_close(pg_enum,RowExclusiveLock);
493+
494+
/* Set up the blacklist hash if not already done in this transaction */
495+
if (enum_blacklist==NULL)
496+
init_enum_blacklist();
497+
498+
/* Add the new value to the blacklist */
499+
(void)hash_search(enum_blacklist,&newOid,HASH_ENTER,NULL);
463500
}
464501

465502

@@ -547,6 +584,39 @@ RenameEnumLabel(Oid enumTypeOid,
547584
}
548585

549586

587+
/*
588+
* Test if the given enum value is on the blacklist
589+
*/
590+
bool
591+
EnumBlacklisted(Oidenum_id)
592+
{
593+
boolfound;
594+
595+
/* If we've made no blacklist table, all values are safe */
596+
if (enum_blacklist==NULL)
597+
return false;
598+
599+
/* Else, is it in the table? */
600+
(void)hash_search(enum_blacklist,&enum_id,HASH_FIND,&found);
601+
returnfound;
602+
}
603+
604+
605+
/*
606+
* Clean up enum stuff after end of top-level transaction.
607+
*/
608+
void
609+
AtEOXact_Enum(void)
610+
{
611+
/*
612+
* Reset the blacklist table, as all our enum values are now committed.
613+
* The memory will go away automatically when TopTransactionContext is
614+
* freed; it's sufficient to clear our pointer.
615+
*/
616+
enum_blacklist=NULL;
617+
}
618+
619+
550620
/*
551621
* RenumberEnumType
552622
*Renumber existing enum elements to have sort positions 1..n.
@@ -620,3 +690,72 @@ sort_order_cmp(const void *p1, const void *p2)
620690
else
621691
return0;
622692
}
693+
694+
Size
695+
EstimateEnumBlacklistSpace(void)
696+
{
697+
size_tentries;
698+
699+
if (enum_blacklist)
700+
entries=hash_get_num_entries(enum_blacklist);
701+
else
702+
entries=0;
703+
704+
/* Add one for the terminator. */
705+
returnsizeof(Oid)* (entries+1);
706+
}
707+
708+
void
709+
SerializeEnumBlacklist(void*space,Sizesize)
710+
{
711+
Oid*serialized= (Oid*)space;
712+
713+
/*
714+
* Make sure the hash table hasn't changed in size since the caller
715+
* reserved the space.
716+
*/
717+
Assert(size==EstimateEnumBlacklistSpace());
718+
719+
/* Write out all the values from the hash table, if there is one. */
720+
if (enum_blacklist)
721+
{
722+
HASH_SEQ_STATUSstatus;
723+
Oid*value;
724+
725+
hash_seq_init(&status,enum_blacklist);
726+
while ((value= (Oid*)hash_seq_search(&status)))
727+
*serialized++=*value;
728+
}
729+
730+
/* Write out the terminator. */
731+
*serialized=InvalidOid;
732+
733+
/*
734+
* Make sure the amount of space we actually used matches what was
735+
* estimated.
736+
*/
737+
Assert((char*) (serialized+1)== ((char*)space)+size);
738+
}
739+
740+
void
741+
RestoreEnumBlacklist(void*space)
742+
{
743+
Oid*serialized= (Oid*)space;
744+
745+
Assert(!enum_blacklist);
746+
747+
/*
748+
* As a special case, if the list is empty then don't even bother to
749+
* create the hash table. This is the usual case, since enum alteration
750+
* is expected to be rare.
751+
*/
752+
if (!OidIsValid(*serialized))
753+
return;
754+
755+
/* Read all the values into a new hash table. */
756+
init_enum_blacklist();
757+
do
758+
{
759+
hash_search(enum_blacklist,serialized++,HASH_ENTER,NULL);
760+
}while (OidIsValid(*serialized));
761+
}

‎src/backend/commands/typecmds.c

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,10 +1270,10 @@ DefineEnum(CreateEnumStmt *stmt)
12701270

12711271
/*
12721272
* AlterEnum
1273-
*ALTER TYPE on an enum.
1273+
*Adds a new label to an existing enum.
12741274
*/
12751275
ObjectAddress
1276-
AlterEnum(AlterEnumStmt*stmt,boolisTopLevel)
1276+
AlterEnum(AlterEnumStmt*stmt)
12771277
{
12781278
Oidenum_type_oid;
12791279
TypeName*typename;
@@ -1291,6 +1291,8 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
12911291
/* Check it's an enum and check user has permission to ALTER the enum */
12921292
checkEnumOwner(tup);
12931293

1294+
ReleaseSysCache(tup);
1295+
12941296
if (stmt->oldVal)
12951297
{
12961298
/* Rename an existing label */
@@ -1299,27 +1301,6 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
12991301
else
13001302
{
13011303
/* Add a new label */
1302-
1303-
/*
1304-
* Ordinarily we disallow adding values within transaction blocks,
1305-
* because we can't cope with enum OID values getting into indexes and
1306-
* then having their defining pg_enum entries go away. However, it's
1307-
* okay if the enum type was created in the current transaction, since
1308-
* then there can be no such indexes that wouldn't themselves go away
1309-
* on rollback. (We support this case because pg_dump
1310-
* --binary-upgrade needs it.) We test this by seeing if the pg_type
1311-
* row has xmin == current XID and is not HEAP_UPDATED. If it is
1312-
* HEAP_UPDATED, we can't be sure whether the type was created or only
1313-
* modified in this xact. So we are disallowing some cases that could
1314-
* theoretically be safe; but fortunately pg_dump only needs the
1315-
* simplest case.
1316-
*/
1317-
if (HeapTupleHeaderGetXmin(tup->t_data)==GetCurrentTransactionId()&&
1318-
!(tup->t_data->t_infomask&HEAP_UPDATED))
1319-
/* safe to do inside transaction block */ ;
1320-
else
1321-
PreventInTransactionBlock(isTopLevel,"ALTER TYPE ... ADD");
1322-
13231304
AddEnumLabel(enum_type_oid,stmt->newVal,
13241305
stmt->newValNeighbor,stmt->newValIsAfter,
13251306
stmt->skipIfNewValExists);
@@ -1329,8 +1310,6 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
13291310

13301311
ObjectAddressSet(address,TypeRelationId,enum_type_oid);
13311312

1332-
ReleaseSysCache(tup);
1333-
13341313
returnaddress;
13351314
}
13361315

‎src/backend/tcop/utility.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ ProcessUtilitySlow(ParseState *pstate,
14391439
break;
14401440

14411441
caseT_AlterEnumStmt:/* ALTER TYPE (enum) */
1442-
address=AlterEnum((AlterEnumStmt*)parsetree,isTopLevel);
1442+
address=AlterEnum((AlterEnumStmt*)parsetree);
14431443
break;
14441444

14451445
caseT_ViewStmt:/* CREATE VIEW */

0 commit comments

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