agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2] Fix various shared memory estimates.
41+ messages / 6 participants
[nested] [flat]
* [PATCH v2] Fix various shared memory estimates.
@ 2021-02-27 07:45 Julien Rouhaud <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Julien Rouhaud @ 2021-02-27 07:45 UTC (permalink / raw)
---
contrib/pg_prewarm/autoprewarm.c | 2 +-
.../pg_stat_statements/pg_stat_statements.c | 2 +-
src/backend/access/common/syncscan.c | 2 +-
src/backend/access/nbtree/nbtutils.c | 3 +-
src/backend/access/transam/multixact.c | 2 +-
src/backend/access/transam/slru.c | 2 +-
src/backend/access/transam/twophase.c | 2 +-
src/backend/access/transam/xlog.c | 2 +-
src/backend/commands/async.c | 1 +
src/backend/postmaster/autovacuum.c | 3 +-
src/backend/postmaster/bgworker.c | 2 +-
src/backend/postmaster/checkpointer.c | 2 +-
src/backend/postmaster/pgstat.c | 17 ++---
src/backend/postmaster/postmaster.c | 2 +-
src/backend/replication/logical/launcher.c | 3 +-
src/backend/replication/logical/origin.c | 3 +-
src/backend/replication/slot.c | 2 +-
src/backend/replication/walreceiverfuncs.c | 2 +-
src/backend/replication/walsender.c | 2 +-
src/backend/storage/buffer/buf_init.c | 14 ++--
src/backend/storage/buffer/buf_table.c | 3 +-
src/backend/storage/buffer/freelist.c | 2 +-
src/backend/storage/ipc/dsm.c | 2 +-
src/backend/storage/ipc/pmsignal.c | 2 +-
src/backend/storage/ipc/procarray.c | 8 +--
src/backend/storage/ipc/procsignal.c | 2 +-
src/backend/storage/ipc/sinvaladt.c | 2 +-
src/backend/storage/lmgr/lock.c | 16 ++++-
src/backend/storage/lmgr/lwlock.c | 2 +-
src/backend/storage/lmgr/predicate.c | 22 +++---
src/backend/storage/lmgr/proc.c | 12 ++--
src/backend/utils/hash/dynahash.c | 69 ++++++++++++++-----
src/backend/utils/time/snapmgr.c | 2 +-
src/include/storage/predicate_internals.h | 4 +-
src/include/storage/shmem.h | 13 ++++
src/include/utils/hsearch.h | 2 +
36 files changed, 146 insertions(+), 87 deletions(-)
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index b3f73ea92d..887e68b288 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -138,7 +138,7 @@ _PG_init(void)
EmitWarningsOnPlaceholders("pg_prewarm");
- RequestAddinShmemSpace(MAXALIGN(sizeof(AutoPrewarmSharedState)));
+ RequestAddinShmemSpace(CACHELINEALIGN(sizeof(AutoPrewarmSharedState)));
/* Register autoprewarm worker, if enabled. */
if (autoprewarm)
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 62cccbfa44..4e045ffba7 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1933,7 +1933,7 @@ pgss_memsize(void)
{
Size size;
- size = MAXALIGN(sizeof(pgssSharedState));
+ size = CACHELINEALIGN(sizeof(pgssSharedState));
size = add_size(size, hash_estimate_size(pgss_max, sizeof(pgssEntry)));
return size;
diff --git a/src/backend/access/common/syncscan.c b/src/backend/access/common/syncscan.c
index b7a28af4ad..c19f9cab59 100644
--- a/src/backend/access/common/syncscan.c
+++ b/src/backend/access/common/syncscan.c
@@ -125,7 +125,7 @@ static BlockNumber ss_search(RelFileNode relfilenode,
Size
SyncScanShmemSize(void)
{
- return SizeOfScanLocations(SYNC_SCAN_NELEM);
+ return (Size) CACHELINEALIGN(SizeOfScanLocations(SYNC_SCAN_NELEM));
}
/*
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index d524310723..310ea904df 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2066,7 +2066,8 @@ BTreeShmemSize(void)
size = offsetof(BTVacInfo, vacuums);
size = add_size(size, mul_size(MaxBackends, sizeof(BTOneVacInfo)));
- return size;
+
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 1f9f1a1fa1..c789623c34 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1830,7 +1830,7 @@ MultiXactShmemSize(void)
add_size(offsetof(MultiXactStateData, perBackendXactIds) + sizeof(MultiXactId), \
mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot))
- size = SHARED_MULTIXACT_STATE_SIZE;
+ size = CACHELINEALIGN(SHARED_MULTIXACT_STATE_SIZE);
size = add_size(size, SimpleLruShmemSize(NUM_MULTIXACTOFFSET_BUFFERS, 0));
size = add_size(size, SimpleLruShmemSize(NUM_MULTIXACTMEMBER_BUFFERS, 0));
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 82149ad782..2271e092e9 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -168,7 +168,7 @@ SimpleLruShmemSize(int nslots, int nlsns)
if (nlsns > 0)
sz += MAXALIGN(nslots * nlsns * sizeof(XLogRecPtr)); /* group_lsn[] */
- return BUFFERALIGN(sz) + BLCKSZ * nslots;
+ return (Size) CACHELINEALIGN(BUFFERALIGN(sz) + BLCKSZ * nslots);
}
/*
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 80d2d20d6c..fca70fabac 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -245,7 +245,7 @@ TwoPhaseShmemSize(void)
size = add_size(size, mul_size(max_prepared_xacts,
sizeof(GlobalTransactionData)));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
void
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 377afb8732..64de453340 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5096,7 +5096,7 @@ XLOGShmemSize(void)
* routine again below to compute the actual allocation size.
*/
- return size;
+ return (Size) CACHELINEALIGN(size);
}
void
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 4b16fb5682..6a0fd6ca10 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -513,6 +513,7 @@ AsyncShmemSize(void)
/* This had better match AsyncShmemInit */
size = mul_size(MaxBackends + 1, sizeof(QueueBackendStatus));
size = add_size(size, offsetof(AsyncQueueControl, backend));
+ size = (Size) CACHELINEALIGN(size);
size = add_size(size, SimpleLruShmemSize(NUM_NOTIFY_BUFFERS, 0));
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 23ef23c13e..7ddf86af74 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -3414,7 +3414,8 @@ AutoVacuumShmemSize(void)
size = MAXALIGN(size);
size = add_size(size, mul_size(autovacuum_max_workers,
sizeof(WorkerInfoData)));
- return size;
+
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index bbbc09b0b5..d2f838eaed 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -148,7 +148,7 @@ BackgroundWorkerShmemSize(void)
size = add_size(size, mul_size(max_worker_processes,
sizeof(BackgroundWorkerSlot)));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 76f9f98ebb..6acd5e4c2b 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -862,7 +862,7 @@ CheckpointerShmemSize(void)
size = offsetof(CheckpointerShmemStruct, requests);
size = add_size(size, mul_size(NBuffers, sizeof(CheckpointerRequest)));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index f75b52719d..b08f7f1d56 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -2930,25 +2930,20 @@ BackendStatusShmemSize(void)
Size size;
/* BackendStatusArray: */
- size = mul_size(sizeof(PgBackendStatus), NumBackendStatSlots);
+ size = mul_size_and_shmem_align(sizeof(PgBackendStatus), NumBackendStatSlots);
/* BackendAppnameBuffer: */
- size = add_size(size,
- mul_size(NAMEDATALEN, NumBackendStatSlots));
+ size = add_shmem_aligned_size(size, mul_size(NAMEDATALEN, NumBackendStatSlots));
/* BackendClientHostnameBuffer: */
- size = add_size(size,
- mul_size(NAMEDATALEN, NumBackendStatSlots));
+ size = add_shmem_aligned_size(size, mul_size(NAMEDATALEN, NumBackendStatSlots));
/* BackendActivityBuffer: */
- size = add_size(size,
- mul_size(pgstat_track_activity_query_size, NumBackendStatSlots));
+ size = add_shmem_aligned_size(size, mul_size(pgstat_track_activity_query_size, NumBackendStatSlots));
#ifdef USE_SSL
/* BackendSslStatusBuffer: */
- size = add_size(size,
- mul_size(sizeof(PgBackendSSLStatus), NumBackendStatSlots));
+ size = add_shmem_aligned_size(size, mul_size(sizeof(PgBackendSSLStatus), NumBackendStatSlots));
#endif
#ifdef ENABLE_GSS
/* BackendGssStatusBuffer: */
- size = add_size(size,
- mul_size(sizeof(PgBackendGSSStatus), NumBackendStatSlots));
+ size = add_shmem_aligned_size(size, mul_size(sizeof(PgBackendGSSStatus), NumBackendStatSlots));
#endif
return size;
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index edab95a19e..716f7434e6 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -6425,7 +6425,7 @@ restore_backend_variables(BackendParameters *param, Port *port)
Size
ShmemBackendArraySize(void)
{
- return mul_size(MaxLivePostmasterChildren(), sizeof(Backend));
+ return mul_size_and_shmem_align(MaxLivePostmasterChildren(), sizeof(Backend));
}
void
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index cb462a052a..7d20877920 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -697,7 +697,8 @@ ApplyLauncherShmemSize(void)
size = MAXALIGN(size);
size = add_size(size, mul_size(max_logical_replication_workers,
sizeof(LogicalRepWorker)));
- return size;
+
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 39471fddad..b9e9fa121a 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -503,7 +503,8 @@ ReplicationOriginShmemSize(void)
size = add_size(size,
mul_size(max_replication_slots, sizeof(ReplicationState)));
- return size;
+
+ return (Size) CACHELINEALIGN(size);
}
void
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 75a087c2f9..c399c491e9 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -124,7 +124,7 @@ ReplicationSlotsShmemSize(void)
size = add_size(size,
mul_size(max_replication_slots, sizeof(ReplicationSlot)));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c
index 63e60478ea..a8cab3fa28 100644
--- a/src/backend/replication/walreceiverfuncs.c
+++ b/src/backend/replication/walreceiverfuncs.c
@@ -45,7 +45,7 @@ WalRcvShmemSize(void)
size = add_size(size, sizeof(WalRcvData));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/* Allocate and initialize walreceiver-related shared memory */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 23baa4498a..d1c91cf354 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3055,7 +3055,7 @@ WalSndShmemSize(void)
size = offsetof(WalSndCtlData, walsnds);
size = add_size(size, mul_size(max_wal_senders, sizeof(WalSnd)));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/* Allocate and initialize walsender-related shared memory */
diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c
index e9e4f35bb5..b92fdeef14 100644
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -159,15 +159,13 @@ BufferShmemSize(void)
Size size = 0;
/* size of buffer descriptors */
- size = add_size(size, mul_size(NBuffers, sizeof(BufferDescPadded)));
- /* to allow aligning buffer descriptors */
- size = add_size(size, PG_CACHE_LINE_SIZE);
+ size = add_shmem_aligned_size(size, mul_size(NBuffers, sizeof(BufferDescPadded)));
/* size of data pages */
- size = add_size(size, mul_size(NBuffers, BLCKSZ));
+ size = add_shmem_aligned_size(size, mul_size(NBuffers, BLCKSZ));
/* size of stuff controlled by freelist.c */
- size = add_size(size, StrategyShmemSize());
+ size = add_shmem_aligned_size(size, StrategyShmemSize());
/*
* It would be nice to include the I/O locks in the BufferDesc, but that
@@ -178,12 +176,10 @@ BufferShmemSize(void)
* locks are not highly contended, we lay out the array with minimal
* padding.
*/
- size = add_size(size, mul_size(NBuffers, sizeof(LWLockMinimallyPadded)));
- /* to allow aligning the above */
- size = add_size(size, PG_CACHE_LINE_SIZE);
+ size = add_shmem_aligned_size(size, mul_size(NBuffers, sizeof(LWLockMinimallyPadded)));
/* size of checkpoint sort array in bufmgr.c */
- size = add_size(size, mul_size(NBuffers, sizeof(CkptSortItem)));
+ size = add_shmem_aligned_size(size, mul_size(NBuffers, sizeof(CkptSortItem)));
return size;
}
diff --git a/src/backend/storage/buffer/buf_table.c b/src/backend/storage/buffer/buf_table.c
index caa03ae123..4200f99340 100644
--- a/src/backend/storage/buffer/buf_table.c
+++ b/src/backend/storage/buffer/buf_table.c
@@ -41,7 +41,8 @@ static HTAB *SharedBufHash;
Size
BufTableShmemSize(int size)
{
- return hash_estimate_size(size, sizeof(BufferLookupEnt));
+ return hash_estimate_size_ext(size, sizeof(BufferLookupEnt),
+ size, true);
}
/*
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 6be80476db..33e9e74f40 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -459,7 +459,7 @@ StrategyShmemSize(void)
size = add_size(size, BufTableShmemSize(NBuffers + NUM_BUFFER_PARTITIONS));
/* size of the shared replacement strategy control block */
- size = add_size(size, MAXALIGN(sizeof(BufferStrategyControl)));
+ size = add_shmem_aligned_size(size, sizeof(BufferStrategyControl));
return size;
}
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index b461a5f7e9..6ecf195c55 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -440,7 +440,7 @@ dsm_set_control_handle(dsm_handle h)
size_t
dsm_estimate_size(void)
{
- return 1024 * 1024 * (size_t) min_dynamic_shared_memory;
+ return (size_t) CACHELINEALIGN(1024 * 1024 * min_dynamic_shared_memory);
}
/*
diff --git a/src/backend/storage/ipc/pmsignal.c b/src/backend/storage/ipc/pmsignal.c
index 280c2395c9..d25080224a 100644
--- a/src/backend/storage/ipc/pmsignal.c
+++ b/src/backend/storage/ipc/pmsignal.c
@@ -124,7 +124,7 @@ PMSignalShmemSize(void)
size = add_size(size, mul_size(MaxLivePostmasterChildren(),
sizeof(sig_atomic_t)));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4fc6ffb917..9b804f1152 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -358,6 +358,7 @@ ProcArrayShmemSize(void)
size = offsetof(ProcArrayStruct, pgprocnos);
size = add_size(size, mul_size(sizeof(int), PROCARRAY_MAXPROCS));
+ size = CACHELINEALIGN(size);
/*
* During Hot Standby processing we have a data structure called
@@ -377,11 +378,8 @@ ProcArrayShmemSize(void)
if (EnableHotStandby)
{
- size = add_size(size,
- mul_size(sizeof(TransactionId),
- TOTAL_MAX_CACHED_SUBXIDS));
- size = add_size(size,
- mul_size(sizeof(bool), TOTAL_MAX_CACHED_SUBXIDS));
+ size = add_shmem_aligned_size(size, mul_size(sizeof(TransactionId), TOTAL_MAX_CACHED_SUBXIDS));
+ size = add_shmem_aligned_size(size, mul_size(sizeof(bool), TOTAL_MAX_CACHED_SUBXIDS));
}
return size;
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index c6a8d4611e..2af7f3e343 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -113,7 +113,7 @@ ProcSignalShmemSize(void)
size = mul_size(NumProcSignalSlots, sizeof(ProcSignalSlot));
size = add_size(size, offsetof(ProcSignalHeader, psh_slot));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c
index 946bd8e3cb..8379bf12e7 100644
--- a/src/backend/storage/ipc/sinvaladt.c
+++ b/src/backend/storage/ipc/sinvaladt.c
@@ -207,7 +207,7 @@ SInvalShmemSize(void)
size = offsetof(SISeg, procState);
size = add_size(size, mul_size(sizeof(ProcState), MaxBackends));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 108b4d9023..f2fc8af5c5 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -54,7 +54,7 @@
int max_locks_per_xact; /* set by guc.c */
#define NLOCKENTS() \
- mul_size(max_locks_per_xact, add_size(MaxBackends, max_prepared_xacts))
+ mul_size_and_shmem_align(max_locks_per_xact, add_size(MaxBackends, max_prepared_xacts))
/*
@@ -3527,17 +3527,27 @@ LockShmemSize(void)
/* lock hash table */
max_table_size = NLOCKENTS();
- size = add_size(size, hash_estimate_size(max_table_size, sizeof(LOCK)));
+ size = add_size(size, hash_estimate_size_ext(max_table_size, sizeof(LOCK),
+ max_table_size / 2,
+ true));
/* proclock hash table */
max_table_size *= 2;
- size = add_size(size, hash_estimate_size(max_table_size, sizeof(PROCLOCK)));
+ size = add_size(size, hash_estimate_size_ext(max_table_size, sizeof(PROCLOCK),
+ max_table_size / 2,
+ true));
/*
* Since NLOCKENTS is only an estimate, add 10% safety margin.
*/
size = add_size(size, size / 10);
+ /*
+ * Note: we don't count fast-path structure, it comes out of the "slop
+ * factor" added by CreateSharedMemoryAndSemaphores. This lets us use this
+ * routine again below to compute the actual allocation size.
+ */
+
return size;
}
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 8cb6a6f042..6a47918616 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -456,7 +456,7 @@ LWLockShmemSize(void)
/* Disallow adding any more named tranches. */
lock_named_request_allowed = false;
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index d493aeef0f..1dcadc00ae 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -1359,13 +1359,17 @@ PredicateLockShmemSize(void)
/* predicate lock target hash table */
max_table_size = NPREDICATELOCKTARGETENTS();
- size = add_size(size, hash_estimate_size(max_table_size,
- sizeof(PREDICATELOCKTARGET)));
+ size = add_size(size, hash_estimate_size_ext(max_table_size,
+ sizeof(PREDICATELOCKTARGET),
+ max_table_size,
+ true));
/* predicate lock hash table */
max_table_size *= 2;
- size = add_size(size, hash_estimate_size(max_table_size,
- sizeof(PREDICATELOCK)));
+ size = add_size(size, hash_estimate_size_ext(max_table_size,
+ sizeof(PREDICATELOCK),
+ max_table_size,
+ true));
/*
* Since NPREDICATELOCKTARGETENTS is only an estimate, add 10% safety
@@ -1377,8 +1381,7 @@ PredicateLockShmemSize(void)
max_table_size = MaxBackends + max_prepared_xacts;
max_table_size *= 10;
size = add_size(size, PredXactListDataSize);
- size = add_size(size, mul_size((Size) max_table_size,
- PredXactListElementDataSize));
+ size = add_shmem_aligned_size(size, mul_size((Size) max_table_size, PredXactListElementDataSize));
/* transaction xid table */
size = add_size(size, hash_estimate_size(max_table_size,
@@ -1387,14 +1390,13 @@ PredicateLockShmemSize(void)
/* rw-conflict pool */
max_table_size *= 5;
size = add_size(size, RWConflictPoolHeaderDataSize);
- size = add_size(size, mul_size((Size) max_table_size,
- RWConflictDataSize));
+ size = add_shmem_aligned_size(size, mul_size((Size) max_table_size, RWConflictDataSize));
/* Head for list of finished serializable transactions. */
- size = add_size(size, sizeof(SHM_QUEUE));
+ size = add_shmem_aligned_size(size, sizeof(SHM_QUEUE));
/* Shared memory structures for SLRU tracking of old committed xids. */
- size = add_size(size, sizeof(SerialControlData));
+ size = add_shmem_aligned_size(size, sizeof(SerialControlData));
size = add_size(size, SimpleLruShmemSize(NUM_SERIAL_BUFFERS, 0));
return size;
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 897045ee27..28031c7d22 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -106,13 +106,13 @@ ProcGlobalShmemSize(void)
add_size(MaxBackends, add_size(NUM_AUXILIARY_PROCS, max_prepared_xacts));
/* ProcGlobal */
- size = add_size(size, sizeof(PROC_HDR));
- size = add_size(size, mul_size(TotalProcs, sizeof(PGPROC)));
- size = add_size(size, sizeof(slock_t));
+ size = add_shmem_aligned_size(size, sizeof(PROC_HDR));
+ size = add_shmem_aligned_size(size, mul_size(TotalProcs, sizeof(PGPROC)));
+ size = add_shmem_aligned_size(size, sizeof(slock_t));
- size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->xids)));
- size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->subxidStates)));
- size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->statusFlags)));
+ size = add_shmem_aligned_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->xids)));
+ size = add_shmem_aligned_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->subxidStates)));
+ size = add_shmem_aligned_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->statusFlags)));
return size;
}
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 6546e3c7c7..030ca3917c 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -769,23 +769,43 @@ init_htab(HTAB *hashp, long nelem)
return true;
}
+/*
+ * Wrapper around hash_estimate_size for non partitioned hash table and all
+ * entries preallocated.
+ */
+Size
+hash_estimate_size(long num_entries, Size entrysize)
+{
+ return hash_estimate_size_ext(num_entries, entrysize, num_entries, false);
+}
+
/*
* Estimate the space needed for a hashtable containing the given number
- * of entries of given size.
+ * of entries of given size, and given it's partitioned or not.
* NOTE: this is used to estimate the footprint of hashtables in shared
* memory; therefore it does not count HTAB which is in local memory.
* NB: assumes that all hash structure parameters have default values!
*/
Size
-hash_estimate_size(long num_entries, Size entrysize)
+hash_estimate_size_ext(long num_entries, Size entrysize, long init_entries,
+ bool partitioned)
{
- Size size;
+ Size size,
+ segmentSize,
+ freelistAllocSize;
long nBuckets,
nSegments,
nDirEntries,
- nElementAllocs,
elementSize,
- elementAllocCnt;
+ nelemFreelist;
+ int nFreelists;
+
+ if (partitioned)
+ nFreelists = NUM_FREELISTS;
+ else
+ nFreelists = 1;
+
+ Assert(init_entries <= num_entries);
/* estimate number of buckets wanted */
nBuckets = next_pow2_long(num_entries);
@@ -797,19 +817,36 @@ hash_estimate_size(long num_entries, Size entrysize)
nDirEntries <<= 1; /* dir_alloc doubles dsize at each call */
/* fixed control info */
- size = MAXALIGN(sizeof(HASHHDR)); /* but not HTAB, per above */
+ size = CACHELINEALIGN(sizeof(HASHHDR)); /* but not HTAB, per above */
/* directory */
- size = add_size(size, mul_size(nDirEntries, sizeof(HASHSEGMENT)));
+ size = add_shmem_aligned_size(size, mul_size(nDirEntries, sizeof(HASHSEGMENT)));
/* segments */
- size = add_size(size, mul_size(nSegments,
- MAXALIGN(DEF_SEGSIZE * sizeof(HASHBUCKET))));
- /* elements --- allocated in groups of choose_nelem_alloc() entries */
- elementAllocCnt = choose_nelem_alloc(entrysize);
- nElementAllocs = (num_entries - 1) / elementAllocCnt + 1;
+ segmentSize = CACHELINEALIGN(DEF_SEGSIZE * sizeof(HASHBUCKET));
+ size = add_size(size, mul_size(nSegments, segmentSize));
+
elementSize = MAXALIGN(sizeof(HASHELEMENT)) + MAXALIGN(entrysize);
- size = add_size(size,
- mul_size(nElementAllocs,
- mul_size(elementAllocCnt, elementSize)));
+
+ /*
+ * elements --- allocated in groups of choose_nelem_alloc() entries, except
+ * for the first init_entries, allocated in num_freelists groups.
+ */
+ nelemFreelist = init_entries / nFreelists;
+ freelistAllocSize = mul_size_and_shmem_align(nelemFreelist, elementSize);
+
+ size = add_size(size, mul_size(nFreelists, freelistAllocSize));
+
+ if (num_entries > init_entries)
+ {
+ Size nelem,
+ nAlloc,
+ allocSize;
+
+ nelem = choose_nelem_alloc(entrysize);
+ nAlloc = (num_entries - init_entries - 1) / nelem + 1;
+ allocSize = mul_size_and_shmem_align(nelem, elementSize);
+
+ size = add_size(size, mul_size(nAlloc, allocSize));
+ }
return size;
}
@@ -852,7 +889,7 @@ hash_get_shared_size(HASHCTL *info, int flags)
{
Assert(flags & HASH_DIRSIZE);
Assert(info->dsize == info->max_dsize);
- return sizeof(HASHHDR) + info->dsize * sizeof(HASHSEGMENT);
+ return (Size) CACHELINEALIGN(sizeof(HASHHDR) + info->dsize * sizeof(HASHSEGMENT));
}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 95704265b6..b66278e466 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -203,7 +203,7 @@ SnapMgrShmemSize(void)
size = add_size(size, mul_size(sizeof(TransactionId),
OLD_SNAPSHOT_TIME_MAP_ENTRIES));
- return size;
+ return (Size) CACHELINEALIGN(size);
}
/*
diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index 104f560d38..bbc6ca4cbf 100644
--- a/src/include/storage/predicate_internals.h
+++ b/src/include/storage/predicate_internals.h
@@ -192,7 +192,7 @@ typedef struct PredXactListData
typedef struct PredXactListData *PredXactList;
#define PredXactListDataSize \
- ((Size)MAXALIGN(sizeof(PredXactListData)))
+ ((Size)CACHELINEALIGN(sizeof(PredXactListData)))
/*
@@ -227,7 +227,7 @@ typedef struct RWConflictPoolHeaderData
typedef struct RWConflictPoolHeaderData *RWConflictPoolHeader;
#define RWConflictPoolHeaderDataSize \
- ((Size)MAXALIGN(sizeof(RWConflictPoolHeaderData)))
+ ((Size)CACHELINEALIGN(sizeof(RWConflictPoolHeaderData)))
/*
diff --git a/src/include/storage/shmem.h b/src/include/storage/shmem.h
index 31024e5a50..897672bc8e 100644
--- a/src/include/storage/shmem.h
+++ b/src/include/storage/shmem.h
@@ -45,6 +45,19 @@ extern void *ShmemInitStruct(const char *name, Size size, bool *foundPtr);
extern Size add_size(Size s1, Size s2);
extern Size mul_size(Size s1, Size s2);
+/*
+ * Macro around add_size and mul_size to account for extra padding as done in
+ * ShmemAlloc().
+ *
+ * add_shmem_aligned_size() will add the padding bytes to the 2nd argument
+ * mul_size_and_shmem_align will return the aligned size
+ */
+#define add_shmem_aligned_size(s1, s2) add_size((s1), \
+ (Size) CACHELINEALIGN(s2))
+
+#define mul_size_and_shmem_align(s1, s2) add_shmem_aligned_size(0, \
+ mul_size((s1), (s2)))
+
/* ipci.c */
extern void RequestAddinShmemSpace(Size size);
diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index d7af0239c8..20c0ff3036 100644
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -145,6 +145,8 @@ extern void *hash_seq_search(HASH_SEQ_STATUS *status);
extern void hash_seq_term(HASH_SEQ_STATUS *status);
extern void hash_freeze(HTAB *hashp);
extern Size hash_estimate_size(long num_entries, Size entrysize);
+extern Size hash_estimate_size_ext(long num_entries, Size entrysize,
+ long init_entries, bool num_freelists);
extern long hash_select_dirsize(long num_entries);
extern Size hash_get_shared_size(HASHCTL *info, int flags);
extern void AtEOXact_HashTables(bool isCommit);
--
2.30.1
--qghjt52pj73pl6gt--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v3 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..a35696480c7 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,45 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +607,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Thu__5_Jun_2025_16_26_08_+0900_Z3LZV.DHOLuCekYi--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
---
src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index fe0490259e9..cfd2e08ea23 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -35,6 +35,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -355,24 +356,45 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -553,11 +575,13 @@ ProcedureCreate(const char *procedureName,
replaces[Anum_pg_proc_proowner - 1] = false;
replaces[Anum_pg_proc_proacl - 1] = false;
+
+
/* Okay, do it... */
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.34.1
--Multipart=_Mon__31_Mar_2025_20_22_35_+0900_icGj3NSLeP4Y27Qc--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v7 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..734508c7f58 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,47 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ heap_freetuple(oldtup);
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +609,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Thu__3_Jul_2025_23_18_12_+0900_9OhecyAr5v_aEYXi--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v9 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 41 ++++++++++++++++++++++++++++-------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 75b17fed15e..889e9b68a0d 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,48 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
+ Oid procoid = oldproc->oid;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ heap_freetuple(oldtup);
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, procoid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +610,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Wed__20_Aug_2025_17_01_56_+0900_8Htb/SMotxazoe2R--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v6 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..734508c7f58 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,47 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ heap_freetuple(oldtup);
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +609,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Tue__1_Jul_2025_19_47_26_+0900_Hfm68M1moVPepmMg--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v4 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..a35696480c7 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,45 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +607,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Thu__5_Jun_2025_19_20_58_+0900_MlxgAk6H+gGvpZew--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
---
src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index fe0490259e9..cfd2e08ea23 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -35,6 +35,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -355,24 +356,45 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -553,11 +575,13 @@ ProcedureCreate(const char *procedureName,
replaces[Anum_pg_proc_proowner - 1] = false;
replaces[Anum_pg_proc_proacl - 1] = false;
+
+
/* Okay, do it... */
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.34.1
--Multipart=_Mon__31_Mar_2025_20_00_57_+0900_=a2jVPgfV36qgodU--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v2 1/2] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
---
src/backend/catalog/pg_proc.c | 38 +++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index fe0490259e9..3a15374b261 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -35,6 +35,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -355,24 +356,45 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -557,7 +579,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Tue__27_May_2025_03_17_51_+0900_4z1nO_cGkjxsWjZt--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v5 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..734508c7f58 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,47 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ heap_freetuple(oldtup);
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +609,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Mon__30_Jun_2025_17_47_44_+0900_2lefeRoUXzTcBKe0--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v8 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..734508c7f58 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,47 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ heap_freetuple(oldtup);
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +609,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Thu__17_Jul_2025_14_09_14_+0900_WdPnEQ7x1hu.aLa8--
^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH v10 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 09:46 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 09:46 UTC (permalink / raw)
Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail
with an internal error "tuple concurrently updated". This occurred
because multiple sessions attempted to modify the same catalog tuple
simultaneously.
To prevent this, ensure that an exclusive lock on the function object
is acquired earlier in the process.
Additionally, if the target function is dropped by another session while
waiting for the lock, a new function is created instead.
---
src/backend/catalog/pg_proc.c | 41 ++++++++++++++++++++++++++++-------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b89b9ccda0e..b459badf074 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -34,6 +34,7 @@
#include "parser/parse_coerce.h"
#include "pgstat.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
@@ -383,24 +384,48 @@ ProcedureCreate(const char *procedureName,
tupDesc = RelationGetDescr(rel);
/* Check for pre-existing definition */
- oldtup = SearchSysCache3(PROCNAMEARGSNSP,
- PointerGetDatum(procedureName),
- PointerGetDatum(parameterTypes),
- ObjectIdGetDatum(procNamespace));
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
- Datum proargnames;
- bool isnull;
- const char *dropcmd;
+ Oid procoid = oldproc->oid;
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same argument types",
procedureName)));
+
+ heap_freetuple(oldtup);
+
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, procoid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
+ }
+
+ if (HeapTupleIsValid(oldtup))
+ {
+
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
+
if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
procedureName);
@@ -585,7 +610,7 @@ ProcedureCreate(const char *procedureName,
tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces);
CatalogTupleUpdate(rel, &tup->t_self, tup);
- ReleaseSysCache(oldtup);
+ heap_freetuple(oldtup);
is_update = true;
}
else
--
2.43.0
--Multipart=_Tue__30_Sep_2025_11_01_58_+0900_79ESurbZ01uj0JWr--
^ permalink raw reply [nested|flat] 41+ messages in thread
* Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
@ 2025-03-31 11:00 Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 11:00 UTC (permalink / raw)
To: pgsql-hackers
Hi,
I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION
for a same function, the error "tuple concurrently updated" is raised. This is
an internal error output by elog, also the message is not user-friendly.
I've attached a patch to prevent this internal error by locking an exclusive
lock before the command and get the read tuple after acquiring the lock.
Also, if the function has been removed during the lock waiting, the new entry
is created.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-03-31 11:22 ` Yugo Nagata <[email protected]>
2025-05-20 15:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
0 siblings, 3 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-03-31 11:22 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers
On Mon, 31 Mar 2025 20:00:57 +0900
Yugo Nagata <[email protected]> wrote:
> Hi,
>
> I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION
> for a same function, the error "tuple concurrently updated" is raised. This is
> an internal error output by elog, also the message is not user-friendly.
>
> I've attached a patch to prevent this internal error by locking an exclusive
> lock before the command and get the read tuple after acquiring the lock.
> Also, if the function has been removed during the lock waiting, the new entry
> is created.
I also found the same error is raised when concurrent ALTER FUNCTION commands are
executed. I've added a patch to fix this in the similar way.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-05-20 15:30 ` Jim Jones <[email protected]>
2025-05-26 16:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2 siblings, 1 reply; 41+ messages in thread
From: Jim Jones @ 2025-05-20 15:30 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers
Hi!
On 31.03.25 13:22, Yugo Nagata wrote:
> On Mon, 31 Mar 2025 20:00:57 +0900
> Yugo Nagata <[email protected]> wrote:
>
>> Hi,
>>
>> I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION
>> for a same function, the error "tuple concurrently updated" is raised. This is
>> an internal error output by elog, also the message is not user-friendly.
>>
>> I've attached a patch to prevent this internal error by locking an exclusive
>> lock before the command and get the read tuple after acquiring the lock.
>> Also, if the function has been removed during the lock waiting, the new entry
>> is created.
> I also found the same error is raised when concurrent ALTER FUNCTION commands are
> executed. I've added a patch to fix this in the similar way.
>
> Regards,
> Yugo Nagata
I just briefly tested this patch and it seems to work as expected for
CREATE OF REPLACE FUNCTION:
-- Session 1 (t1):
postgres=# BEGIN;
BEGIN
postgres=*# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 1; END;$$;
CREATE FUNCTION
-- Session 2 (t2)
postgres=# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 2; END;$$;
(wait)
-- Session 3 (t3)
postgres=# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 3; END;$$;
(wait)
-- Session 4 (t4)
postgres=# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 4; END;$$;
CREATE FUNCTION
(wait)
-- Session 1 (t5)
postgres=*# END;
COMMIT
at this point Sessions 2, 3, and 4 were released with: CREATE FUNCTION
-- Session 1 (t6)
postgres=# \sf f1
CREATE OR REPLACE FUNCTION public.f1()
RETURNS integer
LANGUAGE plpgsql
AS $function$ BEGIN RETURN 4; END;$function$
So... it no longer shows the error message:
ERROR: tuple concurrently updated
I did the same for ALTER FUNCTION but I was unable to reproduce the
error your reported. Could you provide your script?
Best regards, Jim
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-20 15:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
@ 2025-05-26 16:39 ` Yugo Nagata <[email protected]>
2025-05-27 06:33 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-05-26 16:39 UTC (permalink / raw)
To: Jim Jones <[email protected]>; +Cc: pgsql-hackers
On Tue, 20 May 2025 17:30:35 +0200
Jim Jones <[email protected]> wrote:
> I just briefly tested this patch and it seems to work as expected for
> CREATE OF REPLACE FUNCTION:
Thank you for reviewing the patch!
> I did the same for ALTER FUNCTION but I was unable to reproduce the
> error your reported. Could you provide your script?
I can see the error when two concurrent transactions issue
"alter function f() immutable".
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-20 15:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
2025-05-26 16:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-05-27 06:33 ` Jim Jones <[email protected]>
2025-05-27 08:52 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Jim Jones @ 2025-05-27 06:33 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers
Hi Yugo
On 26.05.25 18:39, Yugo Nagata wrote:
> I can see the error when two concurrent transactions issue
> "alter function f() immutable".
I might have missed something in my last tests... I could now reproduce
the behaviour you mentioned.
I've tested v2 and it works as described. CREATE OR REPLACE FUNCTION and
ALTER TABLE no longer raise an error after the lock by the concurrent
transaction was freed.
One quick question in v2-002:
tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
- if (!HeapTupleIsValid(tup)) /* should not happen */
- elog(ERROR, "cache lookup failed for function %u", funcOid);
+ if (!HeapTupleIsValid(tup))
+ ereport(ERROR,
+ errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("function \"%s\" does not exist",
+ NameListToString(stmt->func->objname)));
Is it really ok to change this error message here? Did the addition of
LockDatabaseObject change the semantics of the previous message? Other
similar parts of the code still report "cache lookup failed for function
x". I don't have a strong opinion here, but perhaps we should keep these
messages consistent at least throughout the file?
Thanks!
Best, Jim
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-20 15:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
2025-05-26 16:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:33 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
@ 2025-05-27 08:52 ` Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-05-27 08:52 UTC (permalink / raw)
To: Jim Jones <[email protected]>; +Cc: pgsql-hackers
On Tue, 27 May 2025 08:33:42 +0200
Jim Jones <[email protected]> wrote:
> Hi Yugo
>
> On 26.05.25 18:39, Yugo Nagata wrote:
> > I can see the error when two concurrent transactions issue
> > "alter function f() immutable".
>
>
> I might have missed something in my last tests... I could now reproduce
> the behaviour you mentioned.
>
> I've tested v2 and it works as described. CREATE OR REPLACE FUNCTION and
> ALTER TABLE no longer raise an error after the lock by the concurrent
> transaction was freed.
>
> One quick question in v2-002:
>
> tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
> - if (!HeapTupleIsValid(tup)) /* should not happen */
> - elog(ERROR, "cache lookup failed for function %u", funcOid);
> + if (!HeapTupleIsValid(tup))
> + ereport(ERROR,
> + errcode(ERRCODE_UNDEFINED_OBJECT),
> + errmsg("function \"%s\" does not exist",
> + NameListToString(stmt->func->objname)));
>
>
> Is it really ok to change this error message here? Did the addition of
> LockDatabaseObject change the semantics of the previous message?
Yes. AcceptInvalidationMessages() is called in LockDatabaseObject() after wait,
and this enables the detection of object deletion during the wait.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-05-22 02:25 ` jian he <[email protected]>
2025-05-23 02:37 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-26 17:35 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2 siblings, 2 replies; 41+ messages in thread
From: jian he @ 2025-05-22 02:25 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers
On Mon, Mar 31, 2025 at 7:22 PM Yugo Nagata <[email protected]> wrote:
>
> On Mon, 31 Mar 2025 20:00:57 +0900
> Yugo Nagata <[email protected]> wrote:
>
> > Hi,
> >
> > I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION
> > for a same function, the error "tuple concurrently updated" is raised. This is
> > an internal error output by elog, also the message is not user-friendly.
> >
> > I've attached a patch to prevent this internal error by locking an exclusive
> > lock before the command and get the read tuple after acquiring the lock.
> > Also, if the function has been removed during the lock waiting, the new entry
> > is created.
>
> I also found the same error is raised when concurrent ALTER FUNCTION commands are
> executed. I've added a patch to fix this in the similar way.
>
hi.
+ /* Lock the function so nobody else can do anything with it. */
+ LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
+
+ /*
+ * It is possible that by the time we acquire the lock on function,
+ * concurrent DDL has removed it. We can test this by checking the
+ * existence of function. We get the tuple again to avoid the risk
+ * of function definition getting changed.
+ */
+ oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
+ PointerGetDatum(procedureName),
+ PointerGetDatum(parameterTypes),
+ ObjectIdGetDatum(procNamespace));
we already called LockDatabaseObject, concurrent DDL can
not do DROP FUNCTION or ALTER FUNCTION.
so no need to call SearchSysCacheCopy3 again?
@@ -553,11 +575,13 @@ ProcedureCreate(const char *procedureName,
replaces[Anum_pg_proc_proowner - 1] = false;
replaces[Anum_pg_proc_proacl - 1] = false;
+
+
/* Okay, do it... */
no need to add these two new lines.
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
@ 2025-05-23 02:37 ` jian he <[email protected]>
2025-05-26 18:17 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
1 sibling, 1 reply; 41+ messages in thread
From: jian he @ 2025-05-23 02:37 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers
On Thu, May 22, 2025 at 10:25 AM jian he <[email protected]> wrote:
>
hi.
earlier, i didn't check patch 0002.
i think in AlterFunction add
/* Lock the function so nobody else can do anything with it. */
LockDatabaseObject(ProcedureRelationId, funcOid, 0, AccessExclusiveLock);
right after
funcOid = LookupFuncWithArgs(stmt->objtype, stmt->func, false);
should be fine.
attached are some simple isolation tests for
CREATE OR REPLACE FUNCTION, ALTER FUNCTION, DROP FUNCTION.
Attachments:
[text/x-patch] v1-0001-isolation-tests-for-concurrent-change-FUNCTION-definition.patch (3.8K, ../../CACJufxECujbwxdrQ8v3QzQWOaj-KpkDCrmA=NDURH5j87=KdQg@mail.gmail.com/2-v1-0001-isolation-tests-for-concurrent-change-FUNCTION-definition.patch)
download | inline diff:
From 040c5f739dbd4e5640ba40ae7c30b86d312bd004 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Fri, 23 May 2025 10:33:39 +0800
Subject: [PATCH v1 1/1] isolation tests for concurrent change FUNCTION
definition
discussion: https://postgr.es/m/[email protected]
---
.../isolation/expected/change-function.out | 39 +++++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/change-function.spec | 29 ++++++++++++++
3 files changed, 69 insertions(+)
create mode 100644 src/test/isolation/expected/change-function.out
create mode 100644 src/test/isolation/specs/change-function.spec
diff --git a/src/test/isolation/expected/change-function.out b/src/test/isolation/expected/change-function.out
new file mode 100644
index 00000000000..812346f2fc1
--- /dev/null
+++ b/src/test/isolation/expected/change-function.out
@@ -0,0 +1,39 @@
+unused step name: r1
+Parsed test spec with 2 sessions
+
+starting permutation: b1 b2 create_f1 alter_f s1 c1 r2
+step b1: BEGIN;
+step b2: BEGIN;
+step create_f1: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$;
+step alter_f: ALTER FUNCTION f1() COST 71; <waiting ...>
+step s1: SELECT pg_get_functiondef(oid) FROM pg_proc pp WHERE proname = 'f1';
+pg_get_functiondef
+--------------------------------------------------------------------------------------------------------
+CREATE OR REPLACE FUNCTION public.f1()
+ RETURNS integer
+ LANGUAGE sql
+AS $function$ SELECT 2;$function$
+
+(1 row)
+
+step c1: COMMIT;
+step alter_f: <... completed>
+step r2: ROLLBACK;
+
+starting permutation: b1 b2 drop_f create_f1 c2 c1
+step b1: BEGIN;
+step b2: BEGIN;
+step drop_f: DROP FUNCTION f1;
+step create_f1: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$; <waiting ...>
+step c2: COMMIT;
+step create_f1: <... completed>
+step c1: COMMIT;
+
+starting permutation: b1 b2 create_f2 create_f1 c2 c1
+step b1: BEGIN;
+step b2: BEGIN;
+step create_f2: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 3;$$;
+step create_f1: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$; <waiting ...>
+step c2: COMMIT;
+step create_f1: <... completed>
+step c1: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index e3c669a29c7..99e3ce780a7 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -116,3 +116,4 @@ test: serializable-parallel-2
test: serializable-parallel-3
test: matview-write-skew
test: lock-nowait
+test: change-function
diff --git a/src/test/isolation/specs/change-function.spec b/src/test/isolation/specs/change-function.spec
new file mode 100644
index 00000000000..987e1ee7cf1
--- /dev/null
+++ b/src/test/isolation/specs/change-function.spec
@@ -0,0 +1,29 @@
+setup
+{
+ CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 1;$$;
+}
+
+teardown
+{
+ DROP FUNCTION IF EXISTS f1;
+}
+
+session "s1"
+step b1 { BEGIN;}
+step create_f1 { CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$; }
+step c1 { COMMIT; }
+step s1 { SELECT pg_get_functiondef(oid) FROM pg_proc pp WHERE proname = 'f1'; }
+step r1 { ROLLBACK; }
+
+session "s2"
+step b2 { BEGIN;}
+step alter_f { ALTER FUNCTION f1() COST 71; }
+step create_f2 { CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 3;$$; }
+step drop_f { DROP FUNCTION f1; }
+step c2 { COMMIT; }
+step r2 { ROLLBACK; }
+
+# Basic effects
+permutation b1 b2 create_f1 alter_f s1 c1 r2
+permutation b1 b2 drop_f create_f1 c2 c1
+permutation b1 b2 create_f2 create_f1 c2 c1
--
2.34.1
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-23 02:37 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
@ 2025-05-26 18:17 ` Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-05-26 18:17 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers
On Fri, 23 May 2025 10:37:42 +0800
jian he <[email protected]> wrote:
> On Thu, May 22, 2025 at 10:25 AM jian he <[email protected]> wrote:
> >
> hi.
> earlier, i didn't check patch 0002.
>
> i think in AlterFunction add
> /* Lock the function so nobody else can do anything with it. */
> LockDatabaseObject(ProcedureRelationId, funcOid, 0, AccessExclusiveLock);
>
> right after
> funcOid = LookupFuncWithArgs(stmt->objtype, stmt->func, false);
>
> should be fine.
Thank you. That makes sense because we can reduce redundant call of SearchSysCacheCopy1
and HeapTupleIsValid. I've attached a updated patch.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
@ 2025-05-26 17:35 ` Yugo Nagata <[email protected]>
2025-05-27 02:03 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
1 sibling, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-05-26 17:35 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers
Hi,
On Thu, 22 May 2025 10:25:58 +0800
jian he <[email protected]> wrote:
Thank you for looking into it.
> + /* Lock the function so nobody else can do anything with it. */
> + LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
> +
> + /*
> + * It is possible that by the time we acquire the lock on function,
> + * concurrent DDL has removed it. We can test this by checking the
> + * existence of function. We get the tuple again to avoid the risk
> + * of function definition getting changed.
> + */
> + oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
> + PointerGetDatum(procedureName),
> + PointerGetDatum(parameterTypes),
> + ObjectIdGetDatum(procNamespace));
>
> we already called LockDatabaseObject, concurrent DDL can
> not do DROP FUNCTION or ALTER FUNCTION.
> so no need to call SearchSysCacheCopy3 again?
The function may be dropped *before* we call LockDatabaseObject.
SearchSysCacheCopy3 is called for check this.
Plese see AlterPublication() as a similar code example.
>
> @@ -553,11 +575,13 @@ ProcedureCreate(const char *procedureName,
> replaces[Anum_pg_proc_proowner - 1] = false;
> replaces[Anum_pg_proc_proacl - 1] = false;
>
> +
> +
> /* Okay, do it... */
> no need to add these two new lines.
I'll remove the lines. Thanks.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-26 17:35 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-05-27 02:03 ` jian he <[email protected]>
2025-05-27 03:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: jian he @ 2025-05-27 02:03 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers
On Tue, May 27, 2025 at 1:35 AM Yugo Nagata <[email protected]> wrote:
>
> > + /* Lock the function so nobody else can do anything with it. */
> > + LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
> > +
> > + /*
> > + * It is possible that by the time we acquire the lock on function,
> > + * concurrent DDL has removed it. We can test this by checking the
> > + * existence of function. We get the tuple again to avoid the risk
> > + * of function definition getting changed.
> > + */
> > + oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
> > + PointerGetDatum(procedureName),
> > + PointerGetDatum(parameterTypes),
> > + ObjectIdGetDatum(procNamespace));
> >
> > we already called LockDatabaseObject, concurrent DDL can
> > not do DROP FUNCTION or ALTER FUNCTION.
> > so no need to call SearchSysCacheCopy3 again?
>
> The function may be dropped *before* we call LockDatabaseObject.
> SearchSysCacheCopy3 is called for check this.
> Plese see AlterPublication() as a similar code example.
>
I am wondering, can we do it the following way for v2-0001?
/* Check for pre-existing definition */
oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
PointerGetDatum(procedureName),
PointerGetDatum(parameterTypes),
ObjectIdGetDatum(procNamespace));
if (HeapTupleIsValid(oldtup))
{
/* There is one; okay to replace it? */
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same
argument types",
procedureName)));
/*
* It is possible that by the time we acquire the lock on function,
* concurrent DDL has removed it. We can test this by checking the
* existence of function. We get the tuple again to avoid the risk
* of function definition getting changed.
*/
if (!ConditionalLockDatabaseObject(ProcedureRelationId,
oldproc->oid, 0, AccessExclusiveLock))
oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
PointerGetDatum(procedureName),
PointerGetDatum(parameterTypes),
ObjectIdGetDatum(procNamespace));
}
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-26 17:35 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 02:03 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
@ 2025-05-27 03:30 ` Yugo Nagata <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-05-27 03:30 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers
On Tue, 27 May 2025 10:03:58 +0800
jian he <[email protected]> wrote:
> On Tue, May 27, 2025 at 1:35 AM Yugo Nagata <[email protected]> wrote:
> >
> > > + /* Lock the function so nobody else can do anything with it. */
> > > + LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock);
> > > +
> > > + /*
> > > + * It is possible that by the time we acquire the lock on function,
> > > + * concurrent DDL has removed it. We can test this by checking the
> > > + * existence of function. We get the tuple again to avoid the risk
> > > + * of function definition getting changed.
> > > + */
> > > + oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
> > > + PointerGetDatum(procedureName),
> > > + PointerGetDatum(parameterTypes),
> > > + ObjectIdGetDatum(procNamespace));
> > >
> > > we already called LockDatabaseObject, concurrent DDL can
> > > not do DROP FUNCTION or ALTER FUNCTION.
> > > so no need to call SearchSysCacheCopy3 again?
> >
> > The function may be dropped *before* we call LockDatabaseObject.
> > SearchSysCacheCopy3 is called for check this.
> > Plese see AlterPublication() as a similar code example.
> >
>
> I am wondering, can we do it the following way for v2-0001?
>
>
> /* Check for pre-existing definition */
> oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
> PointerGetDatum(procedureName),
> PointerGetDatum(parameterTypes),
> ObjectIdGetDatum(procNamespace));
> if (HeapTupleIsValid(oldtup))
> {
> /* There is one; okay to replace it? */
> Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
> if (!replace)
> ereport(ERROR,
> (errcode(ERRCODE_DUPLICATE_FUNCTION),
> errmsg("function \"%s\" already exists with same
> argument types",
> procedureName)));
> /*
> * It is possible that by the time we acquire the lock on function,
> * concurrent DDL has removed it. We can test this by checking the
> * existence of function. We get the tuple again to avoid the risk
> * of function definition getting changed.
> */
> if (!ConditionalLockDatabaseObject(ProcedureRelationId,
> oldproc->oid, 0, AccessExclusiveLock))
> oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
> PointerGetDatum(procedureName),
> PointerGetDatum(parameterTypes),
> ObjectIdGetDatum(procNamespace));
> }
No. This cannot prevent the error "ERROR: tuple concurrently updated"
because it doesn't wait for end of the concurrently running session
if the lock cannot be acquired.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-05-27 06:00 ` Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2 siblings, 1 reply; 41+ messages in thread
From: Alexander Lakhin @ 2025-05-27 06:00 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers; jian he <[email protected]>
Hello Yugo,
31.03.2025 14:22, Yugo Nagata wrote:
>> I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION
>> for a same function, the error "tuple concurrently updated" is raised. This is
>> an internal error output by elog, also the message is not user-friendly.
> I also found the same error is raised when concurrent ALTER FUNCTION commands are
> executed. I've added a patch to fix this in the similar way.
FWIW, the same error is raised also with concurrent GRANT/REVOKE on a
database:
https://www.postgresql.org/message-id/18dcfb7f-5deb-4487-ae22-a2c16839519a%40gmail.com
Maybe you would also find relevant this thread:
https://www.postgresql.org/message-id/flat/ZiYjn0eVc7pxVY45%40ip-10-97-1-34.eu-west-3.compute.intern...
Best regards,
Alexander Lakhin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
@ 2025-06-03 08:39 ` Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-06-03 08:39 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: pgsql-hackers; jian he <[email protected]>
On Tue, 27 May 2025 09:00:01 +0300
Alexander Lakhin <[email protected]> wrote:
> Hello Yugo,
>
> 31.03.2025 14:22, Yugo Nagata wrote:
> >> I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION
> >> for a same function, the error "tuple concurrently updated" is raised. This is
> >> an internal error output by elog, also the message is not user-friendly.
> > I also found the same error is raised when concurrent ALTER FUNCTION commands are
> > executed. I've added a patch to fix this in the similar way.
>
> FWIW, the same error is raised also with concurrent GRANT/REVOKE on a
> database:
> https://www.postgresql.org/message-id/18dcfb7f-5deb-4487-ae22-a2c16839519a%40gmail.com
>
> Maybe you would also find relevant this thread:
> https://www.postgresql.org/message-id/flat/ZiYjn0eVc7pxVY45%40ip-10-97-1-34.eu-west-3.compute.intern...
Thank you for sharing the information.
I know there are other scenarios where the same is raises and I agree that
it would be better to consider a more global solution instead of addressing
each of them. However, I am not sure that improving the error message for
each case doesn't not make sense.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-06-05 07:26 ` Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-06-05 07:26 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Tue, 3 Jun 2025 17:39:50 +0900
Yugo Nagata <[email protected]> wrote:
> On Tue, 27 May 2025 09:00:01 +0300
> Alexander Lakhin <[email protected]> wrote:
> I know there are other scenarios where the same is raises and I agree that
> it would be better to consider a more global solution instead of addressing
> each of them. However, I am not sure that improving the error message for
> each case doesn't not make sense.
To address the remaining cases where DDL commands fail with the internal
error "ERROR: tuple concurrently updated" due to insufficient locking,
I would like to propose improving the error reporting to produce a more
appropriate and user-facing error message. This should make it easier for
users to understand the cause of the failure.
Patch 0003 improves the error message shown when concurrent updates to a
system catalog tuple occur, producing output like:
ERROR: operation failed due to a concurrent command
DETAIL: Another command modified the same object in a concurrent session.
Patches 0001 and 0002 are unchanged from v2, except for updated commit messages.
I believe these patches are still useful, as they allow the operation to complete
successfully after waiting, or to behave appropriately when the target function
is dropped by another session during the wait.
Best regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-06-05 10:20 ` Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-06-05 10:20 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Thu, 5 Jun 2025 16:26:08 +0900
Yugo Nagata <[email protected]> wrote:
> On Tue, 3 Jun 2025 17:39:50 +0900
> Yugo Nagata <[email protected]> wrote:
>
> > On Tue, 27 May 2025 09:00:01 +0300
> > Alexander Lakhin <[email protected]> wrote:
>
> > I know there are other scenarios where the same is raises and I agree that
> > it would be better to consider a more global solution instead of addressing
> > each of them. However, I am not sure that improving the error message for
> > each case doesn't not make sense.
>
> To address the remaining cases where DDL commands fail with the internal
> error "ERROR: tuple concurrently updated" due to insufficient locking,
> I would like to propose improving the error reporting to produce a more
> appropriate and user-facing error message. This should make it easier for
> users to understand the cause of the failure.
>
> Patch 0003 improves the error message shown when concurrent updates to a
> system catalog tuple occur, producing output like:
>
> ERROR: operation failed due to a concurrent command
> DETAIL: Another command modified the same object in a concurrent session.
>
> Patches 0001 and 0002 are unchanged from v2, except for updated commit messages.
> I believe these patches are still useful, as they allow the operation to complete
> successfully after waiting, or to behave appropriately when the target function
> is dropped by another session during the wait.
I found that the error "tuple concurrently updated" was expected as the results
of injection_points test , so I've fixed it so that the new message is expected
instead.
I've attached updated patches.
Best regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-06-27 11:53 ` Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Daniil Davydov @ 2025-06-27 11:53 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
Hi,
On Thu, Jun 5, 2025 at 5:21 PM Yugo Nagata <[email protected]> wrote:
>
> I've attached updated patches.
>
I have some comments on v4-0001 patch :
1)
heap_freetuple should be called for every tuple that we get from
SearchSysCacheCopy3.
But if tuple is valid after the first SearchSysCacheCopy3, we
overwrite the old pointer (by the second SearchSysCacheCopy3 call) and
forget to free it.
I suggest adding heap_freetuple call before the second SearchSysCacheCopy3 call.
2)
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
Strange alignment. I guess you should keep the same alignment as in
deleted declarations.
3)
This patch fixes postgres behavior if I first create a function and
then try to CREATE OR REPLACE it in concurrent transactions.
But if the function doesn't exist and I try to call CREATE OR REPLACE
in concurrent transactions, I will get an error.
I wrote about it in this thread [1] and Tom Lane said that this
behavior is kinda expected.
Just in case, I decided to mention it here anyway - perhaps you will
have other thoughts on this matter.
[1] https://www.postgresql.org/message-id/flat/CAJDiXghv2JF5zbLyyybokWKM%2B-GYsTG%2Bhw7xseLNgJOJwf0%2B8w...
--
Best regards,
Daniil Davydov
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
@ 2025-06-30 08:47 ` Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-06-30 08:47 UTC (permalink / raw)
To: Daniil Davydov <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Fri, 27 Jun 2025 18:53:02 +0700
Daniil Davydov <[email protected]> wrote:
> Hi,
>
> On Thu, Jun 5, 2025 at 5:21 PM Yugo Nagata <[email protected]> wrote:
> >
> > I've attached updated patches.
> >
>
> I have some comments on v4-0001 patch :
Thank you for your comments!
> 1)
> heap_freetuple should be called for every tuple that we get from
> SearchSysCacheCopy3.
> But if tuple is valid after the first SearchSysCacheCopy3, we
> overwrite the old pointer (by the second SearchSysCacheCopy3 call) and
> forget to free it.
> I suggest adding heap_freetuple call before the second SearchSysCacheCopy3 call.
Good catches. Fixed.
> 2)
> + Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
> + Datum proargnames;
> + bool isnull;
> + const char *dropcmd;
> Strange alignment. I guess you should keep the same alignment as in
> deleted declarations.
Fixed.
I've attached patches including these fixes.
> 3)
> This patch fixes postgres behavior if I first create a function and
> then try to CREATE OR REPLACE it in concurrent transactions.
> But if the function doesn't exist and I try to call CREATE OR REPLACE
> in concurrent transactions, I will get an error.
> I wrote about it in this thread [1] and Tom Lane said that this
> behavior is kinda expected.
> Just in case, I decided to mention it here anyway - perhaps you will
> have other thoughts on this matter.
>
> [1] https://www.postgresql.org/message-id/flat/CAJDiXghv2JF5zbLyyybokWKM%2B-GYsTG%2Bhw7xseLNgJOJwf0%2B8w...
I agree with Tom Lane that the behavior is expected, although it would be better
if the error message were more user-friendly. We could improve it by checking the
unique constraint before calling index_insert in CatalogIndexInsert.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-06-30 11:32 ` Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Daniil Davydov @ 2025-06-30 11:32 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
Hi,
On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <[email protected]> wrote:
>
> On Fri, 27 Jun 2025 18:53:02 +0700
> Daniil Davydov <[email protected]> wrote:
> > This patch fixes postgres behavior if I first create a function and
> > then try to CREATE OR REPLACE it in concurrent transactions.
> > But if the function doesn't exist and I try to call CREATE OR REPLACE
> > in concurrent transactions, I will get an error.
> > I wrote about it in this thread [1] and Tom Lane said that this
> > behavior is kinda expected.
> > Just in case, I decided to mention it here anyway - perhaps you will
> > have other thoughts on this matter.
> >
> > [1] https://www.postgresql.org/message-id/flat/CAJDiXghv2JF5zbLyyybokWKM%2B-GYsTG%2Bhw7xseLNgJOJwf0%2B8w...
>
> I agree with Tom Lane that the behavior is expected, although it would be better
> if the error message were more user-friendly. We could improve it by checking the
> unique constraint before calling index_insert in CatalogIndexInsert.
>
As far as I understand, unique constraint checking is specific for
each index access method.
Thus, to implement the proposed idea, you will have to create a
separate callback for check_unique function.
It doesn't seem like a very neat solution, but there aren't many other
options left.
I would suggest intercepting the error (via PG_CATCH), and if it has
the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
precisely, throw another error with the desired message).
If we caught an error during the CatalogTupleInsert call, we can be
sure that the problem is in concurrent execution, because before the
insertion, we checked that such a tuple does not exist.
What do you think? And in general, are you going to fix this behavior
within this thread?
P.S.
Thank you for updating the patch.
--
Best regards,
Daniil Davydov
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
@ 2025-07-01 10:47 ` Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-07-01 10:47 UTC (permalink / raw)
To: Daniil Davydov <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Mon, 30 Jun 2025 18:32:47 +0700
Daniil Davydov <[email protected]> wrote:
> Hi,
>
> On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <[email protected]> wrote:
> >
> > On Fri, 27 Jun 2025 18:53:02 +0700
> > Daniil Davydov <[email protected]> wrote:
> > > This patch fixes postgres behavior if I first create a function and
> > > then try to CREATE OR REPLACE it in concurrent transactions.
> > > But if the function doesn't exist and I try to call CREATE OR REPLACE
> > > in concurrent transactions, I will get an error.
> > > I wrote about it in this thread [1] and Tom Lane said that this
> > > behavior is kinda expected.
> > > Just in case, I decided to mention it here anyway - perhaps you will
> > > have other thoughts on this matter.
> > >
> > > [1] https://www.postgresql.org/message-id/flat/CAJDiXghv2JF5zbLyyybokWKM%2B-GYsTG%2Bhw7xseLNgJOJwf0%2B8w...
> >
> > I agree with Tom Lane that the behavior is expected, although it would be better
> > if the error message were more user-friendly. We could improve it by checking the
> > unique constraint before calling index_insert in CatalogIndexInsert.
> >
>
> As far as I understand, unique constraint checking is specific for
> each index access method.
> Thus, to implement the proposed idea, you will have to create a
> separate callback for check_unique function.
> It doesn't seem like a very neat solution, but there aren't many other
> options left.
I believe check_exclusion_or_unique_constraint() can be used independently of
a specific index access method.
> I would suggest intercepting the error (via PG_CATCH), and if it has
> the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
> precisely, throw another error with the desired message).
> If we caught an error during the CatalogTupleInsert call, we can be
> sure that the problem is in concurrent execution, because before the
> insertion, we checked that such a tuple does not exist.
>
> What do you think? And in general, are you going to fix this behavior
> within this thread?
Initially, I wasn't planning to do so, but I gave it a try and wrote a
patch to fix the issue based on my idea.
I've attached the patch as 0004. Other patches 0001-0003 are not changed.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-07-01 11:56 ` Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Daniil Davydov @ 2025-07-01 11:56 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
Hi,
On Tue, Jul 1, 2025 at 5:47 PM Yugo Nagata <[email protected]> wrote:
>
> On Mon, 30 Jun 2025 18:32:47 +0700
> Daniil Davydov <[email protected]> wrote:
>
> > On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <[email protected]> wrote:
> > >
> > > I agree with Tom Lane that the behavior is expected, although it would be better
> > > if the error message were more user-friendly. We could improve it by checking the
> > > unique constraint before calling index_insert in CatalogIndexInsert.
> > >
> >
> > As far as I understand, unique constraint checking is specific for
> > each index access method.
> > Thus, to implement the proposed idea, you will have to create a
> > separate callback for check_unique function.
> > It doesn't seem like a very neat solution, but there aren't many other
> > options left.
>
> I believe check_exclusion_or_unique_constraint() can be used independently of
> a specific index access method.
>
> > I would suggest intercepting the error (via PG_CATCH), and if it has
> > the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
> > precisely, throw another error with the desired message).
> > If we caught an error during the CatalogTupleInsert call, we can be
> > sure that the problem is in concurrent execution, because before the
> > insertion, we checked that such a tuple does not exist.
> >
> > What do you think? And in general, are you going to fix this behavior
> > within this thread?
>
> Initially, I wasn't planning to do so, but I gave it a try and wrote a
> patch to fix the issue based on my idea.
Thanks for the patch! Some comments on it :
1)
I found two typos :
+ if (HeapTupleIsHeapOenly(heapTuple) && !onlySummarized)
and
+ sartisfied = check_unique_constraint(heapRelation,
2)
CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.
For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true.
That is why I suggested handling unique violations exactly inside
ProcedureCreate - the only place where we can be sure about reasons of
error.
--
Best regards,
Daniil Davydov
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
@ 2025-07-03 14:18 ` Yugo Nagata <[email protected]>
2025-07-03 15:06 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
0 siblings, 2 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-07-03 14:18 UTC (permalink / raw)
To: Daniil Davydov <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <[email protected]> wrote:
> Hi,
>
> On Tue, Jul 1, 2025 at 5:47 PM Yugo Nagata <[email protected]> wrote:
> >
> > On Mon, 30 Jun 2025 18:32:47 +0700
> > Daniil Davydov <[email protected]> wrote:
> >
> > > On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <[email protected]> wrote:
> > > >
> > > > I agree with Tom Lane that the behavior is expected, although it would be better
> > > > if the error message were more user-friendly. We could improve it by checking the
> > > > unique constraint before calling index_insert in CatalogIndexInsert.
> > > >
> > >
> > > As far as I understand, unique constraint checking is specific for
> > > each index access method.
> > > Thus, to implement the proposed idea, you will have to create a
> > > separate callback for check_unique function.
> > > It doesn't seem like a very neat solution, but there aren't many other
> > > options left.
> >
> > I believe check_exclusion_or_unique_constraint() can be used independently of
> > a specific index access method.
> >
> > > I would suggest intercepting the error (via PG_CATCH), and if it has
> > > the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
> > > precisely, throw another error with the desired message).
> > > If we caught an error during the CatalogTupleInsert call, we can be
> > > sure that the problem is in concurrent execution, because before the
> > > insertion, we checked that such a tuple does not exist.
> > >
> > > What do you think? And in general, are you going to fix this behavior
> > > within this thread?
> >
> > Initially, I wasn't planning to do so, but I gave it a try and wrote a
> > patch to fix the issue based on my idea.
>
> Thanks for the patch! Some comments on it :
> 1)
> I found two typos :
> + if (HeapTupleIsHeapOenly(heapTuple) && !onlySummarized)
> and
> + sartisfied = check_unique_constraint(heapRelation,
Thank you for pointing out them. Fixed.
> 2)
> CatalogIndexInsert is kinda "popular" function. It can be called in
> different situations, not in all of which a violation of unique
> constraint means an error due to competitiveness.
>
> For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> ('happy', 'sad', 'happy');"
> Will throw this error : "operation failed due to a concurrent command"
> Of course, it isn't true
You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.
In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.
I've updated the patch accordingly to reflect this direction in the error message.
ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.
However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.
> That is why I suggested handling unique violations exactly inside
> ProcedureCreate - the only place where we can be sure about reasons of
> error.
If we were to fix the error message outside of CatalogIndexInsert, we would need to
modify CatalogTupleInsert, CatalogTupleUpdate, and related functions to allow them to
report the failure appropriately.
You suggested using PG_TRY/PG_CATCH, but these do not suppress the error message from
the btree code, so this approach seems not to fully address the issue.
Moreover, the places affected are not limited to ProcedureCreate, for example,
concurrent CREATE TABLE commands can also lead to the same situation, and possibly
other commands as well. Therefore, I think it would be sufficient if the
improved message in CatalogIndexInsert makes sense on its own.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-07-03 15:06 ` Yugo Nagata <[email protected]>
1 sibling, 0 replies; 41+ messages in thread
From: Yugo Nagata @ 2025-07-03 15:06 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: Daniil Davydov <[email protected]>; pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Thu, 3 Jul 2025 23:18:12 +0900
Yugo Nagata <[email protected]> wrote:
> On Tue, 1 Jul 2025 18:56:11 +0700
> Daniil Davydov <[email protected]> wrote:
> > For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> > ('happy', 'sad', 'happy');"
> > Will throw this error : "operation failed due to a concurrent command"
> > Of course, it isn't true
>
> You're right ― this error is not caused by a concurrent command.
> However, I believe the error message in cases like creating an ENUM type with
> duplicate labels could be improved to explain the issue more clearly, rather
> than just reporting it as a unique constraint violation.
I have submitted a patch addressing this in a separate thread [1].
[1] https://www.postgresql.org/message-id/20250704000402.37e605ab0c59c300965a17ee%40sraoss.co.jp
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-07-04 03:48 ` Daniil Davydov <[email protected]>
2025-07-04 05:58 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
1 sibling, 1 reply; 41+ messages in thread
From: Daniil Davydov @ 2025-07-04 03:48 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
Hi,
On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <[email protected]> wrote:
>
> On Tue, 1 Jul 2025 18:56:11 +0700
> Daniil Davydov <[email protected]> wrote:
>
> > CatalogIndexInsert is kinda "popular" function. It can be called in
> > different situations, not in all of which a violation of unique
> > constraint means an error due to competitiveness.
> >
> > For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> > ('happy', 'sad', 'happy');"
> > Will throw this error : "operation failed due to a concurrent command"
> > Of course, it isn't true
>
> You're right — this error is not caused by a concurrent command.
> However, I believe the error message in cases like creating an ENUM type with
> duplicate labels could be improved to explain the issue more clearly, rather
> than just reporting it as a unique constraint violation.
>
> In any case, a unique constraint violation in a system catalog is not necessarily
> due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
> only cause. Instead, it should clearly report the constraint violation as the primary
> issue, and mention concurrent DDL as just one possible explanation in HINT.
>
> I've updated the patch accordingly to reflect this direction in the error message.
>
> ERROR: operation failed due to duplicate key object
> DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
> HINT: Another command might have created a object with the same key in a concurrent session.
>
> However, as a result, the message ends up being similar to the current one raised
> by the btree code, so the overall improvement in user-friendliness might be limited.
>
Thanks for updating the patch!
+1 for adding such a hint for this error.
> > That is why I suggested handling unique violations exactly inside
> > ProcedureCreate - the only place where we can be sure about reasons of
> > error.
>
> If we were to fix the error message outside of CatalogIndexInsert, we would need to
> modify CatalogTupleInsert, CatalogTupleUpdate, and related functions to allow them to
> report the failure appropriately.
>
> You suggested using PG_TRY/PG_CATCH, but these do not suppress the error message from
> the btree code, so this approach seems not to fully address the issue.
>
> Moreover, the places affected are not limited to ProcedureCreate, for example,
> concurrent CREATE TABLE commands can also lead to the same situation, and possibly
> other commands as well.
Actually, we can suppress errors from btree (by flushing error context
and creating another), but it doesn't look like the best design
decision.
It was an idea for one concrete error fix. Anyway,I like the
correction you suggested better.
--
Best regards,
Daniil Davydov
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
@ 2025-07-04 05:58 ` Yugo Nagata <[email protected]>
2025-07-17 05:09 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-07-04 05:58 UTC (permalink / raw)
To: Daniil Davydov <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Fri, 4 Jul 2025 10:48:26 +0700
Daniil Davydov <[email protected]> wrote:
> Hi,
>
> On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <[email protected]> wrote:
> >
> > On Tue, 1 Jul 2025 18:56:11 +0700
> > Daniil Davydov <[email protected]> wrote:
> >
> > > CatalogIndexInsert is kinda "popular" function. It can be called in
> > > different situations, not in all of which a violation of unique
> > > constraint means an error due to competitiveness.
> > >
> > > For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> > > ('happy', 'sad', 'happy');"
> > > Will throw this error : "operation failed due to a concurrent command"
> > > Of course, it isn't true
> >
> > You're right — this error is not caused by a concurrent command.
> > However, I believe the error message in cases like creating an ENUM type with
> > duplicate labels could be improved to explain the issue more clearly, rather
> > than just reporting it as a unique constraint violation.
> >
> > In any case, a unique constraint violation in a system catalog is not necessarily
> > due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
> > only cause. Instead, it should clearly report the constraint violation as the primary
> > issue, and mention concurrent DDL as just one possible explanation in HINT.
> >
> > I've updated the patch accordingly to reflect this direction in the error message.
> >
> > ERROR: operation failed due to duplicate key object
> > DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
> > HINT: Another command might have created a object with the same key in a concurrent session.
> >
> > However, as a result, the message ends up being similar to the current one raised
> > by the btree code, so the overall improvement in user-friendliness might be limited.
> >
>
> Thanks for updating the patch!
> +1 for adding such a hint for this error.
>
> > > That is why I suggested handling unique violations exactly inside
> > > ProcedureCreate - the only place where we can be sure about reasons of
> > > error.
> >
> > If we were to fix the error message outside of CatalogIndexInsert, we would need to
> > modify CatalogTupleInsert, CatalogTupleUpdate, and related functions to allow them to
> > report the failure appropriately.
> >
> > You suggested using PG_TRY/PG_CATCH, but these do not suppress the error message from
> > the btree code, so this approach seems not to fully address the issue.
> >
> > Moreover, the places affected are not limited to ProcedureCreate, for example,
> > concurrent CREATE TABLE commands can also lead to the same situation, and possibly
> > other commands as well.
>
> Actually, we can suppress errors from btree (by flushing error context
> and creating another),
Right. That was my misunderstanding.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-04 05:58 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-07-17 05:09 ` Yugo Nagata <[email protected]>
2025-08-20 08:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-07-17 05:09 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: Daniil Davydov <[email protected]>; pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Fri, 4 Jul 2025 14:58:05 +0900
Yugo Nagata <[email protected]> wrote:
> On Fri, 4 Jul 2025 10:48:26 +0700
> Daniil Davydov <[email protected]> wrote:
>
> > Hi,
> >
> > On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <[email protected]> wrote:
> > >
> > > On Tue, 1 Jul 2025 18:56:11 +0700
> > > Daniil Davydov <[email protected]> wrote:
> > >
> > > > CatalogIndexInsert is kinda "popular" function. It can be called in
> > > > different situations, not in all of which a violation of unique
> > > > constraint means an error due to competitiveness.
> > > >
> > > > For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> > > > ('happy', 'sad', 'happy');"
> > > > Will throw this error : "operation failed due to a concurrent command"
> > > > Of course, it isn't true
> > >
> > > You're right — this error is not caused by a concurrent command.
> > > However, I believe the error message in cases like creating an ENUM type with
> > > duplicate labels could be improved to explain the issue more clearly, rather
> > > than just reporting it as a unique constraint violation.
> > >
> > > In any case, a unique constraint violation in a system catalog is not necessarily
> > > due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
> > > only cause. Instead, it should clearly report the constraint violation as the primary
> > > issue, and mention concurrent DDL as just one possible explanation in HINT.
> > >
> > > I've updated the patch accordingly to reflect this direction in the error message.
> > >
> > > ERROR: operation failed due to duplicate key object
> > > DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
> > > HINT: Another command might have created a object with the same key in a concurrent session.
> > >
> > > However, as a result, the message ends up being similar to the current one raised
> > > by the btree code, so the overall improvement in user-friendliness might be limited.
> > >
> >
> > Thanks for updating the patch!
> > +1 for adding such a hint for this error.
I've attached updated patches since I found some test failed.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-04 05:58 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-17 05:09 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-08-20 08:01 ` Yugo Nagata <[email protected]>
2025-09-30 02:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-08-20 08:01 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: Daniil Davydov <[email protected]>; pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Thu, 17 Jul 2025 14:09:14 +0900
Yugo Nagata <[email protected]> wrote:
> On Fri, 4 Jul 2025 14:58:05 +0900
> Yugo Nagata <[email protected]> wrote:
>
> > On Fri, 4 Jul 2025 10:48:26 +0700
> > Daniil Davydov <[email protected]> wrote:
> >
> > > Hi,
> > >
> > > On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <[email protected]> wrote:
> > > >
> > > > On Tue, 1 Jul 2025 18:56:11 +0700
> > > > Daniil Davydov <[email protected]> wrote:
> > > >
> > > > > CatalogIndexInsert is kinda "popular" function. It can be called in
> > > > > different situations, not in all of which a violation of unique
> > > > > constraint means an error due to competitiveness.
> > > > >
> > > > > For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> > > > > ('happy', 'sad', 'happy');"
> > > > > Will throw this error : "operation failed due to a concurrent command"
> > > > > Of course, it isn't true
> > > >
> > > > You're right — this error is not caused by a concurrent command.
> > > > However, I believe the error message in cases like creating an ENUM type with
> > > > duplicate labels could be improved to explain the issue more clearly, rather
> > > > than just reporting it as a unique constraint violation.
> > > >
> > > > In any case, a unique constraint violation in a system catalog is not necessarily
> > > > due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
> > > > only cause. Instead, it should clearly report the constraint violation as the primary
> > > > issue, and mention concurrent DDL as just one possible explanation in HINT.
> > > >
> > > > I've updated the patch accordingly to reflect this direction in the error message.
> > > >
> > > > ERROR: operation failed due to duplicate key object
> > > > DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
> > > > HINT: Another command might have created a object with the same key in a concurrent session.
> > > >
> > > > However, as a result, the message ends up being similar to the current one raised
> > > > by the btree code, so the overall improvement in user-friendliness might be limited.
> > > >
> > >
> > > Thanks for updating the patch!
> > > +1 for adding such a hint for this error.
I've fixed the following cfbot failure:
SUMMARY: AddressSanitizer: heap-use-after-free /tmp/cirrus-ci-build/src/backend/catalog/pg_proc.c:406 in ProcedureCreate
This was caused by accessing oldproc->oid after oldtup had already been freed.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-04 05:58 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-17 05:09 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-08-20 08:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-09-30 02:01 ` Yugo Nagata <[email protected]>
2025-10-01 13:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
0 siblings, 1 reply; 41+ messages in thread
From: Yugo Nagata @ 2025-09-30 02:01 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: Daniil Davydov <[email protected]>; pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
On Wed, 20 Aug 2025 17:01:56 +0900
Yugo Nagata <[email protected]> wrote:
> On Thu, 17 Jul 2025 14:09:14 +0900
> Yugo Nagata <[email protected]> wrote:
>
> > On Fri, 4 Jul 2025 14:58:05 +0900
> > Yugo Nagata <[email protected]> wrote:
> >
> > > On Fri, 4 Jul 2025 10:48:26 +0700
> > > Daniil Davydov <[email protected]> wrote:
> > >
> > > > Hi,
> > > >
> > > > On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <[email protected]> wrote:
> > > > >
> > > > > On Tue, 1 Jul 2025 18:56:11 +0700
> > > > > Daniil Davydov <[email protected]> wrote:
> > > > >
> > > > > > CatalogIndexInsert is kinda "popular" function. It can be called in
> > > > > > different situations, not in all of which a violation of unique
> > > > > > constraint means an error due to competitiveness.
> > > > > >
> > > > > > For example, with this patch such a query : "CREATE TYPE mood AS ENUM
> > > > > > ('happy', 'sad', 'happy');"
> > > > > > Will throw this error : "operation failed due to a concurrent command"
> > > > > > Of course, it isn't true
> > > > >
> > > > > You're right — this error is not caused by a concurrent command.
> > > > > However, I believe the error message in cases like creating an ENUM type with
> > > > > duplicate labels could be improved to explain the issue more clearly, rather
> > > > > than just reporting it as a unique constraint violation.
> > > > >
> > > > > In any case, a unique constraint violation in a system catalog is not necessarily
> > > > > due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
> > > > > only cause. Instead, it should clearly report the constraint violation as the primary
> > > > > issue, and mention concurrent DDL as just one possible explanation in HINT.
> > > > >
> > > > > I've updated the patch accordingly to reflect this direction in the error message.
> > > > >
> > > > > ERROR: operation failed due to duplicate key object
> > > > > DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
> > > > > HINT: Another command might have created a object with the same key in a concurrent session.
> > > > >
> > > > > However, as a result, the message ends up being similar to the current one raised
> > > > > by the btree code, so the overall improvement in user-friendliness might be limited.
> > > > >
> > > >
> > > > Thanks for updating the patch!
> > > > +1 for adding such a hint for this error.
I've attached rebase patches.
Regards,
Yugo Nagata
--
Yugo Nagata <[email protected]>
^ permalink raw reply [nested|flat] 41+ messages in thread
* Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-04 05:58 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-17 05:09 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-08-20 08:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-09-30 02:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
@ 2025-10-01 13:30 ` Daniil Davydov <[email protected]>
0 siblings, 0 replies; 41+ messages in thread
From: Daniil Davydov @ 2025-10-01 13:30 UTC (permalink / raw)
To: Yugo Nagata <[email protected]>; +Cc: pgsql-hackers; Alexander Lakhin <[email protected]>; jian he <[email protected]>
Hi,
On Tue, Sep 30, 2025 at 9:02 AM Yugo Nagata <[email protected]> wrote:
>
> I've attached rebase patches.
>
It seems redundant to me that in CatalogIndexInsert we first check the
unique constraint, and then pass the UNIQUE_CHECK_YES parameter
to the index_insert function call below. As far as I understand, after
check_unique_constraint we can be sure that everything is okay with
inserted values. Am I missing something?
Also, why should we add "IsCatalogRelation(heapRelation)" check inside
the CatalogIndexInsert function? We know for sure that a given table is a
catalog relation.
BTW, what do you think about adding an isolation test for a concurrent
"CREATE OR REPLACE FUNCTION" command? This is one of the
'problems' we're struggling with, but I don't see this case being explicitly
tested anywhere.
All other changes look good to me.
--
Best regards,
Daniil Davydov
^ permalink raw reply [nested|flat] 41+ messages in thread
end of thread, other threads:[~2025-10-01 13:30 UTC | newest]
Thread overview: 41+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-27 07:45 [PATCH v2] Fix various shared memory estimates. Julien Rouhaud <[email protected]>
2025-03-31 09:46 [PATCH v3 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH 1/2] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v7 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v9 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v6 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v4 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v2 1/2] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v5 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v8 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 09:46 [PATCH v10 1/4] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:00 Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-03-31 11:22 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-20 15:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
2025-05-26 16:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:33 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Jim Jones <[email protected]>
2025-05-27 08:52 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-22 02:25 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-23 02:37 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-26 18:17 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-26 17:35 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 02:03 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION jian he <[email protected]>
2025-05-27 03:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-05-27 06:00 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Alexander Lakhin <[email protected]>
2025-06-03 08:39 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 07:26 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-05 10:20 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-27 11:53 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-06-30 08:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-06-30 11:32 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-01 10:47 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-01 11:56 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-03 14:18 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-03 15:06 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-04 03:48 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
2025-07-04 05:58 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-07-17 05:09 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-08-20 08:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-09-30 02:01 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Yugo Nagata <[email protected]>
2025-10-01 13:30 ` Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Daniil Davydov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox