aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/backend/catalog/toasting.c51
-rw-r--r--src/backend/commands/cluster.c14
-rw-r--r--src/backend/commands/createas.c4
-rw-r--r--src/backend/commands/tablecmds.c174
-rw-r--r--src/backend/tcop/utility.c4
-rw-r--r--src/backend/utils/adt/ruleutils.c33
-rw-r--r--src/backend/utils/cache/relcache.c74
-rw-r--r--src/include/catalog/toasting.h8
-rw-r--r--src/test/isolation/isolation_schedule1
-rw-r--r--src/test/regress/expected/alter_table.out81
-rw-r--r--src/test/regress/sql/alter_table.sql29
11 files changed, 361 insertions, 112 deletions
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index f044280dba3..5275e4bfdb3 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -27,6 +27,7 @@
#include "catalog/toasting.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
+#include "storage/lock.h"
#include "utils/builtins.h"
#include "utils/rel.h"
#include "utils/syscache.h"
@@ -34,13 +35,15 @@
/* Potentially set by contrib/pg_upgrade_support functions */
Oid binary_upgrade_next_toast_pg_type_oid = InvalidOid;
+static void CheckAndCreateToastTable(Oid relOid, Datum reloptions,
+ LOCKMODE lockmode, bool check);
static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
- Datum reloptions);
+ Datum reloptions, LOCKMODE lockmode, bool check);
static bool needs_toast_table(Relation rel);
/*
- * AlterTableCreateToastTable
+ * CreateToastTable variants
* If the table needs a toast table, and doesn't already have one,
* then create a toast table for it.
*
@@ -52,21 +55,32 @@ static bool needs_toast_table(Relation rel);
* to end with CommandCounterIncrement if it makes any changes.
*/
void
-AlterTableCreateToastTable(Oid relOid, Datum reloptions)
+AlterTableCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode)
+{
+ CheckAndCreateToastTable(relOid, reloptions, lockmode, true);
+}
+
+void
+NewHeapCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode)
+{
+ CheckAndCreateToastTable(relOid, reloptions, lockmode, false);
+}
+
+void
+NewRelationCreateToastTable(Oid relOid, Datum reloptions)
+{
+ CheckAndCreateToastTable(relOid, reloptions, AccessExclusiveLock, false);
+}
+
+static void
+CheckAndCreateToastTable(Oid relOid, Datum reloptions, LOCKMODE lockmode, bool check)
{
Relation rel;
- /*
- * Grab an exclusive lock on the target table, since we'll update its
- * pg_class tuple. This is redundant for all present uses, since caller
- * will have such a lock already. But the lock is needed to ensure that
- * concurrent readers of the pg_class tuple won't have visibility issues,
- * so let's be safe.
- */
- rel = heap_open(relOid, AccessExclusiveLock);
+ rel = heap_open(relOid, lockmode);
/* create_toast_table does all the work */
- (void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions);
+ (void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions, lockmode, check);
heap_close(rel, NoLock);
}
@@ -91,7 +105,8 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
relName)));
/* create_toast_table does all the work */
- if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0))
+ if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0,
+ AccessExclusiveLock, false))
elog(ERROR, "\"%s\" does not require a toast table",
relName);
@@ -107,7 +122,8 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
* bootstrap they can be nonzero to specify hand-assigned OIDs
*/
static bool
-create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptions)
+create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
+ Datum reloptions, LOCKMODE lockmode, bool check)
{
Oid relOid = RelationGetRelid(rel);
HeapTuple reltup;
@@ -161,6 +177,13 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
return false;
/*
+ * If requested check lockmode is sufficient. This is a cross check
+ * in case of errors or conflicting decisions in earlier code.
+ */
+ if (check && lockmode != AccessExclusiveLock)
+ elog(ERROR, "AccessExclusiveLock required to add toast table.");
+
+ /*
* Create the toast table and its index
*/
snprintf(toast_relname, sizeof(toast_relname),
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 620f071aafb..4ac1e0b864f 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -408,10 +408,10 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
/*
* Verify that the specified heap and index are valid to cluster on
*
- * Side effect: obtains exclusive lock on the index. The caller should
- * already have exclusive lock on the table, so the index lock is likely
- * redundant, but it seems best to grab it anyway to ensure the index
- * definition can't change under us.
+ * Side effect: obtains lock on the index. The caller may
+ * in some cases already have AccessExclusiveLock on the table, but
+ * not in all cases so we can't rely on the table-level lock for
+ * protection here.
*/
void
check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMODE lockmode)
@@ -696,10 +696,10 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
*
* If the relation doesn't have a TOAST table already, we can't need one
* for the new relation. The other way around is possible though: if some
- * wide columns have been dropped, AlterTableCreateToastTable can decide
+ * wide columns have been dropped, NewHeapCreateToastTable can decide
* that no TOAST table is needed for the new table.
*
- * Note that AlterTableCreateToastTable ends with CommandCounterIncrement,
+ * Note that NewHeapCreateToastTable ends with CommandCounterIncrement,
* so that the TOAST table will be visible for insertion.
*/
toastid = OldHeap->rd_rel->reltoastrelid;
@@ -714,7 +714,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
if (isNull)
reloptions = (Datum) 0;
- AlterTableCreateToastTable(OIDNewHeap, reloptions);
+ NewHeapCreateToastTable(OIDNewHeap, reloptions, lockmode);
ReleaseSysCache(tuple);
}
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 73e6e209855..e434d38702e 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -359,7 +359,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
/*
* If necessary, create a TOAST table for the target table. Note that
- * AlterTableCreateToastTable ends with CommandCounterIncrement(), so that
+ * NewRelationCreateToastTable ends with CommandCounterIncrement(), so that
* the TOAST table will be visible for insertion.
*/
CommandCounterIncrement();
@@ -373,7 +373,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
- AlterTableCreateToastTable(intoRelationId, toast_options);
+ NewRelationCreateToastTable(intoRelationId, toast_options);
/* Create the "view" part of a materialized view. */
if (is_matview)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7f3f730a87b..8f9e5e56079 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2685,10 +2685,8 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode)
* the whole operation; we don't have to do anything special to clean up.
*
* The caller must lock the relation, with an appropriate lock level
- * for the subcommands requested. Any subcommand that needs to rewrite
- * tuples in the table forces the whole command to be executed with
- * AccessExclusiveLock (actually, that is currently required always, but
- * we hope to relax it at some point). We pass the lock level down
+ * for the subcommands requested, using AlterTableGetLockLevel(stmt->cmds)
+ * or higher. We pass the lock level down
* so that we can apply it recursively to inherited tables. Note that the
* lock level we want as we recurse might well be higher than required for
* that specific subcommand. So we pass down the overall lock requirement,
@@ -2750,35 +2748,24 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse)
* ALTER TABLE RENAME which is treated as a different statement type T_RenameStmt
* and does not travel through this section of code and cannot be combined with
* any of the subcommands given here.
+ *
+ * Note that Hot Standby only knows about AccessExclusiveLocks on the master
+ * so any changes that might affect SELECTs running on standbys need to use
+ * AccessExclusiveLocks even if you think a lesser lock would do, unless you
+ * have a solution for that also.
+ *
+ * Also note that pg_dump uses only an AccessShareLock, meaning that anything
+ * that takes a lock less than AccessExclusiveLock can change object definitions
+ * while pg_dump is running. Be careful to check that the appropriate data is
+ * derived by pg_dump using an MVCC snapshot, rather than syscache lookups,
+ * otherwise we might end up with an inconsistent dump that can't restore.
*/
LOCKMODE
AlterTableGetLockLevel(List *cmds)
{
/*
- * Late in 9.1 dev cycle a number of issues were uncovered with access to
- * catalog relations, leading to the decision to re-enforce all DDL at
- * AccessExclusiveLock level by default.
- *
- * The issues are that there is a pervasive assumption in the code that
- * the catalogs will not be read unless an AccessExclusiveLock is held. If
- * that rule is relaxed, we must protect against a number of potential
- * effects - infrequent, but proven possible with test cases where
- * multiple DDL operations occur in a stream against frequently accessed
- * tables.
- *
- * 1. Catalog tables were read using SnapshotNow, which has a race bug that
- * allows a scan to return no valid rows even when one is present in the
- * case of a commit of a concurrent update of the catalog table.
- * SnapshotNow also ignores transactions in progress, so takes the latest
- * committed version without waiting for the latest changes.
- *
- * 2. Relcache needs to be internally consistent, so unless we lock the
- * definition during reads we have no way to guarantee that.
- *
- * 3. Catcache access isn't coordinated at all so refreshes can occur at
- * any time.
+ * This only works if we read catalog tables using MVCC snapshots.
*/
-#ifdef REDUCED_ALTER_TABLE_LOCK_LEVELS
ListCell *lcmd;
LOCKMODE lockmode = ShareUpdateExclusiveLock;
@@ -2790,28 +2777,58 @@ AlterTableGetLockLevel(List *cmds)
switch (cmd->subtype)
{
/*
- * Need AccessExclusiveLock for these subcommands because they
- * affect or potentially affect both read and write
- * operations.
- *
- * New subcommand types should be added here by default.
+ * These subcommands rewrite the heap, so require full locks.
*/
case AT_AddColumn: /* may rewrite heap, in some cases and visible
* to SELECT */
- case AT_DropColumn: /* change visible to SELECT */
- case AT_AddColumnToView: /* CREATE VIEW */
+ case AT_SetTableSpace: /* must rewrite heap */
case AT_AlterColumnType: /* must rewrite heap */
+ case AT_AddOids: /* must rewrite heap */
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
+ /*
+ * These subcommands may require addition of toast tables. If we
+ * add a toast table to a table currently being scanned, we
+ * might miss data added to the new toast table by concurrent
+ * insert transactions.
+ */
+ case AT_SetStorage: /* may add toast tables, see ATRewriteCatalogs() */
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
+ /*
+ * Removing constraints can affect SELECTs that have been
+ * optimised assuming the constraint holds true.
+ */
case AT_DropConstraint: /* as DROP INDEX */
- case AT_AddOids: /* must rewrite heap */
- case AT_DropOids: /* calls AT_DropColumn */
+ case AT_DropNotNull: /* may change some SQL plans */
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
+ /*
+ * Subcommands that may be visible to concurrent SELECTs
+ */
+ case AT_DropColumn: /* change visible to SELECT */
+ case AT_AddColumnToView: /* CREATE VIEW */
+ case AT_DropOids: /* calls AT_DropColumn */
case AT_EnableAlwaysRule: /* may change SELECT rules */
case AT_EnableReplicaRule: /* may change SELECT rules */
- case AT_EnableRule: /* may change SELECT rules */
+ case AT_EnableRule: /* may change SELECT rules */
case AT_DisableRule: /* may change SELECT rules */
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
+ /*
+ * Changing owner may remove implicit SELECT privileges
+ */
case AT_ChangeOwner: /* change visible to SELECT */
- case AT_SetTableSpace: /* must rewrite heap */
- case AT_DropNotNull: /* may change some SQL plans */
- case AT_SetNotNull:
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
+ /*
+ * Changing foreign table options may affect optimisation.
+ */
case AT_GenericOptions:
case AT_AlterColumnGenericOptions:
cmd_lockmode = AccessExclusiveLock;
@@ -2819,6 +2836,7 @@ AlterTableGetLockLevel(List *cmds)
/*
* These subcommands affect write operations only.
+ * XXX Theoretically, these could be ShareRowExclusiveLock.
*/
case AT_ColumnDefault:
case AT_ProcessedConstraint: /* becomes AT_AddConstraint */
@@ -2832,10 +2850,12 @@ AlterTableGetLockLevel(List *cmds)
case AT_DisableTrig:
case AT_DisableTrigAll:
case AT_DisableTrigUser:
+ case AT_AlterConstraint:
case AT_AddIndex: /* from ADD CONSTRAINT */
case AT_AddIndexConstraint:
case AT_ReplicaIdentity:
- cmd_lockmode = ShareRowExclusiveLock;
+ case AT_SetNotNull:
+ cmd_lockmode = AccessExclusiveLock;
break;
case AT_AddConstraint:
@@ -2854,8 +2874,10 @@ AlterTableGetLockLevel(List *cmds)
* could reduce the lock strength to ShareLock if
* we can work out how to allow concurrent catalog
* updates.
+ * XXX Might be set down to ShareRowExclusiveLock
+ * but requires further analysis.
*/
- cmd_lockmode = ShareRowExclusiveLock;
+ cmd_lockmode = AccessExclusiveLock;
break;
case CONSTR_FOREIGN:
@@ -2863,12 +2885,15 @@ AlterTableGetLockLevel(List *cmds)
* We add triggers to both tables when we add a
* Foreign Key, so the lock level must be at least
* as strong as CREATE TRIGGER.
+ * XXX Might be set down to ShareRowExclusiveLock
+ * though trigger info is accessed by
+ * pg_get_triggerdef
*/
- cmd_lockmode = ShareRowExclusiveLock;
+ cmd_lockmode = AccessExclusiveLock;
break;
default:
- cmd_lockmode = ShareRowExclusiveLock;
+ cmd_lockmode = AccessExclusiveLock;
}
}
break;
@@ -2879,22 +2904,31 @@ AlterTableGetLockLevel(List *cmds)
* behaviour, while queries started after we commit will see
* new behaviour. No need to prevent reads or writes to the
* subtable while we hook it up though.
+ * Changing the TupDesc may be a problem, so keep highest lock.
*/
case AT_AddInherit:
case AT_DropInherit:
- cmd_lockmode = ShareUpdateExclusiveLock;
+ cmd_lockmode = AccessExclusiveLock;
break;
/*
* These subcommands affect implicit row type conversion. They
- * have affects similar to CREATE/DROP CAST on queries. We
+ * have affects similar to CREATE/DROP CAST on queries.
* don't provide for invalidating parse trees as a result of
- * such changes. Do avoid concurrent pg_class updates,
- * though.
+ * such changes, so we keep these at AccessExclusiveLock.
*/
case AT_AddOf:
case AT_DropOf:
- cmd_lockmode = ShareUpdateExclusiveLock;
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
+ /*
+ * Only used by CREATE OR REPLACE VIEW which must conflict
+ * with an SELECTs currently using the view.
+ */
+ case AT_ReplaceRelOptions:
+ cmd_lockmode = AccessExclusiveLock;
+ break;
/*
* These subcommands affect general strategies for performance
@@ -2906,20 +2940,33 @@ AlterTableGetLockLevel(List *cmds)
* applies: we don't currently allow concurrent catalog
* updates.
*/
- case AT_SetStatistics:
- case AT_ClusterOn:
- case AT_DropCluster:
- case AT_SetRelOptions:
- case AT_ResetRelOptions:
- case AT_ReplaceRelOptions:
- case AT_SetOptions:
- case AT_ResetOptions:
- case AT_SetStorage:
- case AT_AlterConstraint:
- case AT_ValidateConstraint:
+ case AT_SetStatistics: /* Uses MVCC in getTableAttrs() */
+ case AT_ClusterOn: /* Uses MVCC in getIndexes() */
+ case AT_DropCluster: /* Uses MVCC in getIndexes() */
+ case AT_SetOptions: /* Uses MVCC in getTableAttrs() */
+ case AT_ResetOptions: /* Uses MVCC in getTableAttrs() */
cmd_lockmode = ShareUpdateExclusiveLock;
break;
+ case AT_ValidateConstraint: /* Uses MVCC in getConstraints() */
+ cmd_lockmode = ShareUpdateExclusiveLock;
+ break;
+
+ /*
+ * Rel options are more complex than first appears. Options
+ * are set here for tables, views and indexes; for historical
+ * reasons these can all be used with ALTER TABLE, so we
+ * can't decide between them using the basic grammar.
+ *
+ * XXX Look in detail at each option to determine lock level,
+ * e.g.
+ * cmd_lockmode = GetRelOptionsLockLevel((List *) cmd->def);
+ */
+ case AT_SetRelOptions: /* Uses MVCC in getIndexes() and getTables() */
+ case AT_ResetRelOptions: /* Uses MVCC in getIndexes() and getTables() */
+ cmd_lockmode = AccessExclusiveLock;
+ break;
+
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
(int) cmd->subtype);
@@ -2932,9 +2979,6 @@ AlterTableGetLockLevel(List *cmds)
if (cmd_lockmode > lockmode)
lockmode = cmd_lockmode;
}
-#else
- LOCKMODE lockmode = AccessExclusiveLock;
-#endif
return lockmode;
}
@@ -3272,7 +3316,7 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode)
if (tab->relkind == RELKIND_RELATION ||
tab->relkind == RELKIND_MATVIEW)
- AlterTableCreateToastTable(tab->relid, (Datum) 0);
+ AlterTableCreateToastTable(tab->relid, (Datum) 0, lockmode);
}
}
@@ -3586,7 +3630,7 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
/* Create transient table that will receive the modified data */
OIDNewHeap = make_new_heap(tab->relid, NewTableSpace, false,
- AccessExclusiveLock);
+ lockmode);
/*
* Copy the heap data into the new table with the desired
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index d1621add7d8..1846570a3e3 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -908,7 +908,7 @@ ProcessUtilitySlow(Node *parsetree,
InvalidOid);
/*
- * Let AlterTableCreateToastTable decide if this
+ * Let NewRelationCreateToastTable decide if this
* one needs a secondary relation too.
*/
CommandCounterIncrement();
@@ -927,7 +927,7 @@ ProcessUtilitySlow(Node *parsetree,
toast_options,
true);
- AlterTableCreateToastTable(relOid, toast_options);
+ NewRelationCreateToastTable(relOid, toast_options);
}
else if (IsA(stmt, CreateForeignTableStmt))
{
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b1bac866aa1..ea7b8c59429 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -54,6 +54,7 @@
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
+#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
#include "utils/typcache.h"
@@ -1284,6 +1285,9 @@ pg_get_constraintdef_string(Oid constraintId)
return pg_get_constraintdef_worker(constraintId, true, 0);
}
+/*
+ * As of 9.4, we now use an MVCC snapshot for this.
+ */
static char *
pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
int prettyFlags)
@@ -1291,10 +1295,34 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
HeapTuple tup;
Form_pg_constraint conForm;
StringInfoData buf;
+ SysScanDesc scandesc;
+ ScanKeyData scankey[1];
+ Snapshot snapshot = RegisterSnapshot(GetTransactionSnapshot());
+ Relation relation = heap_open(ConstraintRelationId, AccessShareLock);
+
+ ScanKeyInit(&scankey[0],
+ ObjectIdAttributeNumber,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(constraintId));
+
+ scandesc = systable_beginscan(relation,
+ ConstraintOidIndexId,
+ true,
+ snapshot,
+ 1,
+ scankey);
+
+ /*
+ * We later use the tuple with SysCacheGetAttr() as if we
+ * had obtained it via SearchSysCache, which works fine.
+ */
+ tup = systable_getnext(scandesc);
+
+ UnregisterSnapshot(snapshot);
- tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(constraintId));
if (!HeapTupleIsValid(tup)) /* should not happen */
elog(ERROR, "cache lookup failed for constraint %u", constraintId);
+
conForm = (Form_pg_constraint) GETSTRUCT(tup);
initStringInfo(&buf);
@@ -1575,7 +1603,8 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
appendStringInfoString(&buf, " NOT VALID");
/* Cleanup */
- ReleaseSysCache(tup);
+ systable_endscan(scandesc);
+ heap_close(relation, AccessShareLock);
return buf.data;
}
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 32313244adb..c8cea028d4e 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -162,6 +162,14 @@ static bool eoxact_list_overflowed = false;
eoxact_list_overflowed = true; \
} while (0)
+/*
+ * EOXactTupleDescArray stores TupleDescs that (might) need AtEOXact
+ * cleanup work. The array expands as needed; there is no hashtable because
+ * we don't need to access individual items except at EOXact.
+ */
+static TupleDesc *EOXactTupleDescArray;
+static int NextEOXactTupleDescNum = 0;
+static int EOXactTupleDescArrayLen = 0;
/*
* macros to manipulate the lookup hashtables
@@ -220,11 +228,12 @@ static HTAB *OpClassCache = NULL;
/* non-export function prototypes */
-static void RelationDestroyRelation(Relation relation);
+static void RelationDestroyRelation(Relation relation, bool remember_tupdesc);
static void RelationClearRelation(Relation relation, bool rebuild);
static void RelationReloadIndexInfo(Relation relation);
static void RelationFlushRelation(Relation relation);
+static void RememberToFreeTupleDescAtEOX(TupleDesc td);
static void AtEOXact_cleanup(Relation relation, bool isCommit);
static void AtEOSubXact_cleanup(Relation relation, bool isCommit,
SubTransactionId mySubid, SubTransactionId parentSubid);
@@ -1858,7 +1867,7 @@ RelationReloadIndexInfo(Relation relation)
* Caller must already have unhooked the entry from the hash table.
*/
static void
-RelationDestroyRelation(Relation relation)
+RelationDestroyRelation(Relation relation, bool remember_tupdesc)
{
Assert(RelationHasReferenceCountZero(relation));
@@ -1878,7 +1887,20 @@ RelationDestroyRelation(Relation relation)
/* can't use DecrTupleDescRefCount here */
Assert(relation->rd_att->tdrefcount > 0);
if (--relation->rd_att->tdrefcount == 0)
- FreeTupleDesc(relation->rd_att);
+ {
+ /*
+ * If we Rebuilt a relcache entry during a transaction then its
+ * possible we did that because the TupDesc changed as the result
+ * of an ALTER TABLE that ran at less than AccessExclusiveLock.
+ * It's possible someone copied that TupDesc, in which case the
+ * copy would point to free'd memory. So if we rebuild an entry
+ * we keep the TupDesc around until end of transaction, to be safe.
+ */
+ if (remember_tupdesc)
+ RememberToFreeTupleDescAtEOX(relation->rd_att);
+ else
+ FreeTupleDesc(relation->rd_att);
+ }
list_free(relation->rd_indexlist);
bms_free(relation->rd_indexattr);
FreeTriggerDesc(relation->trigdesc);
@@ -1992,7 +2014,7 @@ RelationClearRelation(Relation relation, bool rebuild)
RelationCacheDelete(relation);
/* And release storage */
- RelationDestroyRelation(relation);
+ RelationDestroyRelation(relation, false);
}
else if (!IsTransactionState())
{
@@ -2059,7 +2081,7 @@ RelationClearRelation(Relation relation, bool rebuild)
{
/* Should only get here if relation was deleted */
RelationCacheDelete(relation);
- RelationDestroyRelation(relation);
+ RelationDestroyRelation(relation, false);
elog(ERROR, "relation %u deleted while still in use", save_relid);
}
@@ -2121,7 +2143,7 @@ RelationClearRelation(Relation relation, bool rebuild)
#undef SWAPFIELD
/* And now we can throw away the temporary entry */
- RelationDestroyRelation(newrel);
+ RelationDestroyRelation(newrel, !keep_tupdesc);
}
}
@@ -2359,6 +2381,33 @@ RelationCloseSmgrByOid(Oid relationId)
RelationCloseSmgr(relation);
}
+void
+RememberToFreeTupleDescAtEOX(TupleDesc td)
+{
+ if (EOXactTupleDescArray == NULL)
+ {
+ MemoryContext oldcxt;
+ oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+
+ EOXactTupleDescArray = (TupleDesc *) palloc(16 * sizeof(TupleDesc));
+ EOXactTupleDescArrayLen = 16;
+ NextEOXactTupleDescNum = 0;
+ MemoryContextSwitchTo(oldcxt);
+ }
+ else if (NextEOXactTupleDescNum >= EOXactTupleDescArrayLen)
+ {
+ int32 newlen = EOXactTupleDescArrayLen * 2;
+
+ Assert(EOXactTupleDescArrayLen > 0);
+
+ EOXactTupleDescArray = (TupleDesc *) repalloc(EOXactTupleDescArray,
+ newlen * sizeof(TupleDesc));
+ EOXactTupleDescArrayLen = newlen;
+ }
+
+ EOXactTupleDescArray[NextEOXactTupleDescNum++] = td;
+}
+
/*
* AtEOXact_RelationCache
*
@@ -2414,9 +2463,20 @@ AtEOXact_RelationCache(bool isCommit)
}
}
- /* Now we're out of the transaction and can clear the list */
+ if (EOXactTupleDescArrayLen > 0)
+ {
+ Assert(EOXactTupleDescArray != NULL);
+ for (i = 0; i < NextEOXactTupleDescNum; i++)
+ FreeTupleDesc(EOXactTupleDescArray[i]);
+ pfree(EOXactTupleDescArray);
+ EOXactTupleDescArray = NULL;
+ }
+
+ /* Now we're out of the transaction and can clear the lists */
eoxact_list_len = 0;
eoxact_list_overflowed = false;
+ NextEOXactTupleDescNum = 0;
+ EOXactTupleDescArrayLen = 0;
}
/*
diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h
index ca047f06185..0947760118f 100644
--- a/src/include/catalog/toasting.h
+++ b/src/include/catalog/toasting.h
@@ -14,10 +14,16 @@
#ifndef TOASTING_H
#define TOASTING_H
+#include "storage/lock.h"
+
/*
* toasting.c prototypes
*/
-extern void AlterTableCreateToastTable(Oid relOid, Datum reloptions);
+extern void NewRelationCreateToastTable(Oid relOid, Datum reloptions);
+extern void NewHeapCreateToastTable(Oid relOid, Datum reloptions,
+ LOCKMODE lockmode);
+extern void AlterTableCreateToastTable(Oid relOid, Datum reloptions,
+ LOCKMODE lockmode);
extern void BootstrapToastTable(char *relName,
Oid toastOid, Oid toastIndexOid);
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 1e73b4aebd7..36acec1ca8e 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -23,4 +23,5 @@ test: multixact-no-deadlock
test: multixact-no-forget
test: propagate-lock-delete
test: drop-index-concurrently-1
+test: alter-table-1
test: timeouts
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e2279e63c11..a182176ba60 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1840,28 +1840,31 @@ and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
and c.relname != 'my_locks'
group by c.relname;
create table alterlock (f1 int primary key, f2 text);
+insert into alterlock values (1, 'foo');
+create table alterlock2 (f3 int primary key, f1 int);
+insert into alterlock2 values (1, 1);
begin; alter table alterlock alter column f2 set statistics 150;
select * from my_locks order by 1;
- relname | max_lockmode
------------+---------------------
- alterlock | AccessExclusiveLock
+ relname | max_lockmode
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
(1 row)
rollback;
begin; alter table alterlock cluster on alterlock_pkey;
select * from my_locks order by 1;
- relname | max_lockmode
-----------------+---------------------
- alterlock | AccessExclusiveLock
- alterlock_pkey | AccessExclusiveLock
+ relname | max_lockmode
+----------------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+ alterlock_pkey | ShareUpdateExclusiveLock
(2 rows)
commit;
begin; alter table alterlock set without cluster;
select * from my_locks order by 1;
- relname | max_lockmode
------------+---------------------
- alterlock | AccessExclusiveLock
+ relname | max_lockmode
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
(1 row)
commit;
@@ -1903,13 +1906,21 @@ select * from my_locks order by 1;
commit;
begin; alter table alterlock alter column f2 set (n_distinct = 1);
select * from my_locks order by 1;
+ relname | max_lockmode
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+(1 row)
+
+rollback;
+begin; alter table alterlock alter column f2 set storage extended;
+select * from my_locks order by 1;
relname | max_lockmode
-----------+---------------------
alterlock | AccessExclusiveLock
(1 row)
rollback;
-begin; alter table alterlock alter column f2 set storage extended;
+begin; alter table alterlock alter column f2 set default 'x';
select * from my_locks order by 1;
relname | max_lockmode
-----------+---------------------
@@ -1917,7 +1928,12 @@ select * from my_locks order by 1;
(1 row)
rollback;
-begin; alter table alterlock alter column f2 set default 'x';
+begin;
+create trigger ttdummy
+ before delete or update on alterlock
+ for each row
+ execute procedure
+ ttdummy (1, 1);
select * from my_locks order by 1;
relname | max_lockmode
-----------+---------------------
@@ -1925,7 +1941,48 @@ select * from my_locks order by 1;
(1 row)
rollback;
+begin;
+select * from my_locks order by 1;
+ relname | max_lockmode
+---------+--------------
+(0 rows)
+
+alter table alterlock2 add foreign key (f1) references alterlock (f1);
+select * from my_locks order by 1;
+ relname | max_lockmode
+-----------------+---------------------
+ alterlock | AccessExclusiveLock
+ alterlock2 | AccessExclusiveLock
+ alterlock2_pkey | AccessShareLock
+ alterlock_pkey | AccessShareLock
+(4 rows)
+
+rollback;
+begin;
+alter table alterlock2
+add constraint alterlock2nv foreign key (f1) references alterlock (f1) NOT VALID;
+select * from my_locks order by 1;
+ relname | max_lockmode
+------------+---------------------
+ alterlock | AccessExclusiveLock
+ alterlock2 | AccessExclusiveLock
+(2 rows)
+
+commit;
+begin;
+alter table alterlock2 validate constraint alterlock2nv;
+select * from my_locks order by 1;
+ relname | max_lockmode
+-----------------+--------------------------
+ alterlock | RowShareLock
+ alterlock2 | ShareUpdateExclusiveLock
+ alterlock2_pkey | AccessShareLock
+ alterlock_pkey | AccessShareLock
+(4 rows)
+
+rollback;
-- cleanup
+drop table alterlock2;
drop table alterlock;
drop view my_locks;
drop type lockmodes;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 2f2c2e35026..3f641f9596c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1283,6 +1283,9 @@ and c.relname != 'my_locks'
group by c.relname;
create table alterlock (f1 int primary key, f2 text);
+insert into alterlock values (1, 'foo');
+create table alterlock2 (f3 int primary key, f1 int);
+insert into alterlock2 values (1, 1);
begin; alter table alterlock alter column f2 set statistics 150;
select * from my_locks order by 1;
@@ -1324,7 +1327,33 @@ begin; alter table alterlock alter column f2 set default 'x';
select * from my_locks order by 1;
rollback;
+begin;
+create trigger ttdummy
+ before delete or update on alterlock
+ for each row
+ execute procedure
+ ttdummy (1, 1);
+select * from my_locks order by 1;
+rollback;
+
+begin;
+select * from my_locks order by 1;
+alter table alterlock2 add foreign key (f1) references alterlock (f1);
+select * from my_locks order by 1;
+rollback;
+
+begin;
+alter table alterlock2
+add constraint alterlock2nv foreign key (f1) references alterlock (f1) NOT VALID;
+select * from my_locks order by 1;
+commit;
+begin;
+alter table alterlock2 validate constraint alterlock2nv;
+select * from my_locks order by 1;
+rollback;
+
-- cleanup
+drop table alterlock2;
drop table alterlock;
drop view my_locks;
drop type lockmodes;
generated by cgit v1.2.3 (git 2.25.1) at 2025-05-28 15:42:53 +0000