agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 09/12] overridden vs overwritten 7+ messages / 3 participants [nested] [flat]
* [PATCH v5 09/12] overridden vs overwritten @ 2019-03-29 18:18 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Justin Pryzby @ 2019-03-29 18:18 UTC (permalink / raw) --- doc/src/sgml/ref/pgbench.sgml | 2 +- doc/src/sgml/ref/psql-ref.sgml | 2 +- src/test/regress/expected/create_table.out | 2 +- src/test/regress/sql/create_table.sql | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 5460765..f0f0a0c 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -942,7 +942,7 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <row> <entry> <literal>random_seed</literal> </entry> - <entry>random generator seed (unless overwritten with <option>-D</option>)</entry> + <entry>random generator seed (unless overridden with <option>-D</option>)</entry> </row> <row> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 8076495..7b5949b 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -478,7 +478,7 @@ EOF leave off the equal sign. To set a variable with an empty value, use the equal sign but leave off the value. These assignments are done during command line processing, so variables that reflect - connection state will get overwritten later. + connection state will be overridden later. </para> </listitem> </varlistentry> diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 262abf2..654cbfe 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -874,7 +874,7 @@ create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a no insert into parted_notnull_inh_test (b) values (null); ERROR: null value in column "b" violates not-null constraint DETAIL: Failing row contains (1, null). --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 Table "public.parted_notnull_inh_test1" Column | Type | Collation | Nullable | Default diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 9c6d86a..b2eeb34 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -720,7 +720,7 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a); create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a not null, b default 1) for values in (1); insert into parted_notnull_inh_test (b) values (null); --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 drop table parted_notnull_inh_test; -- 2.7.4 --FkmkrVfFsRoUs1wW Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0010-spelling-and-typos.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v3 07/12] overridden vs overwritten @ 2019-03-29 18:18 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Justin Pryzby @ 2019-03-29 18:18 UTC (permalink / raw) --- doc/src/sgml/ref/pgbench.sgml | 2 +- doc/src/sgml/ref/psql-ref.sgml | 2 +- src/test/regress/expected/create_table.out | 2 +- src/test/regress/sql/create_table.sql | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 54a2f9b..88bcd34 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -941,7 +941,7 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <row> <entry> <literal>random_seed</literal> </entry> - <entry>random generator seed (unless overwritten with <option>-D</option>)</entry> + <entry>random generator seed (unless overridden with <option>-D</option>)</entry> </row> <row> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 55cc78c..3bf11f0 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -478,7 +478,7 @@ EOF leave off the equal sign. To set a variable with an empty value, use the equal sign but leave off the value. These assignments are done during command line processing, so variables that reflect - connection state will get overwritten later. + connection state will be overridden later. </para> </listitem> </varlistentry> diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 204441e..3ee5e6b 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -874,7 +874,7 @@ create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a no insert into parted_notnull_inh_test (b) values (null); ERROR: null value in column "b" violates not-null constraint DETAIL: Failing row contains (1, null). --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 Table "public.parted_notnull_inh_test1" Column | Type | Collation | Nullable | Default diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 751c0d3..36222c2 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -720,7 +720,7 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a); create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a not null, b default 1) for values in (1); insert into parted_notnull_inh_test (b) values (null); --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 drop table parted_notnull_inh_test; -- 2.7.4 --cWoXeonUoKmBZSoM Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0008-Clean-up-language-in-cf984672-Improve-behavior-of.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v2 07/12] overridden vs overwritten @ 2019-03-29 18:18 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Justin Pryzby @ 2019-03-29 18:18 UTC (permalink / raw) --- doc/src/sgml/ref/pgbench.sgml | 2 +- doc/src/sgml/ref/psql-ref.sgml | 2 +- src/test/regress/expected/create_table.out | 2 +- src/test/regress/sql/create_table.sql | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index ee111b8..0334229 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -941,7 +941,7 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <row> <entry> <literal>random_seed</literal> </entry> - <entry>random generator seed (unless overwritten with <option>-D</option>)</entry> + <entry>random generator seed (unless overridden with <option>-D</option>)</entry> </row> <row> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 3ce9ef6..464c196 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -478,7 +478,7 @@ EOF leave off the equal sign. To set a variable with an empty value, use the equal sign but leave off the value. These assignments are done during command line processing, so variables that reflect - connection state will get overwritten later. + connection state will be overridden later. </para> </listitem> </varlistentry> diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index ad0cb32..44c7864 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -874,7 +874,7 @@ create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a no insert into parted_notnull_inh_test (b) values (null); ERROR: null value in column "b" violates not-null constraint DETAIL: Failing row contains (1, null). --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 Table "public.parted_notnull_inh_test1" Column | Type | Collation | Nullable | Default diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 751c0d3..36222c2 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -720,7 +720,7 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a); create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a not null, b default 1) for values in (1); insert into parted_notnull_inh_test (b) values (null); --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 drop table parted_notnull_inh_test; -- 2.1.4 --u65IjBhB3TIa72Vp Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0008-Clean-up-language-in-cf984672-Improve-behavior-of.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v1 7/8] overridden vs overwritten @ 2019-03-29 18:18 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Justin Pryzby @ 2019-03-29 18:18 UTC (permalink / raw) --- doc/src/sgml/ref/pgbench.sgml | 2 +- doc/src/sgml/ref/psql-ref.sgml | 2 +- src/test/regress/expected/create_table.out | 2 +- src/test/regress/sql/create_table.sql | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index dbeec28..d0a79b9 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -941,7 +941,7 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d <row> <entry> <literal>random_seed</literal> </entry> - <entry>random generator seed (unless overwritten with <option>-D</option>)</entry> + <entry>random generator seed (unless overridden with <option>-D</option>)</entry> </row> <row> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 2f86885..ff32981 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -478,7 +478,7 @@ EOF leave off the equal sign. To set a variable with an empty value, use the equal sign but leave off the value. These assignments are done during command line processing, so variables that reflect - connection state will get overwritten later. + connection state will be overridden later. </para> </listitem> </varlistentry> diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index ad0cb32..44c7864 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -874,7 +874,7 @@ create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a no insert into parted_notnull_inh_test (b) values (null); ERROR: null value in column "b" violates not-null constraint DETAIL: Failing row contains (1, null). --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 Table "public.parted_notnull_inh_test1" Column | Type | Collation | Nullable | Default diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 751c0d3..36222c2 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -720,7 +720,7 @@ CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10); create table parted_notnull_inh_test (a int default 1, b int not null default 0) partition by list (a); create table parted_notnull_inh_test1 partition of parted_notnull_inh_test (a not null, b default 1) for values in (1); insert into parted_notnull_inh_test (b) values (null); --- note that while b's default is overriden, a's default is preserved +-- note that while b's default is overridden, a's default is preserved \d parted_notnull_inh_test1 drop table parted_notnull_inh_test; -- 2.1.4 --IMjqdzrDRly81ofr Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0008-Clean-up-language-in-cf984672.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v9 1/4] move PG_TEMP_FILE* macros to file_utils.h @ 2023-08-22 02:05 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Nathan Bossart @ 2023-08-22 02:05 UTC (permalink / raw) --- src/backend/backup/basebackup.c | 2 +- src/backend/postmaster/postmaster.c | 1 + src/backend/storage/file/fileset.c | 1 + src/bin/pg_checksums/pg_checksums.c | 10 ---------- src/bin/pg_rewind/filemap.c | 2 +- src/include/common/file_utils.h | 4 ++++ src/include/storage/fd.h | 4 ---- 7 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index 45be21131c..5d66014499 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -25,6 +25,7 @@ #include "commands/defrem.h" #include "common/compression.h" #include "common/file_perm.h" +#include "common/file_utils.h" #include "lib/stringinfo.h" #include "miscadmin.h" #include "nodes/pg_list.h" @@ -37,7 +38,6 @@ #include "storage/bufpage.h" #include "storage/checksum.h" #include "storage/dsm_impl.h" -#include "storage/fd.h" #include "storage/ipc.h" #include "storage/reinit.h" #include "utils/builtins.h" diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index d7bfb28ff3..54e9bfb8c4 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -94,6 +94,7 @@ #include "access/xlogrecovery.h" #include "catalog/pg_control.h" #include "common/file_perm.h" +#include "common/file_utils.h" #include "common/ip.h" #include "common/pg_prng.h" #include "common/string.h" diff --git a/src/backend/storage/file/fileset.c b/src/backend/storage/file/fileset.c index e9951b0269..84cae32548 100644 --- a/src/backend/storage/file/fileset.c +++ b/src/backend/storage/file/fileset.c @@ -25,6 +25,7 @@ #include "catalog/pg_tablespace.h" #include "commands/tablespace.h" +#include "common/file_utils.h" #include "common/hashfn.h" #include "miscadmin.h" #include "storage/ipc.h" diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c index 19eb67e485..9011a19b4e 100644 --- a/src/bin/pg_checksums/pg_checksums.c +++ b/src/bin/pg_checksums/pg_checksums.c @@ -52,16 +52,6 @@ typedef enum PG_MODE_ENABLE } PgChecksumMode; -/* - * Filename components. - * - * XXX: fd.h is not declared here as frontend side code is not able to - * interact with the backend-side definitions for the various fsync - * wrappers. - */ -#define PG_TEMP_FILES_DIR "pgsql_tmp" -#define PG_TEMP_FILE_PREFIX "pgsql_tmp" - static PgChecksumMode mode = PG_MODE_CHECK; static const char *progname; diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c index bd5c598e20..58280d9abc 100644 --- a/src/bin/pg_rewind/filemap.c +++ b/src/bin/pg_rewind/filemap.c @@ -27,12 +27,12 @@ #include <unistd.h> #include "catalog/pg_tablespace_d.h" +#include "common/file_utils.h" #include "common/hashfn.h" #include "common/string.h" #include "datapagemap.h" #include "filemap.h" #include "pg_rewind.h" -#include "storage/fd.h" /* * Define a hash table which we can use to store information about the files diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h index b7efa1226d..dd1532bcb0 100644 --- a/src/include/common/file_utils.h +++ b/src/include/common/file_utils.h @@ -46,4 +46,8 @@ extern ssize_t pg_pwritev_with_retry(int fd, extern ssize_t pg_pwrite_zeros(int fd, size_t size, off_t offset); +/* Filename components */ +#define PG_TEMP_FILES_DIR "pgsql_tmp" +#define PG_TEMP_FILE_PREFIX "pgsql_tmp" + #endif /* FILE_UTILS_H */ diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 6791a406fc..aea30c0622 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -195,8 +195,4 @@ extern int durable_unlink(const char *fname, int elevel); extern void SyncDataDirectory(void); extern int data_sync_elevel(int elevel); -/* Filename components */ -#define PG_TEMP_FILES_DIR "pgsql_tmp" -#define PG_TEMP_FILE_PREFIX "pgsql_tmp" - #endif /* FD_H */ -- 2.25.1 --2fHTh5uZTiUOsy+g Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0002-make-common-enum-for-sync-methods.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH v1 2/3] Move x86 popcount code to pg_popcount_x86_64.c. @ 2026-01-14 17:54 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Nathan Bossart @ 2026-01-14 17:54 UTC (permalink / raw) This moves the SSE4.2 popcount implementations to the recently renamed file for x86-64-specific popcount code. --- src/include/port/pg_bitutils.h | 20 ++- src/port/pg_bitutils.c | 263 +------------------------------- src/port/pg_popcount_x86_64.c | 264 +++++++++++++++++++++++++++++++-- 3 files changed, 267 insertions(+), 280 deletions(-) diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h index 8ed12f7a021..c9c508d4ba3 100644 --- a/src/include/port/pg_bitutils.h +++ b/src/include/port/pg_bitutils.h @@ -307,23 +307,21 @@ pg_ceil_log2_64(uint64 num) #define POPCNT_AARCH64 1 #endif +extern int pg_popcount32_slow(uint32 word); +extern int pg_popcount64_slow(uint64 word); +extern uint64 pg_popcount_slow(const char *buf, int bytes); +extern uint64 pg_popcount_masked_slow(const char *buf, int bytes, bits8 mask); + #ifdef TRY_POPCNT_X86_64 -/* Attempt to use the POPCNT instruction, but perform a runtime check first */ +/* + * Attempt to use SSE4.2 or AVX-512 instructions, but perform a runtime check + * first. + */ extern PGDLLIMPORT int (*pg_popcount32) (uint32 word); extern PGDLLIMPORT int (*pg_popcount64) (uint64 word); extern PGDLLIMPORT uint64 (*pg_popcount_optimized) (const char *buf, int bytes); extern PGDLLIMPORT uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask); -/* - * We can also try to use the AVX-512 popcount instruction on some systems. - * The implementation of that is located in its own file. - */ -#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK -extern bool pg_popcount_avx512_available(void); -extern uint64 pg_popcount_avx512(const char *buf, int bytes); -extern uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask); -#endif - #elif POPCNT_AARCH64 /* Use the Neon version of pg_popcount{32,64} without function pointer. */ extern int pg_popcount32(uint32 word); diff --git a/src/port/pg_bitutils.c b/src/port/pg_bitutils.c index 7875bbb0f4b..9f9f90ddd4d 100644 --- a/src/port/pg_bitutils.c +++ b/src/port/pg_bitutils.c @@ -12,13 +12,6 @@ */ #include "c.h" -#ifdef HAVE__GET_CPUID -#include <cpuid.h> -#endif -#ifdef HAVE__CPUID -#include <intrin.h> -#endif - #include "port/pg_bitutils.h" @@ -103,257 +96,11 @@ const uint8 pg_number_of_ones[256] = { 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8 }; -/* - * If we are building the Neon versions, we don't need the "slow" fallbacks. - */ -#ifndef POPCNT_AARCH64 -static inline int pg_popcount32_slow(uint32 word); -static inline int pg_popcount64_slow(uint64 word); -static uint64 pg_popcount_slow(const char *buf, int bytes); -static uint64 pg_popcount_masked_slow(const char *buf, int bytes, bits8 mask); -#endif - -#ifdef TRY_POPCNT_X86_64 -static bool pg_popcount_available(void); -static int pg_popcount32_choose(uint32 word); -static int pg_popcount64_choose(uint64 word); -static uint64 pg_popcount_choose(const char *buf, int bytes); -static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask); -static inline int pg_popcount32_fast(uint32 word); -static inline int pg_popcount64_fast(uint64 word); -static uint64 pg_popcount_fast(const char *buf, int bytes); -static uint64 pg_popcount_masked_fast(const char *buf, int bytes, bits8 mask); - -int (*pg_popcount32) (uint32 word) = pg_popcount32_choose; -int (*pg_popcount64) (uint64 word) = pg_popcount64_choose; -uint64 (*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose; -uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose; -#endif /* TRY_POPCNT_X86_64 */ - -#ifdef TRY_POPCNT_X86_64 - -/* - * Return true if CPUID indicates that the POPCNT instruction is available. - */ -static bool -pg_popcount_available(void) -{ - unsigned int exx[4] = {0, 0, 0, 0}; - -#if defined(HAVE__GET_CPUID) - __get_cpuid(1, &exx[0], &exx[1], &exx[2], &exx[3]); -#elif defined(HAVE__CPUID) - __cpuid(exx, 1); -#else -#error cpuid instruction not available -#endif - - return (exx[2] & (1 << 23)) != 0; /* POPCNT */ -} - -/* - * These functions get called on the first call to pg_popcount32 etc. - * They detect whether we can use the asm implementations, and replace - * the function pointers so that subsequent calls are routed directly to - * the chosen implementation. - */ -static inline void -choose_popcount_functions(void) -{ - if (pg_popcount_available()) - { - pg_popcount32 = pg_popcount32_fast; - pg_popcount64 = pg_popcount64_fast; - pg_popcount_optimized = pg_popcount_fast; - pg_popcount_masked_optimized = pg_popcount_masked_fast; - } - else - { - pg_popcount32 = pg_popcount32_slow; - pg_popcount64 = pg_popcount64_slow; - pg_popcount_optimized = pg_popcount_slow; - pg_popcount_masked_optimized = pg_popcount_masked_slow; - } - -#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK - if (pg_popcount_avx512_available()) - { - pg_popcount_optimized = pg_popcount_avx512; - pg_popcount_masked_optimized = pg_popcount_masked_avx512; - } -#endif -} - -static int -pg_popcount32_choose(uint32 word) -{ - choose_popcount_functions(); - return pg_popcount32(word); -} - -static int -pg_popcount64_choose(uint64 word) -{ - choose_popcount_functions(); - return pg_popcount64(word); -} - -static uint64 -pg_popcount_choose(const char *buf, int bytes) -{ - choose_popcount_functions(); - return pg_popcount_optimized(buf, bytes); -} - -static uint64 -pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask) -{ - choose_popcount_functions(); - return pg_popcount_masked(buf, bytes, mask); -} - -/* - * pg_popcount32_fast - * Return the number of 1 bits set in word - */ -static inline int -pg_popcount32_fast(uint32 word) -{ -#ifdef _MSC_VER - return __popcnt(word); -#else - uint32 res; - -__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc"); - return (int) res; -#endif -} - -/* - * pg_popcount64_fast - * Return the number of 1 bits set in word - */ -static inline int -pg_popcount64_fast(uint64 word) -{ -#ifdef _MSC_VER - return __popcnt64(word); -#else - uint64 res; - -__asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc"); - return (int) res; -#endif -} - -/* - * pg_popcount_fast - * Returns the number of 1-bits in buf - */ -static uint64 -pg_popcount_fast(const char *buf, int bytes) -{ - uint64 popcnt = 0; - -#if SIZEOF_VOID_P >= 8 - /* Process in 64-bit chunks if the buffer is aligned. */ - if (buf == (const char *) TYPEALIGN(8, buf)) - { - const uint64 *words = (const uint64 *) buf; - - while (bytes >= 8) - { - popcnt += pg_popcount64_fast(*words++); - bytes -= 8; - } - - buf = (const char *) words; - } -#else - /* Process in 32-bit chunks if the buffer is aligned. */ - if (buf == (const char *) TYPEALIGN(4, buf)) - { - const uint32 *words = (const uint32 *) buf; - - while (bytes >= 4) - { - popcnt += pg_popcount32_fast(*words++); - bytes -= 4; - } - - buf = (const char *) words; - } -#endif - - /* Process any remaining bytes */ - while (bytes--) - popcnt += pg_number_of_ones[(unsigned char) *buf++]; - - return popcnt; -} - -/* - * pg_popcount_masked_fast - * Returns the number of 1-bits in buf after applying the mask to each byte - */ -static uint64 -pg_popcount_masked_fast(const char *buf, int bytes, bits8 mask) -{ - uint64 popcnt = 0; - -#if SIZEOF_VOID_P >= 8 - /* Process in 64-bit chunks if the buffer is aligned */ - uint64 maskv = ~UINT64CONST(0) / 0xFF * mask; - - if (buf == (const char *) TYPEALIGN(8, buf)) - { - const uint64 *words = (const uint64 *) buf; - - while (bytes >= 8) - { - popcnt += pg_popcount64_fast(*words++ & maskv); - bytes -= 8; - } - - buf = (const char *) words; - } -#else - /* Process in 32-bit chunks if the buffer is aligned. */ - uint32 maskv = ~((uint32) 0) / 0xFF * mask; - - if (buf == (const char *) TYPEALIGN(4, buf)) - { - const uint32 *words = (const uint32 *) buf; - - while (bytes >= 4) - { - popcnt += pg_popcount32_fast(*words++ & maskv); - bytes -= 4; - } - - buf = (const char *) words; - } -#endif - - /* Process any remaining bytes */ - while (bytes--) - popcnt += pg_number_of_ones[(unsigned char) *buf++ & mask]; - - return popcnt; -} - -#endif /* TRY_POPCNT_X86_64 */ - -/* - * If we are building the Neon versions, we don't need the "slow" fallbacks. - */ -#ifndef POPCNT_AARCH64 - /* * pg_popcount32_slow * Return the number of 1 bits set in word */ -static inline int +int pg_popcount32_slow(uint32 word) { #ifdef HAVE__BUILTIN_POPCOUNT @@ -375,7 +122,7 @@ pg_popcount32_slow(uint32 word) * pg_popcount64_slow * Return the number of 1 bits set in word */ -static inline int +int pg_popcount64_slow(uint64 word) { #ifdef HAVE__BUILTIN_POPCOUNT @@ -403,7 +150,7 @@ pg_popcount64_slow(uint64 word) * pg_popcount_slow * Returns the number of 1-bits in buf */ -static uint64 +uint64 pg_popcount_slow(const char *buf, int bytes) { uint64 popcnt = 0; @@ -449,7 +196,7 @@ pg_popcount_slow(const char *buf, int bytes) * pg_popcount_masked_slow * Returns the number of 1-bits in buf after applying the mask to each byte */ -static uint64 +uint64 pg_popcount_masked_slow(const char *buf, int bytes, bits8 mask) { uint64 popcnt = 0; @@ -495,8 +242,6 @@ pg_popcount_masked_slow(const char *buf, int bytes, bits8 mask) return popcnt; } -#endif /* ! POPCNT_AARCH64 */ - #if !defined(TRY_POPCNT_X86_64) && !defined(POPCNT_AARCH64) /* diff --git a/src/port/pg_popcount_x86_64.c b/src/port/pg_popcount_x86_64.c index 453c7a06ce9..f8643642613 100644 --- a/src/port/pg_popcount_x86_64.c +++ b/src/port/pg_popcount_x86_64.c @@ -12,26 +12,74 @@ */ #include "c.h" -#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK +#include "port/pg_bitutils.h" + +#ifdef TRY_POPCNT_X86_64 #if defined(HAVE__GET_CPUID) || defined(HAVE__GET_CPUID_COUNT) #include <cpuid.h> #endif +#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK #include <immintrin.h> +#endif #if defined(HAVE__CPUID) || defined(HAVE__CPUIDEX) #include <intrin.h> #endif -#include "port/pg_bitutils.h" +/* + * The SSE4.2 versions are built regardless of whether we are building the + * AVX-512 versions. + */ +static inline int pg_popcount32_fast(uint32 word); +static inline int pg_popcount64_fast(uint64 word); +static uint64 pg_popcount_fast(const char *buf, int bytes); +static uint64 pg_popcount_masked_fast(const char *buf, int bytes, bits8 mask); /* - * It's probably unlikely that TRY_POPCNT_X86_64 won't be set if we are able to - * use AVX-512 intrinsics, but we check it anyway to be sure. We piggy-back on - * the function pointers that are only used when TRY_POPCNT_X86_64 is set. + * These are the AVX-512 implementations of the popcount functions. */ -#ifdef TRY_POPCNT_X86_64 +#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK +static uint64 pg_popcount_avx512(const char *buf, int bytes); +static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask); +#endif /* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */ + +/* + * The function pointers are initially set to "choose" functions. These + * functions will first set the pointers to the right implementations (base on + * what the current CPU supports) and then will call the pointer to fulfill the + * caller's request. + */ +static int pg_popcount32_choose(uint32 word); +static int pg_popcount64_choose(uint64 word); +static uint64 pg_popcount_choose(const char *buf, int bytes); +static uint64 pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask); +int (*pg_popcount32) (uint32 word) = pg_popcount32_choose; +int (*pg_popcount64) (uint64 word) = pg_popcount64_choose; +uint64 (*pg_popcount_optimized) (const char *buf, int bytes) = pg_popcount_choose; +uint64 (*pg_popcount_masked_optimized) (const char *buf, int bytes, bits8 mask) = pg_popcount_masked_choose; + +/* + * Return true if CPUID indicates that the POPCNT instruction is available. + */ +static bool +pg_popcount_available(void) +{ + unsigned int exx[4] = {0, 0, 0, 0}; + +#if defined(HAVE__GET_CPUID) + __get_cpuid(1, &exx[0], &exx[1], &exx[2], &exx[3]); +#elif defined(HAVE__CPUID) + __cpuid(exx, 1); +#else +#error cpuid instruction not available +#endif + + return (exx[2] & (1 << 23)) != 0; /* POPCNT */ +} + +#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK /* * Does CPUID say there's support for XSAVE instructions? @@ -94,7 +142,7 @@ avx512_popcnt_available(void) * Returns true if the CPU supports the instructions required for the AVX-512 * pg_popcount() implementation. */ -bool +static bool pg_popcount_avx512_available(void) { return xsave_available() && @@ -102,12 +150,77 @@ pg_popcount_avx512_available(void) avx512_popcnt_available(); } +#endif /* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */ + +/* + * These functions get called on the first call to pg_popcount32 etc. + * They detect whether we can use the asm implementations, and replace + * the function pointers so that subsequent calls are routed directly to + * the chosen implementation. + */ +static inline void +choose_popcount_functions(void) +{ + if (pg_popcount_available()) + { + pg_popcount32 = pg_popcount32_fast; + pg_popcount64 = pg_popcount64_fast; + pg_popcount_optimized = pg_popcount_fast; + pg_popcount_masked_optimized = pg_popcount_masked_fast; + } + else + { + pg_popcount32 = pg_popcount32_slow; + pg_popcount64 = pg_popcount64_slow; + pg_popcount_optimized = pg_popcount_slow; + pg_popcount_masked_optimized = pg_popcount_masked_slow; + } + +#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK + if (pg_popcount_avx512_available()) + { + pg_popcount_optimized = pg_popcount_avx512; + pg_popcount_masked_optimized = pg_popcount_masked_avx512; + } +#endif +} + +static int +pg_popcount32_choose(uint32 word) +{ + choose_popcount_functions(); + return pg_popcount32(word); +} + +static int +pg_popcount64_choose(uint64 word) +{ + choose_popcount_functions(); + return pg_popcount64(word); +} + +static uint64 +pg_popcount_choose(const char *buf, int bytes) +{ + choose_popcount_functions(); + return pg_popcount_optimized(buf, bytes); +} + +static uint64 +pg_popcount_masked_choose(const char *buf, int bytes, bits8 mask) +{ + choose_popcount_functions(); + return pg_popcount_masked(buf, bytes, mask); +} + +#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK + /* * pg_popcount_avx512 * Returns the number of 1-bits in buf */ pg_attribute_target("avx512vpopcntdq,avx512bw") -uint64 +static uint64 pg_popcount_avx512(const char *buf, int bytes) { __m512i val, @@ -163,7 +276,7 @@ pg_popcount_avx512(const char *buf, int bytes) * Returns the number of 1-bits in buf after applying the mask to each byte */ pg_attribute_target("avx512vpopcntdq,avx512bw") -uint64 +static uint64 pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask) { __m512i val, @@ -219,5 +332,136 @@ pg_popcount_masked_avx512(const char *buf, int bytes, bits8 mask) return _mm512_reduce_add_epi64(accum); } -#endif /* TRY_POPCNT_X86_64 */ #endif /* USE_AVX512_POPCNT_WITH_RUNTIME_CHECK */ + +/* + * pg_popcount32_fast + * Return the number of 1 bits set in word + */ +static inline int +pg_popcount32_fast(uint32 word) +{ +#ifdef _MSC_VER + return __popcnt(word); +#else + uint32 res; + +__asm__ __volatile__(" popcntl %1,%0\n":"=q"(res):"rm"(word):"cc"); + return (int) res; +#endif +} + +/* + * pg_popcount64_fast + * Return the number of 1 bits set in word + */ +static inline int +pg_popcount64_fast(uint64 word) +{ +#ifdef _MSC_VER + return __popcnt64(word); +#else + uint64 res; + +__asm__ __volatile__(" popcntq %1,%0\n":"=q"(res):"rm"(word):"cc"); + return (int) res; +#endif +} + +/* + * pg_popcount_fast + * Returns the number of 1-bits in buf + */ +static uint64 +pg_popcount_fast(const char *buf, int bytes) +{ + uint64 popcnt = 0; + +#if SIZEOF_VOID_P >= 8 + /* Process in 64-bit chunks if the buffer is aligned. */ + if (buf == (const char *) TYPEALIGN(8, buf)) + { + const uint64 *words = (const uint64 *) buf; + + while (bytes >= 8) + { + popcnt += pg_popcount64_fast(*words++); + bytes -= 8; + } + + buf = (const char *) words; + } +#else + /* Process in 32-bit chunks if the buffer is aligned. */ + if (buf == (const char *) TYPEALIGN(4, buf)) + { + const uint32 *words = (const uint32 *) buf; + + while (bytes >= 4) + { + popcnt += pg_popcount32_fast(*words++); + bytes -= 4; + } + + buf = (const char *) words; + } +#endif + + /* Process any remaining bytes */ + while (bytes--) + popcnt += pg_number_of_ones[(unsigned char) *buf++]; + + return popcnt; +} + +/* + * pg_popcount_masked_fast + * Returns the number of 1-bits in buf after applying the mask to each byte + */ +static uint64 +pg_popcount_masked_fast(const char *buf, int bytes, bits8 mask) +{ + uint64 popcnt = 0; + +#if SIZEOF_VOID_P >= 8 + /* Process in 64-bit chunks if the buffer is aligned */ + uint64 maskv = ~UINT64CONST(0) / 0xFF * mask; + + if (buf == (const char *) TYPEALIGN(8, buf)) + { + const uint64 *words = (const uint64 *) buf; + + while (bytes >= 8) + { + popcnt += pg_popcount64_fast(*words++ & maskv); + bytes -= 8; + } + + buf = (const char *) words; + } +#else + /* Process in 32-bit chunks if the buffer is aligned. */ + uint32 maskv = ~((uint32) 0) / 0xFF * mask; + + if (buf == (const char *) TYPEALIGN(4, buf)) + { + const uint32 *words = (const uint32 *) buf; + + while (bytes >= 4) + { + popcnt += pg_popcount32_fast(*words++ & maskv); + bytes -= 4; + } + + buf = (const char *) words; + } +#endif + + /* Process any remaining bytes */ + while (bytes--) + popcnt += pg_number_of_ones[(unsigned char) *buf++ & mask]; + + return popcnt; +} + +#endif /* TRY_POPCNT_X86_64 */ -- 2.50.1 (Apple Git-155) --RzZ1MyDExwO9gtVK Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v1-0003-Rename-fast-and-slow-popcount-functions.patch ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE @ 2026-07-04 05:26 Bharath Rupireddy <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Bharath Rupireddy @ 2026-07-04 05:26 UTC (permalink / raw) To: Masahiko Sawada <[email protected]>; +Cc: shveta malik <[email protected]>; Ajin Cherian <[email protected]>; Bertrand Drouvot <[email protected]>; PostgreSQL Hackers <[email protected]> Hi, On Tue, Jun 30, 2026 at 10:38 AM Masahiko Sawada <[email protected]> wrote: > > While changes to pg_get_publication_tables() look good to me, Thanks for reviewing! > I think > the regression tests should be implemented as an isolation test rather > than a TAP test in 100_bugs.pl as it can be tested without subscribers > and the isolation test works well for this kind of race condition > tests. Agreed. The isolation test avoids the extra publisher start/stop the TAP version needed, and it's about the same amount of code. Please find the attached v9 patches. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com Attachments: [application/octet-stream] v9-0001-Fix-pg_get_publication_tables-race-with-concurren.patch (8.6K, ../../CALj2ACUhhxChS0viUYP7hGqj+L5U38UPKXxAPX-bpHraAzO5mw@mail.gmail.com/2-v9-0001-Fix-pg_get_publication_tables-race-with-concurren.patch) download | inline diff: From 23e6d0cec536c38329d7b7813340e6a6855418b2 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 4 Jul 2026 02:47:29 +0000 Subject: [PATCH] Fix pg_get_publication_tables race with concurrent DROP TABLE The pg_get_publication_tables() SRF collects table OIDs in its first call, and then opens the tables using table_open() for FOR ALL TABLES publications. If a table is dropped in between, the table_open() errors out with "could not open relation with OID". This is common in environments where many tables are being created and dropped while publication tables are being queried. The bug was introduced by b7ae03953690 in PG16. This commit fixes it by using try_table_open(), which returns NULL instead of erroring out if the relation no longer exists. We now open all tables this way regardless of whether a column list is specified, so a concurrently dropped table is consistently skipped. Such tables are simply absent from the result set, which is the expected point-in-time behavior. To skip dropped tables without emitting a row, we track the current index into the table list ourselves rather than relying on funcctx->call_cntr. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Bertrand Drouvot <[email protected]> Reviewed-by: shveta malik <[email protected]> Reviewed-by: Ajin Cherian <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVYYooWH-5tJ6cPKkU%2BmutVxwb_z4S%2BqAi-zdrFqxXE2Q%40mail.gmail.com Backpatch-through: 16 --- src/backend/catalog/pg_publication.c | 52 +++++++++++++++---- .../expected/pub-concurrent-drop.out | 16 ++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/pub-concurrent-drop.spec | 33 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/test/isolation/expected/pub-concurrent-drop.out create mode 100644 src/test/isolation/specs/pub-concurrent-drop.spec diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 1ec94c851b2..ea28ec319c5 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1424,14 +1424,27 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, bool pub_missing_ok) { #define NUM_PUBLICATION_TABLES_ELEM 4 + + /* + * State carried across SRF calls. We track the index ourselves instead of + * using funcctx->call_cntr, so that concurrently dropped tables can be + * skipped without emitting a row. + */ + typedef struct + { + List *table_infos; /* list of published_rel */ + int curr_idx; /* current index into table_infos */ + } publication_tables_state; + FuncCallContext *funcctx; - List *table_infos = NIL; + publication_tables_state *ptstate = NULL; /* stuff done only on the first call of the function */ if (SRF_IS_FIRSTCALL()) { TupleDesc tupdesc; MemoryContext oldcontext; + List *table_infos = NIL; Datum *elems; int nelems, i; @@ -1554,26 +1567,47 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, TupleDescFinalize(tupdesc); funcctx->tuple_desc = BlessTupleDesc(tupdesc); - funcctx->user_fctx = table_infos; + + /* Store the state to be used across SRF calls. */ + ptstate = palloc_object(publication_tables_state); + ptstate->table_infos = table_infos; + ptstate->curr_idx = 0; + funcctx->user_fctx = ptstate; MemoryContextSwitchTo(oldcontext); } /* stuff done on every call of the function */ funcctx = SRF_PERCALL_SETUP(); - table_infos = (List *) funcctx->user_fctx; + ptstate = (publication_tables_state *) funcctx->user_fctx; - if (funcctx->call_cntr < list_length(table_infos)) + while (ptstate->curr_idx < list_length(ptstate->table_infos)) { HeapTuple pubtuple = NULL; HeapTuple rettuple; Publication *pub; - published_rel *table_info = (published_rel *) list_nth(table_infos, funcctx->call_cntr); + published_rel *table_info = (published_rel *) list_nth(ptstate->table_infos, + ptstate->curr_idx); Oid relid = table_info->relid; - Oid schemaid = get_rel_namespace(relid); + Relation rel; + Oid schemaid; Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0}; bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0}; + /* Advance the index for the next call. */ + ptstate->curr_idx++; + + /* + * The table OIDs were collected earlier, so a table may have been + * dropped before we get here. try_table_open() returns NULL if it is + * already gone, in which case we skip it; such tables are simply + * absent from the result set, which is the expected point-in-time + * behavior. + */ + rel = try_table_open(relid, AccessShareLock); + if (rel == NULL) + continue; + /* * Form tuple with appropriate data. */ @@ -1587,6 +1621,7 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, * We don't consider row filters or column lists for FOR ALL TABLES or * FOR TABLES IN SCHEMA publications. */ + schemaid = RelationGetNamespace(rel); if (!pub->alltables && !SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP, ObjectIdGetDatum(schemaid), @@ -1616,7 +1651,6 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, /* Show all columns when the column list is not specified. */ if (nulls[2]) { - Relation rel = table_open(relid, AccessShareLock); int nattnums = 0; int16 *attnums; TupleDesc desc = RelationGetDescr(rel); @@ -1653,10 +1687,10 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, values[2] = PointerGetDatum(buildint2vector(attnums, nattnums)); nulls[2] = false; } - - table_close(rel, AccessShareLock); } + table_close(rel, AccessShareLock); + rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple)); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out new file mode 100644 index 00000000000..4fd16b1d0ca --- /dev/null +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -0,0 +1,16 @@ +Parsed test spec with 2 sessions + +starting permutation: lock query drop_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step query: + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; + <waiting ...> +step drop_and_commit: DROP TABLE pubdrop.dropme; COMMIT; +step query: <... completed> +tablename +--------- +keepme +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index b8ebe92553c..28a33cc0104 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -101,6 +101,7 @@ test: async-notify test: vacuum-no-cleanup-lock test: timeouts test: vacuum-concurrent-drop +test: pub-concurrent-drop test: vacuum-conflict test: vacuum-skip-locked test: stats diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec new file mode 100644 index 00000000000..aabe3812f3c --- /dev/null +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -0,0 +1,33 @@ +# BUG: pg_get_publication_tables() errors with "could not open relation with +# OID" when a table is dropped concurrently. + +setup +{ + CREATE SCHEMA pubdrop; + CREATE PUBLICATION pub_all FOR ALL TABLES; + CREATE TABLE pubdrop.dropme (id int); + CREATE TABLE pubdrop.keepme (id int); +} + +teardown +{ + DROP SCHEMA pubdrop CASCADE; + DROP PUBLICATION pub_all; +} + +session s1 +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that +# the pg_publication_tables query will block when it tries to open the table. +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } +# Drop the table in the lock-holding session and commit, releasing the lock. +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } + +session s2 +step query +{ + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; +} + +permutation lock query drop_and_commit diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 117e7379f10..553231aeb22 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4205,6 +4205,7 @@ pthread_mutex_t pthread_once_t pthread_t ptrdiff_t +publication_tables_state published_rel pull_var_clause_context pull_varattnos_context -- 2.47.3 [application/octet-stream] v9-0001-REL19-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch (8.6K, ../../CALj2ACUhhxChS0viUYP7hGqj+L5U38UPKXxAPX-bpHraAzO5mw@mail.gmail.com/3-v9-0001-REL19-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch) download | inline diff: From 5686bf41b8f996ffdd80ec7b05060252319d6e64 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 4 Jul 2026 02:47:29 +0000 Subject: [PATCH] Fix pg_get_publication_tables race with concurrent DROP TABLE The pg_get_publication_tables() SRF collects table OIDs in its first call, and then opens the tables using table_open() for FOR ALL TABLES publications. If a table is dropped in between, the table_open() errors out with "could not open relation with OID". This is common in environments where many tables are being created and dropped while publication tables are being queried. The bug was introduced by b7ae03953690 in PG16. This commit fixes it by using try_table_open(), which returns NULL instead of erroring out if the relation no longer exists. We now open all tables this way regardless of whether a column list is specified, so a concurrently dropped table is consistently skipped. Such tables are simply absent from the result set, which is the expected point-in-time behavior. To skip dropped tables without emitting a row, we track the current index into the table list ourselves rather than relying on funcctx->call_cntr. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Bertrand Drouvot <[email protected]> Reviewed-by: shveta malik <[email protected]> Reviewed-by: Ajin Cherian <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVYYooWH-5tJ6cPKkU%2BmutVxwb_z4S%2BqAi-zdrFqxXE2Q%40mail.gmail.com Backpatch-through: 16 --- src/backend/catalog/pg_publication.c | 52 +++++++++++++++---- .../expected/pub-concurrent-drop.out | 16 ++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/pub-concurrent-drop.spec | 33 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/test/isolation/expected/pub-concurrent-drop.out create mode 100644 src/test/isolation/specs/pub-concurrent-drop.spec diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 5c457d9aca8..e6ebc1e2627 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1414,14 +1414,27 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, bool pub_missing_ok) { #define NUM_PUBLICATION_TABLES_ELEM 4 + + /* + * State carried across SRF calls. We track the index ourselves instead of + * using funcctx->call_cntr, so that concurrently dropped tables can be + * skipped without emitting a row. + */ + typedef struct + { + List *table_infos; /* list of published_rel */ + int curr_idx; /* current index into table_infos */ + } publication_tables_state; + FuncCallContext *funcctx; - List *table_infos = NIL; + publication_tables_state *ptstate = NULL; /* stuff done only on the first call of the function */ if (SRF_IS_FIRSTCALL()) { TupleDesc tupdesc; MemoryContext oldcontext; + List *table_infos = NIL; Datum *elems; int nelems, i; @@ -1544,26 +1557,47 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, TupleDescFinalize(tupdesc); funcctx->tuple_desc = BlessTupleDesc(tupdesc); - funcctx->user_fctx = table_infos; + + /* Store the state to be used across SRF calls. */ + ptstate = palloc_object(publication_tables_state); + ptstate->table_infos = table_infos; + ptstate->curr_idx = 0; + funcctx->user_fctx = ptstate; MemoryContextSwitchTo(oldcontext); } /* stuff done on every call of the function */ funcctx = SRF_PERCALL_SETUP(); - table_infos = (List *) funcctx->user_fctx; + ptstate = (publication_tables_state *) funcctx->user_fctx; - if (funcctx->call_cntr < list_length(table_infos)) + while (ptstate->curr_idx < list_length(ptstate->table_infos)) { HeapTuple pubtuple = NULL; HeapTuple rettuple; Publication *pub; - published_rel *table_info = (published_rel *) list_nth(table_infos, funcctx->call_cntr); + published_rel *table_info = (published_rel *) list_nth(ptstate->table_infos, + ptstate->curr_idx); Oid relid = table_info->relid; - Oid schemaid = get_rel_namespace(relid); + Relation rel; + Oid schemaid; Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0}; bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0}; + /* Advance the index for the next call. */ + ptstate->curr_idx++; + + /* + * The table OIDs were collected earlier, so a table may have been + * dropped before we get here. try_table_open() returns NULL if it is + * already gone, in which case we skip it; such tables are simply + * absent from the result set, which is the expected point-in-time + * behavior. + */ + rel = try_table_open(relid, AccessShareLock); + if (rel == NULL) + continue; + /* * Form tuple with appropriate data. */ @@ -1577,6 +1611,7 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, * We don't consider row filters or column lists for FOR ALL TABLES or * FOR TABLES IN SCHEMA publications. */ + schemaid = RelationGetNamespace(rel); if (!pub->alltables && !SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP, ObjectIdGetDatum(schemaid), @@ -1606,7 +1641,6 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, /* Show all columns when the column list is not specified. */ if (nulls[2]) { - Relation rel = table_open(relid, AccessShareLock); int nattnums = 0; int16 *attnums; TupleDesc desc = RelationGetDescr(rel); @@ -1643,10 +1677,10 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, values[2] = PointerGetDatum(buildint2vector(attnums, nattnums)); nulls[2] = false; } - - table_close(rel, AccessShareLock); } + table_close(rel, AccessShareLock); + rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple)); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out new file mode 100644 index 00000000000..4fd16b1d0ca --- /dev/null +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -0,0 +1,16 @@ +Parsed test spec with 2 sessions + +starting permutation: lock query drop_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step query: + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; + <waiting ...> +step drop_and_commit: DROP TABLE pubdrop.dropme; COMMIT; +step query: <... completed> +tablename +--------- +keepme +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index b8ebe92553c..28a33cc0104 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -101,6 +101,7 @@ test: async-notify test: vacuum-no-cleanup-lock test: timeouts test: vacuum-concurrent-drop +test: pub-concurrent-drop test: vacuum-conflict test: vacuum-skip-locked test: stats diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec new file mode 100644 index 00000000000..aabe3812f3c --- /dev/null +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -0,0 +1,33 @@ +# BUG: pg_get_publication_tables() errors with "could not open relation with +# OID" when a table is dropped concurrently. + +setup +{ + CREATE SCHEMA pubdrop; + CREATE PUBLICATION pub_all FOR ALL TABLES; + CREATE TABLE pubdrop.dropme (id int); + CREATE TABLE pubdrop.keepme (id int); +} + +teardown +{ + DROP SCHEMA pubdrop CASCADE; + DROP PUBLICATION pub_all; +} + +session s1 +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that +# the pg_publication_tables query will block when it tries to open the table. +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } +# Drop the table in the lock-holding session and commit, releasing the lock. +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } + +session s2 +step query +{ + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; +} + +permutation lock query drop_and_commit diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 3a2720fb5f9..f2685154db5 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4200,6 +4200,7 @@ pthread_mutex_t pthread_once_t pthread_t ptrdiff_t +publication_tables_state published_rel pull_var_clause_context pull_varattnos_context -- 2.47.3 [application/octet-stream] v9-0001-REL18-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch (8.4K, ../../CALj2ACUhhxChS0viUYP7hGqj+L5U38UPKXxAPX-bpHraAzO5mw@mail.gmail.com/4-v9-0001-REL18-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch) download | inline diff: From 7749ffdebe1ebd7c8ddb285484c9565bf90a45cb Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 4 Jul 2026 03:08:34 +0000 Subject: [PATCH] Fix pg_get_publication_tables race with concurrent DROP TABLE The pg_get_publication_tables() SRF collects table OIDs in its first call, and then opens the tables using table_open() for FOR ALL TABLES publications. If a table is dropped in between, the table_open() errors out with "could not open relation with OID". This is common in environments where many tables are being created and dropped while publication tables are being queried. The bug was introduced by b7ae03953690 in PG16. This commit fixes it by using try_table_open(), which returns NULL instead of erroring out if the relation no longer exists. We now open all tables this way regardless of whether a column list is specified, so a concurrently dropped table is consistently skipped. Such tables are simply absent from the result set, which is the expected point-in-time behavior. To skip dropped tables without emitting a row, we track the current index into the table list ourselves rather than relying on funcctx->call_cntr. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Bertrand Drouvot <[email protected]> Reviewed-by: shveta malik <[email protected]> Reviewed-by: Ajin Cherian <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVYYooWH-5tJ6cPKkU%2BmutVxwb_z4S%2BqAi-zdrFqxXE2Q%40mail.gmail.com Backpatch-through: 16 --- src/backend/catalog/pg_publication.c | 52 +++++++++++++++---- .../expected/pub-concurrent-drop.out | 16 ++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/pub-concurrent-drop.spec | 33 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/test/isolation/expected/pub-concurrent-drop.out create mode 100644 src/test/isolation/specs/pub-concurrent-drop.spec diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index d6f94db5d99..259ecab1688 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1117,14 +1117,27 @@ Datum pg_get_publication_tables(PG_FUNCTION_ARGS) { #define NUM_PUBLICATION_TABLES_ELEM 4 + + /* + * State carried across SRF calls. We track the index ourselves instead of + * using funcctx->call_cntr, so that concurrently dropped tables can be + * skipped without emitting a row. + */ + typedef struct + { + List *table_infos; /* list of published_rel */ + int curr_idx; /* current index into table_infos */ + } publication_tables_state; + FuncCallContext *funcctx; - List *table_infos = NIL; + publication_tables_state *ptstate = NULL; /* stuff done only on the first call of the function */ if (SRF_IS_FIRSTCALL()) { TupleDesc tupdesc; MemoryContext oldcontext; + List *table_infos = NIL; ArrayType *arr; Datum *elems; int nelems, @@ -1222,26 +1235,47 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) PG_NODE_TREEOID, -1, 0); funcctx->tuple_desc = BlessTupleDesc(tupdesc); - funcctx->user_fctx = table_infos; + + /* Store the state to be used across SRF calls. */ + ptstate = palloc_object(publication_tables_state); + ptstate->table_infos = table_infos; + ptstate->curr_idx = 0; + funcctx->user_fctx = ptstate; MemoryContextSwitchTo(oldcontext); } /* stuff done on every call of the function */ funcctx = SRF_PERCALL_SETUP(); - table_infos = (List *) funcctx->user_fctx; + ptstate = (publication_tables_state *) funcctx->user_fctx; - if (funcctx->call_cntr < list_length(table_infos)) + while (ptstate->curr_idx < list_length(ptstate->table_infos)) { HeapTuple pubtuple = NULL; HeapTuple rettuple; Publication *pub; - published_rel *table_info = (published_rel *) list_nth(table_infos, funcctx->call_cntr); + published_rel *table_info = (published_rel *) list_nth(ptstate->table_infos, + ptstate->curr_idx); Oid relid = table_info->relid; - Oid schemaid = get_rel_namespace(relid); + Relation rel; + Oid schemaid; Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0}; bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0}; + /* Advance the index for the next call. */ + ptstate->curr_idx++; + + /* + * The table OIDs were collected earlier, so a table may have been + * dropped before we get here. try_table_open() returns NULL if it is + * already gone, in which case we skip it; such tables are simply + * absent from the result set, which is the expected point-in-time + * behavior. + */ + rel = try_table_open(relid, AccessShareLock); + if (rel == NULL) + continue; + /* * Form tuple with appropriate data. */ @@ -1255,6 +1289,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) * We don't consider row filters or column lists for FOR ALL TABLES or * FOR TABLES IN SCHEMA publications. */ + schemaid = RelationGetNamespace(rel); if (!pub->alltables && !SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP, ObjectIdGetDatum(schemaid), @@ -1284,7 +1319,6 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) /* Show all columns when the column list is not specified. */ if (nulls[2]) { - Relation rel = table_open(relid, AccessShareLock); int nattnums = 0; int16 *attnums; TupleDesc desc = RelationGetDescr(rel); @@ -1321,10 +1355,10 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) values[2] = PointerGetDatum(buildint2vector(attnums, nattnums)); nulls[2] = false; } - - table_close(rel, AccessShareLock); } + table_close(rel, AccessShareLock); + rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple)); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out new file mode 100644 index 00000000000..4fd16b1d0ca --- /dev/null +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -0,0 +1,16 @@ +Parsed test spec with 2 sessions + +starting permutation: lock query drop_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step query: + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; + <waiting ...> +step drop_and_commit: DROP TABLE pubdrop.dropme; COMMIT; +step query: <... completed> +tablename +--------- +keepme +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 204b9e399c6..1f5d529e212 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -95,6 +95,7 @@ test: async-notify test: vacuum-no-cleanup-lock test: timeouts test: vacuum-concurrent-drop +test: pub-concurrent-drop test: vacuum-conflict test: vacuum-skip-locked test: stats diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec new file mode 100644 index 00000000000..aabe3812f3c --- /dev/null +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -0,0 +1,33 @@ +# BUG: pg_get_publication_tables() errors with "could not open relation with +# OID" when a table is dropped concurrently. + +setup +{ + CREATE SCHEMA pubdrop; + CREATE PUBLICATION pub_all FOR ALL TABLES; + CREATE TABLE pubdrop.dropme (id int); + CREATE TABLE pubdrop.keepme (id int); +} + +teardown +{ + DROP SCHEMA pubdrop CASCADE; + DROP PUBLICATION pub_all; +} + +session s1 +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that +# the pg_publication_tables query will block when it tries to open the table. +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } +# Drop the table in the lock-holding session and commit, releasing the lock. +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } + +session s2 +step query +{ + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; +} + +permutation lock query drop_and_commit diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 8cd74c4e5b6..7c5d28d19f3 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3972,6 +3972,7 @@ pthread_mutex_t pthread_once_t pthread_t ptrdiff_t +publication_tables_state published_rel pull_var_clause_context pull_varattnos_context -- 2.47.3 [application/octet-stream] v9-0001-REL17-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch (8.4K, ../../CALj2ACUhhxChS0viUYP7hGqj+L5U38UPKXxAPX-bpHraAzO5mw@mail.gmail.com/5-v9-0001-REL17-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch) download | inline diff: From 93159d53ebfb9cb4f1883761b1028dad2c509537 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 4 Jul 2026 03:09:41 +0000 Subject: [PATCH] Fix pg_get_publication_tables race with concurrent DROP TABLE The pg_get_publication_tables() SRF collects table OIDs in its first call, and then opens the tables using table_open() for FOR ALL TABLES publications. If a table is dropped in between, the table_open() errors out with "could not open relation with OID". This is common in environments where many tables are being created and dropped while publication tables are being queried. The bug was introduced by b7ae03953690 in PG16. This commit fixes it by using try_table_open(), which returns NULL instead of erroring out if the relation no longer exists. We now open all tables this way regardless of whether a column list is specified, so a concurrently dropped table is consistently skipped. Such tables are simply absent from the result set, which is the expected point-in-time behavior. To skip dropped tables without emitting a row, we track the current index into the table list ourselves rather than relying on funcctx->call_cntr. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Bertrand Drouvot <[email protected]> Reviewed-by: shveta malik <[email protected]> Reviewed-by: Ajin Cherian <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVYYooWH-5tJ6cPKkU%2BmutVxwb_z4S%2BqAi-zdrFqxXE2Q%40mail.gmail.com Backpatch-through: 16 --- src/backend/catalog/pg_publication.c | 52 +++++++++++++++---- .../expected/pub-concurrent-drop.out | 16 ++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/pub-concurrent-drop.spec | 33 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/test/isolation/expected/pub-concurrent-drop.out create mode 100644 src/test/isolation/specs/pub-concurrent-drop.spec diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 0602398a545..0b21076dbc4 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1052,14 +1052,27 @@ Datum pg_get_publication_tables(PG_FUNCTION_ARGS) { #define NUM_PUBLICATION_TABLES_ELEM 4 + + /* + * State carried across SRF calls. We track the index ourselves instead of + * using funcctx->call_cntr, so that concurrently dropped tables can be + * skipped without emitting a row. + */ + typedef struct + { + List *table_infos; /* list of published_rel */ + int curr_idx; /* current index into table_infos */ + } publication_tables_state; + FuncCallContext *funcctx; - List *table_infos = NIL; + publication_tables_state *ptstate = NULL; /* stuff done only on the first call of the function */ if (SRF_IS_FIRSTCALL()) { TupleDesc tupdesc; MemoryContext oldcontext; + List *table_infos = NIL; ArrayType *arr; Datum *elems; int nelems, @@ -1158,26 +1171,47 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) PG_NODE_TREEOID, -1, 0); funcctx->tuple_desc = BlessTupleDesc(tupdesc); - funcctx->user_fctx = (void *) table_infos; + + /* Store the state to be used across SRF calls. */ + ptstate = palloc_object(publication_tables_state); + ptstate->table_infos = table_infos; + ptstate->curr_idx = 0; + funcctx->user_fctx = ptstate; MemoryContextSwitchTo(oldcontext); } /* stuff done on every call of the function */ funcctx = SRF_PERCALL_SETUP(); - table_infos = (List *) funcctx->user_fctx; + ptstate = (publication_tables_state *) funcctx->user_fctx; - if (funcctx->call_cntr < list_length(table_infos)) + while (ptstate->curr_idx < list_length(ptstate->table_infos)) { HeapTuple pubtuple = NULL; HeapTuple rettuple; Publication *pub; - published_rel *table_info = (published_rel *) list_nth(table_infos, funcctx->call_cntr); + published_rel *table_info = (published_rel *) list_nth(ptstate->table_infos, + ptstate->curr_idx); Oid relid = table_info->relid; - Oid schemaid = get_rel_namespace(relid); + Relation rel; + Oid schemaid; Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0}; bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0}; + /* Advance the index for the next call. */ + ptstate->curr_idx++; + + /* + * The table OIDs were collected earlier, so a table may have been + * dropped before we get here. try_table_open() returns NULL if it is + * already gone, in which case we skip it; such tables are simply + * absent from the result set, which is the expected point-in-time + * behavior. + */ + rel = try_table_open(relid, AccessShareLock); + if (rel == NULL) + continue; + /* * Form tuple with appropriate data. */ @@ -1191,6 +1225,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) * We don't consider row filters or column lists for FOR ALL TABLES or * FOR TABLES IN SCHEMA publications. */ + schemaid = RelationGetNamespace(rel); if (!pub->alltables && !SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP, ObjectIdGetDatum(schemaid), @@ -1220,7 +1255,6 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) /* Show all columns when the column list is not specified. */ if (nulls[2]) { - Relation rel = table_open(relid, AccessShareLock); int nattnums = 0; int16 *attnums; TupleDesc desc = RelationGetDescr(rel); @@ -1243,10 +1277,10 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) values[2] = PointerGetDatum(buildint2vector(attnums, nattnums)); nulls[2] = false; } - - table_close(rel, AccessShareLock); } + table_close(rel, AccessShareLock); + rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple)); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out new file mode 100644 index 00000000000..4fd16b1d0ca --- /dev/null +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -0,0 +1,16 @@ +Parsed test spec with 2 sessions + +starting permutation: lock query drop_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step query: + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; + <waiting ...> +step drop_and_commit: DROP TABLE pubdrop.dropme; COMMIT; +step query: <... completed> +tablename +--------- +keepme +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 7a4ab2ce805..06efed5279b 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -94,6 +94,7 @@ test: async-notify test: vacuum-no-cleanup-lock test: timeouts test: vacuum-concurrent-drop +test: pub-concurrent-drop test: vacuum-conflict test: vacuum-skip-locked test: stats diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec new file mode 100644 index 00000000000..aabe3812f3c --- /dev/null +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -0,0 +1,33 @@ +# BUG: pg_get_publication_tables() errors with "could not open relation with +# OID" when a table is dropped concurrently. + +setup +{ + CREATE SCHEMA pubdrop; + CREATE PUBLICATION pub_all FOR ALL TABLES; + CREATE TABLE pubdrop.dropme (id int); + CREATE TABLE pubdrop.keepme (id int); +} + +teardown +{ + DROP SCHEMA pubdrop CASCADE; + DROP PUBLICATION pub_all; +} + +session s1 +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that +# the pg_publication_tables query will block when it tries to open the table. +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } +# Drop the table in the lock-holding session and commit, releasing the lock. +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } + +session s2 +step query +{ + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; +} + +permutation lock query drop_and_commit diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index e5634ae2969..86b194da8e3 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3805,6 +3805,7 @@ pthread_mutex_t pthread_once_t pthread_t ptrdiff_t +publication_tables_state published_rel pull_var_clause_context pull_varattnos_context -- 2.47.3 [application/octet-stream] v9-0001-REL16-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch (8.4K, ../../CALj2ACUhhxChS0viUYP7hGqj+L5U38UPKXxAPX-bpHraAzO5mw@mail.gmail.com/6-v9-0001-REL16-Fix-pg_get_publication_tables-race-with-concurrent-DROP.patch) download | inline diff: From 5d56045ab1a36d6c840b6e74f44b18ad4429eb3c Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 4 Jul 2026 03:10:01 +0000 Subject: [PATCH] Fix pg_get_publication_tables race with concurrent DROP TABLE The pg_get_publication_tables() SRF collects table OIDs in its first call, and then opens the tables using table_open() for FOR ALL TABLES publications. If a table is dropped in between, the table_open() errors out with "could not open relation with OID". This is common in environments where many tables are being created and dropped while publication tables are being queried. The bug was introduced by b7ae03953690 in PG16. This commit fixes it by using try_table_open(), which returns NULL instead of erroring out if the relation no longer exists. We now open all tables this way regardless of whether a column list is specified, so a concurrently dropped table is consistently skipped. Such tables are simply absent from the result set, which is the expected point-in-time behavior. To skip dropped tables without emitting a row, we track the current index into the table list ourselves rather than relying on funcctx->call_cntr. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Bertrand Drouvot <[email protected]> Reviewed-by: shveta malik <[email protected]> Reviewed-by: Ajin Cherian <[email protected]> Reviewed-by: Masahiko Sawada <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVYYooWH-5tJ6cPKkU%2BmutVxwb_z4S%2BqAi-zdrFqxXE2Q%40mail.gmail.com Backpatch-through: 16 --- src/backend/catalog/pg_publication.c | 52 +++++++++++++++---- .../expected/pub-concurrent-drop.out | 16 ++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/pub-concurrent-drop.spec | 33 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/test/isolation/expected/pub-concurrent-drop.out create mode 100644 src/test/isolation/specs/pub-concurrent-drop.spec diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index c488b6370b6..19bfd24896b 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1057,14 +1057,27 @@ Datum pg_get_publication_tables(PG_FUNCTION_ARGS) { #define NUM_PUBLICATION_TABLES_ELEM 4 + + /* + * State carried across SRF calls. We track the index ourselves instead of + * using funcctx->call_cntr, so that concurrently dropped tables can be + * skipped without emitting a row. + */ + typedef struct + { + List *table_infos; /* list of published_rel */ + int curr_idx; /* current index into table_infos */ + } publication_tables_state; + FuncCallContext *funcctx; - List *table_infos = NIL; + publication_tables_state *ptstate = NULL; /* stuff done only on the first call of the function */ if (SRF_IS_FIRSTCALL()) { TupleDesc tupdesc; MemoryContext oldcontext; + List *table_infos = NIL; ArrayType *arr; Datum *elems; int nelems, @@ -1163,26 +1176,47 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) PG_NODE_TREEOID, -1, 0); funcctx->tuple_desc = BlessTupleDesc(tupdesc); - funcctx->user_fctx = (void *) table_infos; + + /* Store the state to be used across SRF calls. */ + ptstate = palloc_object(publication_tables_state); + ptstate->table_infos = table_infos; + ptstate->curr_idx = 0; + funcctx->user_fctx = ptstate; MemoryContextSwitchTo(oldcontext); } /* stuff done on every call of the function */ funcctx = SRF_PERCALL_SETUP(); - table_infos = (List *) funcctx->user_fctx; + ptstate = (publication_tables_state *) funcctx->user_fctx; - if (funcctx->call_cntr < list_length(table_infos)) + while (ptstate->curr_idx < list_length(ptstate->table_infos)) { HeapTuple pubtuple = NULL; HeapTuple rettuple; Publication *pub; - published_rel *table_info = (published_rel *) list_nth(table_infos, funcctx->call_cntr); + published_rel *table_info = (published_rel *) list_nth(ptstate->table_infos, + ptstate->curr_idx); Oid relid = table_info->relid; - Oid schemaid = get_rel_namespace(relid); + Relation rel; + Oid schemaid; Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0}; bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0}; + /* Advance the index for the next call. */ + ptstate->curr_idx++; + + /* + * The table OIDs were collected earlier, so a table may have been + * dropped before we get here. try_table_open() returns NULL if it is + * already gone, in which case we skip it; such tables are simply + * absent from the result set, which is the expected point-in-time + * behavior. + */ + rel = try_table_open(relid, AccessShareLock); + if (rel == NULL) + continue; + /* * Form tuple with appropriate data. */ @@ -1196,6 +1230,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) * We don't consider row filters or column lists for FOR ALL TABLES or * FOR TABLES IN SCHEMA publications. */ + schemaid = RelationGetNamespace(rel); if (!pub->alltables && !SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP, ObjectIdGetDatum(schemaid), @@ -1225,7 +1260,6 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) /* Show all columns when the column list is not specified. */ if (nulls[2]) { - Relation rel = table_open(relid, AccessShareLock); int nattnums = 0; int16 *attnums; TupleDesc desc = RelationGetDescr(rel); @@ -1248,10 +1282,10 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) values[2] = PointerGetDatum(buildint2vector(attnums, nattnums)); nulls[2] = false; } - - table_close(rel, AccessShareLock); } + table_close(rel, AccessShareLock); + rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple)); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out new file mode 100644 index 00000000000..4fd16b1d0ca --- /dev/null +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -0,0 +1,16 @@ +Parsed test spec with 2 sessions + +starting permutation: lock query drop_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step query: + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; + <waiting ...> +step drop_and_commit: DROP TABLE pubdrop.dropme; COMMIT; +step query: <... completed> +tablename +--------- +keepme +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 0b9e67347cc..e8aceb42328 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -94,6 +94,7 @@ test: async-notify test: vacuum-no-cleanup-lock test: timeouts test: vacuum-concurrent-drop +test: pub-concurrent-drop test: vacuum-conflict test: vacuum-skip-locked test: stats diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec new file mode 100644 index 00000000000..aabe3812f3c --- /dev/null +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -0,0 +1,33 @@ +# BUG: pg_get_publication_tables() errors with "could not open relation with +# OID" when a table is dropped concurrently. + +setup +{ + CREATE SCHEMA pubdrop; + CREATE PUBLICATION pub_all FOR ALL TABLES; + CREATE TABLE pubdrop.dropme (id int); + CREATE TABLE pubdrop.keepme (id int); +} + +teardown +{ + DROP SCHEMA pubdrop CASCADE; + DROP PUBLICATION pub_all; +} + +session s1 +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that +# the pg_publication_tables query will block when it tries to open the table. +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } +# Drop the table in the lock-holding session and commit, releasing the lock. +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } + +session s2 +step query +{ + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; +} + +permutation lock query drop_and_commit diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 5ee0565a209..f21ddadc4c0 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3654,6 +3654,7 @@ pthread_mutex_t pthread_once_t pthread_t ptrdiff_t +publication_tables_state published_rel pull_var_clause_context pull_varattnos_context -- 2.47.3 ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-07-04 05:26 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-03-29 18:18 [PATCH v3 07/12] overridden vs overwritten Justin Pryzby <[email protected]> 2019-03-29 18:18 [PATCH v1 7/8] overridden vs overwritten Justin Pryzby <[email protected]> 2019-03-29 18:18 [PATCH v5 09/12] overridden vs overwritten Justin Pryzby <[email protected]> 2019-03-29 18:18 [PATCH v2 07/12] overridden vs overwritten Justin Pryzby <[email protected]> 2023-08-22 02:05 [PATCH v9 1/4] move PG_TEMP_FILE* macros to file_utils.h Nathan Bossart <[email protected]> 2026-01-14 17:54 [PATCH v1 2/3] Move x86 popcount code to pg_popcount_x86_64.c. Nathan Bossart <[email protected]> 2026-07-04 05:26 Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE Bharath Rupireddy <[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