From b59f024d3ad781ccdfdfcdcbd538fb4263e18770 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Mon, 7 Aug 2023 15:12:33 -0700 Subject: [PATCH 01/91] Recalculate search_path after ALTER ROLE. Renaming a role can affect the meaning of the special string $user, so must cause search_path to be recalculated. Discussion: https://postgr.es/m/186761d32c0255debbdf50b6310b581b9c973e6c.camel@j-davis.com Reviewed-by: Nathan Bossart, Michael Paquier Backpatch-through: 11 --- src/backend/catalog/namespace.c | 6 +- .../isolation/expected/search-path-inval.out | 97 +++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/search-path-inval.spec | 59 +++++++++++ 4 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 src/test/isolation/expected/search-path-inval.out create mode 100644 src/test/isolation/specs/search-path-inval.spec diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 24011967681..a56d9e66b54 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -4777,11 +4777,15 @@ InitializeSearchPath(void) { /* * In normal mode, arrange for a callback on any syscache invalidation - * of pg_namespace rows. + * of pg_namespace or pg_authid rows. (Changing a role name may affect + * the meaning of the special string $user.) */ CacheRegisterSyscacheCallback(NAMESPACEOID, NamespaceCallback, (Datum) 0); + CacheRegisterSyscacheCallback(AUTHOID, + NamespaceCallback, + (Datum) 0); /* Force search path to be recomputed on next use */ baseSearchPathValid = false; } diff --git a/src/test/isolation/expected/search-path-inval.out b/src/test/isolation/expected/search-path-inval.out new file mode 100644 index 00000000000..e0173afdb2d --- /dev/null +++ b/src/test/isolation/expected/search-path-inval.out @@ -0,0 +1,97 @@ +Parsed test spec with 3 sessions + +starting permutation: s1a s2a s1a s2b +step s1a: + SELECT CURRENT_USER; + SHOW search_path; + SELECT t FROM x; + +current_user +---------------- +regress_sp_user1 +(1 row) + +search_path +-------------------------- +"$user", regress_sp_public +(1 row) + +t +-------------------------- +data in regress_sp_user1.x +(1 row) + +step s2a: + ALTER ROLE regress_sp_user1 RENAME TO regress_sp_user2; + +step s1a: + SELECT CURRENT_USER; + SHOW search_path; + SELECT t FROM x; + +current_user +---------------- +regress_sp_user2 +(1 row) + +search_path +-------------------------- +"$user", regress_sp_public +(1 row) + +t +--------------------------- +data in regress_sp_public.x +(1 row) + +step s2b: + ALTER ROLE regress_sp_user2 RENAME TO regress_sp_user1; + + +starting permutation: s1a s3a s1a s3b +step s1a: + SELECT CURRENT_USER; + SHOW search_path; + SELECT t FROM x; + +current_user +---------------- +regress_sp_user1 +(1 row) + +search_path +-------------------------- +"$user", regress_sp_public +(1 row) + +t +-------------------------- +data in regress_sp_user1.x +(1 row) + +step s3a: + ALTER SCHEMA regress_sp_user1 RENAME TO regress_sp_user2; + +step s1a: + SELECT CURRENT_USER; + SHOW search_path; + SELECT t FROM x; + +current_user +---------------- +regress_sp_user1 +(1 row) + +search_path +-------------------------- +"$user", regress_sp_public +(1 row) + +t +--------------------------- +data in regress_sp_public.x +(1 row) + +step s3b: + ALTER SCHEMA regress_sp_user2 RENAME TO regress_sp_user1; + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 3f12f923c02..ee787b6f74b 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -157,3 +157,4 @@ test: ao-repeatable-read-vacuum test: ao-insert-eof create_index_hot udf-insert-deadlock heap-repeatable-read ao-repeatable-read test: vacuum-ao-concurrent-drop +test: search-path-inval diff --git a/src/test/isolation/specs/search-path-inval.spec b/src/test/isolation/specs/search-path-inval.spec new file mode 100644 index 00000000000..08b1bba2fc9 --- /dev/null +++ b/src/test/isolation/specs/search-path-inval.spec @@ -0,0 +1,59 @@ +# Test search_path invalidation. + +setup +{ + CREATE USER regress_sp_user1; + CREATE SCHEMA regress_sp_user1 AUTHORIZATION regress_sp_user1; + CREATE SCHEMA regress_sp_public; + GRANT ALL PRIVILEGES ON SCHEMA regress_sp_public TO regress_sp_user1; +} + +teardown +{ + DROP SCHEMA regress_sp_public CASCADE; + DROP SCHEMA regress_sp_user1 CASCADE; + DROP USER regress_sp_user1; +} + +session s1 +setup +{ + SET search_path = "$user", regress_sp_public; + SET SESSION AUTHORIZATION regress_sp_user1; + CREATE TABLE regress_sp_user1.x(t) AS SELECT 'data in regress_sp_user1.x'; + CREATE TABLE regress_sp_public.x(t) AS SELECT 'data in regress_sp_public.x'; +} +step s1a +{ + SELECT CURRENT_USER; + SHOW search_path; + SELECT t FROM x; +} + +session s2 +step s2a +{ + ALTER ROLE regress_sp_user1 RENAME TO regress_sp_user2; +} +step s2b +{ + ALTER ROLE regress_sp_user2 RENAME TO regress_sp_user1; +} + +session s3 +step s3a +{ + ALTER SCHEMA regress_sp_user1 RENAME TO regress_sp_user2; +} +step s3b +{ + ALTER SCHEMA regress_sp_user2 RENAME TO regress_sp_user1; +} + +# s1's search_path is invalidated by role name change in s2a, and +# falls back to regress_sp_public.x +permutation s1a s2a s1a s2b + +# s1's search_path is invalidated by schema name change in s2b, and +# falls back to regress_sp_public.x +permutation s1a s3a s1a s3b From 11ce0f31290eed5bb30350ada859fe625d6fab2d Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 10 Aug 2023 10:16:59 -0700 Subject: [PATCH 02/91] Remove test from commit fa2e874946. The fix itself is fine, but the test revealed other problems related to parallel query that are not easily fixable. Remove the test for now to fix the buildfarm. Discussion: https://postgr.es/m/88825.1691665432@sss.pgh.pa.us Backpatch-through: 11 --- .../isolation/expected/search-path-inval.out | 97 ------------------- src/test/isolation/isolation_schedule | 1 - .../isolation/specs/search-path-inval.spec | 59 ----------- 3 files changed, 157 deletions(-) delete mode 100644 src/test/isolation/expected/search-path-inval.out delete mode 100644 src/test/isolation/specs/search-path-inval.spec diff --git a/src/test/isolation/expected/search-path-inval.out b/src/test/isolation/expected/search-path-inval.out deleted file mode 100644 index e0173afdb2d..00000000000 --- a/src/test/isolation/expected/search-path-inval.out +++ /dev/null @@ -1,97 +0,0 @@ -Parsed test spec with 3 sessions - -starting permutation: s1a s2a s1a s2b -step s1a: - SELECT CURRENT_USER; - SHOW search_path; - SELECT t FROM x; - -current_user ----------------- -regress_sp_user1 -(1 row) - -search_path --------------------------- -"$user", regress_sp_public -(1 row) - -t --------------------------- -data in regress_sp_user1.x -(1 row) - -step s2a: - ALTER ROLE regress_sp_user1 RENAME TO regress_sp_user2; - -step s1a: - SELECT CURRENT_USER; - SHOW search_path; - SELECT t FROM x; - -current_user ----------------- -regress_sp_user2 -(1 row) - -search_path --------------------------- -"$user", regress_sp_public -(1 row) - -t ---------------------------- -data in regress_sp_public.x -(1 row) - -step s2b: - ALTER ROLE regress_sp_user2 RENAME TO regress_sp_user1; - - -starting permutation: s1a s3a s1a s3b -step s1a: - SELECT CURRENT_USER; - SHOW search_path; - SELECT t FROM x; - -current_user ----------------- -regress_sp_user1 -(1 row) - -search_path --------------------------- -"$user", regress_sp_public -(1 row) - -t --------------------------- -data in regress_sp_user1.x -(1 row) - -step s3a: - ALTER SCHEMA regress_sp_user1 RENAME TO regress_sp_user2; - -step s1a: - SELECT CURRENT_USER; - SHOW search_path; - SELECT t FROM x; - -current_user ----------------- -regress_sp_user1 -(1 row) - -search_path --------------------------- -"$user", regress_sp_public -(1 row) - -t ---------------------------- -data in regress_sp_public.x -(1 row) - -step s3b: - ALTER SCHEMA regress_sp_user2 RENAME TO regress_sp_user1; - diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index ee787b6f74b..3f12f923c02 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -157,4 +157,3 @@ test: ao-repeatable-read-vacuum test: ao-insert-eof create_index_hot udf-insert-deadlock heap-repeatable-read ao-repeatable-read test: vacuum-ao-concurrent-drop -test: search-path-inval diff --git a/src/test/isolation/specs/search-path-inval.spec b/src/test/isolation/specs/search-path-inval.spec deleted file mode 100644 index 08b1bba2fc9..00000000000 --- a/src/test/isolation/specs/search-path-inval.spec +++ /dev/null @@ -1,59 +0,0 @@ -# Test search_path invalidation. - -setup -{ - CREATE USER regress_sp_user1; - CREATE SCHEMA regress_sp_user1 AUTHORIZATION regress_sp_user1; - CREATE SCHEMA regress_sp_public; - GRANT ALL PRIVILEGES ON SCHEMA regress_sp_public TO regress_sp_user1; -} - -teardown -{ - DROP SCHEMA regress_sp_public CASCADE; - DROP SCHEMA regress_sp_user1 CASCADE; - DROP USER regress_sp_user1; -} - -session s1 -setup -{ - SET search_path = "$user", regress_sp_public; - SET SESSION AUTHORIZATION regress_sp_user1; - CREATE TABLE regress_sp_user1.x(t) AS SELECT 'data in regress_sp_user1.x'; - CREATE TABLE regress_sp_public.x(t) AS SELECT 'data in regress_sp_public.x'; -} -step s1a -{ - SELECT CURRENT_USER; - SHOW search_path; - SELECT t FROM x; -} - -session s2 -step s2a -{ - ALTER ROLE regress_sp_user1 RENAME TO regress_sp_user2; -} -step s2b -{ - ALTER ROLE regress_sp_user2 RENAME TO regress_sp_user1; -} - -session s3 -step s3a -{ - ALTER SCHEMA regress_sp_user1 RENAME TO regress_sp_user2; -} -step s3b -{ - ALTER SCHEMA regress_sp_user2 RENAME TO regress_sp_user1; -} - -# s1's search_path is invalidated by role name change in s2a, and -# falls back to regress_sp_public.x -permutation s1a s2a s1a s2b - -# s1's search_path is invalidated by schema name change in s2b, and -# falls back to regress_sp_public.x -permutation s1a s3a s1a s3b From 361942d5a493966c8a3dace6ee83e26cd3eb97df Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Tue, 22 Aug 2023 11:57:08 -0400 Subject: [PATCH 03/91] Cache by-reference missing values in a long lived context Attribute missing values might be needed past the lifetime of the tuple descriptors from which they are extracted. To avoid possibly using pointers for by-reference values which might thus be left dangling, we cache a datumCopy'd version of the datum in the TopMemoryContext. Since we first search for the value this only needs to be done once per session for any such value. Original complaint from Tom Lane, idea for mitigation by Andrew Dunstan, tweaked by Tom Lane. Backpatch to version 11 where missing values were introduced. Discussion: https://postgr.es/m/1306569.1687978174@sss.pgh.pa.us --- src/backend/access/common/heaptuple.c | 90 ++++++++++++++++++++++++++- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index 66aee71c706..c4ad0d6c3d6 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -62,9 +62,13 @@ #include "access/heaptoast.h" #include "access/sysattr.h" #include "access/tupdesc_details.h" +#include "common/hashfn.h" #include "executor/tuptable.h" #include "foreign/foreign.h" +#include "utils/datum.h" #include "utils/expandeddatum.h" +#include "utils/hsearch.h" +#include "utils/memutils.h" #include "catalog/pg_type.h" #include "cdb/cdbvars.h" /* GpIdentity.segindex */ @@ -76,6 +80,57 @@ #define VARLENA_ATT_IS_PACKABLE(att) \ ((att)->attstorage != TYPSTORAGE_PLAIN) +/* + * Setup for cacheing pass-by-ref missing attributes in a way that survives + * tupleDesc destruction. + */ + +typedef struct +{ + int len; + Datum value; +} missing_cache_key; + +static HTAB *missing_cache = NULL; + +static uint32 +missing_hash(const void *key, Size keysize) +{ + const missing_cache_key *entry = (missing_cache_key *) key; + + return hash_bytes((const unsigned char *) entry->value, entry->len); +} + +static int +missing_match(const void *key1, const void *key2, Size keysize) +{ + const missing_cache_key *entry1 = (missing_cache_key *) key1; + const missing_cache_key *entry2 = (missing_cache_key *) key2; + + if (entry1->len != entry2->len) + return entry1->len > entry2->len ? 1 : -1; + + return memcmp(DatumGetPointer(entry1->value), + DatumGetPointer(entry2->value), + entry1->len); +} + +static void +init_missing_cache() +{ + HASHCTL hash_ctl; + + hash_ctl.keysize = sizeof(missing_cache_key); + hash_ctl.entrysize = sizeof(missing_cache_key); + hash_ctl.hcxt = TopMemoryContext; + hash_ctl.hash = missing_hash; + hash_ctl.match = missing_match; + missing_cache = + hash_create("Missing Values Cache", + 32, + &hash_ctl, + HASH_ELEM | HASH_CONTEXT | HASH_FUNCTION | HASH_COMPARE); +} /* ---------------------------------------------------------------- * misc support routines @@ -107,8 +162,41 @@ getmissingattr(TupleDesc tupleDesc, if (attrmiss->am_present) { + missing_cache_key key; + missing_cache_key *entry; + bool found; + MemoryContext oldctx; + *isnull = false; - return attrmiss->am_value; + + /* no need to cache by-value attributes */ + if (att->attbyval) + return attrmiss->am_value; + + /* set up cache if required */ + if (missing_cache == NULL) + init_missing_cache(); + + /* check if there's a cache entry */ + Assert(att->attlen > 0 || att->attlen == -1); + if (att->attlen > 0) + key.len = att->attlen; + else + key.len = VARSIZE_ANY(attrmiss->am_value); + key.value = attrmiss->am_value; + + entry = hash_search(missing_cache, &key, HASH_ENTER, &found); + + if (!found) + { + /* cache miss, so we need a non-transient copy of the datum */ + oldctx = MemoryContextSwitchTo(TopMemoryContext); + entry->value = + datumCopy(attrmiss->am_value, false, att->attlen); + MemoryContextSwitchTo(oldctx); + } + + return entry->value; } } diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 8a36b6d3dd3..c96852d624a 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3295,6 +3295,7 @@ mbstr_verifier memoize_hash memoize_iterator metastring +missing_cache_key mix_data_t mixedStruct mode_t From 3ad22a14d86871b4a2bcacff72428e46a9837af9 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 23 Aug 2023 14:13:07 +0200 Subject: [PATCH 04/91] doc: Replace list of drivers and PLs with wiki link The list of external language drivers and procedural languages was never complete or exhaustive, and rather than attempting to manage it the content has migrated to the wiki. This replaces the tables altogether with links to the wiki as we regularly get requests for adding various projects, which we reject without any clear policy for why or how the content should be managed. The threads linked to below are the most recent discussions about this, the archives contain many more. Backpatch to all supported branches since the list on the wiki applies to all branches. Author: Jonathan Katz Discussion: https://postgr.es/m/169165415312.635.10247434927885764880@wrigleys.postgresql.org Discussion: https://postgr.es/m/169177958824.635.11087800083040275266@wrigleys.postgresql.org Backpatch-through: v11 --- doc/src/sgml/external-projects.sgml | 158 ++++------------------------ 1 file changed, 18 insertions(+), 140 deletions(-) diff --git a/doc/src/sgml/external-projects.sgml b/doc/src/sgml/external-projects.sgml index bf590aba5d9..50872dfd88e 100644 --- a/doc/src/sgml/external-projects.sgml +++ b/doc/src/sgml/external-projects.sgml @@ -40,99 +40,17 @@ All other language interfaces are external projects and are distributed - separately. includes a list of - some of these projects. Note that some of these packages might not be - released under the same license as PostgreSQL. For more - information on each language interface, including licensing terms, refer to - its website and documentation. + separately. A + list of language interfaces + is maintained on the PostgreSQL wiki. Note that some of these packages are + not released under the same license as PostgreSQL. + For more information on each language interface, including licensing terms, + refer to its website and documentation. - - Externally Maintained Client Interfaces - - - - - Name - Language - Comments - Website - - - - - - DBD::Pg - Perl - Perl DBI driver - - - - - JDBC - Java - Type 4 JDBC driver - - - - - libpqxx - C++ - C++ interface - - - - - node-postgres - JavaScript - Node.js driver - - - - - Npgsql - .NET - .NET data provider - - - - - pgtcl - Tcl - - - - - - pgtclng - Tcl - - - - - - pq - Go - Pure Go driver for Go's database/sql - - - - - psqlODBC - ODBC - ODBC driver - - - - - psycopg - Python - DB API 2.0-compliant - - - - -
+ + + @@ -170,58 +88,18 @@ In addition, there are a number of procedural languages that are developed and maintained outside the core PostgreSQL - distribution. lists some of these - packages. Note that some of these projects might not be released under the same - license as PostgreSQL. For more information on each - procedural language, including licensing information, refer to its website + distribution. A list of + procedural languages + is maintained on the PostgreSQL wiki. Note that some of these projects are + not released under the same license as PostgreSQL. + For more information on each procedural language, including licensing + information, refer to its website and documentation. - - Externally Maintained Procedural Languages - - - - - Name - Language - Website - - - - - - PL/Java - Java - - - - - PL/Lua - Lua - - - - - PL/R - R - - - - - PL/sh - Unix shell - - - - - PL/v8 - JavaScript - - - - -
+ + + From 750d614f38eff02f5f8c7d1cb6381b63f57a5a09 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 24 Aug 2023 12:02:40 -0400 Subject: [PATCH 05/91] Avoid unnecessary plancache revalidation of utility statements. Revalidation of a plancache entry (after a cache invalidation event) requires acquiring a snapshot. Normally that is harmless, but not if the cached statement is one that needs to run without acquiring a snapshot. We were already aware of that for TransactionStmts, but for some reason hadn't extrapolated to the other statements that PlannedStmtRequiresSnapshot() knows mustn't set a snapshot. This can lead to unexpected failures of commands such as SET TRANSACTION ISOLATION LEVEL. We can fix it in the same way, by excluding those command types from revalidation. However, we can do even better than that: there is no need to revalidate for any statement type for which parse analysis, rewrite, and plan steps do nothing interesting, which is nearly all utility commands. To mechanize this, invent a parser function stmt_requires_parse_analysis() that tells whether parse analysis does anything beyond wrapping a CMD_UTILITY Query around the raw parse tree. If that's what it does, then rewrite and plan will just skip the Query, so that it is not possible for the same raw parse tree to produce a different plan tree after cache invalidation. stmt_requires_parse_analysis() is basically equivalent to the existing function analyze_requires_snapshot(), except that for obscure reasons that function omits ReturnStmt and CallStmt. It is unclear whether those were oversights or intentional. I have not been able to demonstrate a bug from not acquiring a snapshot while analyzing these commands, but at best it seems mighty fragile. It seems safer to acquire a snapshot for parse analysis of these commands too, which allows making stmt_requires_parse_analysis and analyze_requires_snapshot equivalent. In passing this fixes a second bug, which is that ResetPlanCache would exclude ReturnStmts and CallStmts from revalidation. That's surely *not* safe, since they contain parsable expressions. Per bug #18059 from Pavel Kulakov. Back-patch to all supported branches. Discussion: https://postgr.es/m/18059-79c692f036b25346@postgresql.org --- src/backend/utils/cache/plancache.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 3cb05f0816b..e75d80f2599 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -82,9 +82,6 @@ /* * We must skip "overhead" operations that involve database access when the - * cached plan's subject statement is a transaction control command. - * For the convenience of postgres.c, treat empty statements as control - * commands too. * cached plan's subject statement is a transaction control command or one * that requires a snapshot not to be set yet (such as SET or LOCK). More * generally, statements that do not require parse analysis/rewrite/plan @@ -95,9 +92,6 @@ ((plansource)->raw_parse_tree != NULL && \ stmt_requires_parse_analysis((plansource)->raw_parse_tree)) -#define IsTransactionStmtPlan(plansource) \ - ((plansource)->raw_parse_tree == NULL || \ - IsA((plansource)->raw_parse_tree->stmt, TransactionStmt)) /* * This is the head of the backend's list of "saved" CachedPlanSources (i.e., * those that are in long-lived storage and are examined for sinval events). From 661450745a967b97283065fe9a077448e9bc1178 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 29 Aug 2023 09:09:40 +0300 Subject: [PATCH 06/91] Initialize ListenSocket array earlier. After commit b0bea38705, syslogger prints 63 warnings about failing to close a listen socket at postmaster startup. That's because the syslogger process forks before the ListenSockets array is initialized, so ClosePostmasterPorts() calls "close(0)" 64 times. The first call succeeds, because fd 0 is stdin. This has been like this since commit 9a86f03b4e in version 13, which moved the SysLogger_Start() call to before initializing ListenSockets. We just didn't notice until commit b0bea38705 added the LOG message. Reported by Michael Paquier and Jeff Janes. Author: Michael Paquier Discussion: https://www.postgresql.org/message-id/ZOvvuQe0rdj2slA9%40paquier.xyz Discussion: https://www.postgresql.org/message-id/ZO0fgDwVw2SUJiZx@paquier.xyz#482670177eb4eaf4c9f03c1eed963e5f Backpatch-through: 13 --- src/backend/postmaster/postmaster.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 164ed03ad6e..4970153d001 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1348,6 +1348,17 @@ PostmasterMain(int argc, char *argv[]) errmsg("could not remove file \"%s\": %m", LOG_METAINFO_DATAFILE))); + /* + * Initialize input sockets. + * + * Mark them all closed, and set up an on_proc_exit function that's + * charged with closing the sockets again at postmaster shutdown. + */ + for (i = 0; i < MAXLISTEN; i++) + ListenSocket[i] = PGINVALID_SOCKET; + + on_proc_exit(CloseServerPorts, 0); + /* * If enabled, start up syslogger collection subprocess */ @@ -1382,15 +1393,7 @@ PostmasterMain(int argc, char *argv[]) /* * Establish input sockets. - * - * First, mark them all closed, and set up an on_proc_exit function that's - * charged with closing the sockets again at postmaster shutdown. */ - for (i = 0; i < MAXLISTEN; i++) - ListenSocket[i] = PGINVALID_SOCKET; - - on_proc_exit(CloseServerPorts, 0); - if (ListenAddresses) { char *rawstring; From 690131235efda1bb746ac76eb7435517f25c7501 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 30 Aug 2023 08:03:52 +0900 Subject: [PATCH 07/91] Avoid possible overflow with ltsGetFreeBlock() in logtape.c nFreeBlocks, defined as a long, stores the number of free blocks in a logical tape. ltsGetFreeBlock() has been using an int to store the value of nFreeBlocks, which could lead to overflows on platforms where long and int are not the same size (in short everything except Windows where long is 4 bytes). The problematic intermediate variable is switched to be a long instead of an int. Issue introduced by c02fdc9223015, so backpatch down to 13. Author: Ranier vilela Reviewed-by: Peter Geoghegan, David Rowley Discussion: https://postgr.es/m/CAEudQApLDWCBR_xmwNjGBrDo+f+S4E87x3s7-+hoaKqYdtC4JQ@mail.gmail.com Backpatch-through: 13 --- src/backend/utils/sort/logtape.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c index 815c9371bcf..cd85ef97944 100644 --- a/src/backend/utils/sort/logtape.c +++ b/src/backend/utils/sort/logtape.c @@ -396,7 +396,7 @@ ltsGetFreeBlock(LogicalTapeSet *lts) { long *heap = lts->freeBlocks; long blocknum; - int heapsize; + long heapsize; unsigned long pos; /* freelist empty; allocate a new block */ From 371417f1c7aff54139f3ad05686a06c8d0455bba Mon Sep 17 00:00:00 2001 From: Etsuro Fujita Date: Wed, 30 Aug 2023 17:15:05 +0900 Subject: [PATCH 08/91] postgres_fdw: Fix test for parameterized foreign scan. Commit e4106b252 should have updated this test, but did not; back-patch to all supported branches. Reviewed by Richard Guo. Discussion: http://postgr.es/m/CAPmGK15nR0NXLSCKQAcqbZbTzrzd5MozowWnTnGfPkayndF43Q%40mail.gmail.com --- contrib/postgres_fdw/expected/postgres_fdw.out | 8 ++++---- contrib/postgres_fdw/sql/postgres_fdw.sql | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 6de00516fed..caa361a8df5 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -836,10 +836,10 @@ EXPLAIN (VERBOSE, COSTS OFF) Optimizer: Postgres query optimizer (18 rows) -SELECT * FROM ft2 a, ft2 b WHERE a.c1 = 47 AND b.c1 = a.c2; - c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 -----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+-------+------------------------------+--------------------------+----+------------+----- - 47 | 7 | 00047 | Tue Feb 17 00:00:00 1970 PST | Tue Feb 17 00:00:00 1970 | 7 | 7 | foo | 7 | 7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7 | 7 | foo +SELECT * FROM "S 1"."T 1" a, ft2 b WHERE a."C 1" = 47 AND b.c1 = a.c2; + C 1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 | c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 +-----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+-------+------------------------------+--------------------------+----+------------+----- + 47 | 7 | 00047 | Tue Feb 17 00:00:00 1970 PST | Tue Feb 17 00:00:00 1970 | 7 | 7 | foo | 7 | 7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7 | 7 | foo (1 row) -- check both safe and unsafe join conditions diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 0b33965a6ac..9a77a47f4e7 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -346,7 +346,7 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c8 = 'foo'; -- can't be -- parameterized remote path for foreign table EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM "S 1"."T 1" a, ft2 b WHERE a."C 1" = 47 AND b.c1 = a.c2; -SELECT * FROM ft2 a, ft2 b WHERE a.c1 = 47 AND b.c1 = a.c2; +SELECT * FROM "S 1"."T 1" a, ft2 b WHERE a."C 1" = 47 AND b.c1 = a.c2; -- check both safe and unsafe join conditions EXPLAIN (VERBOSE, COSTS OFF) From fa31a05f1abcb5bddee61700a5bfb619bf869195 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 4 Sep 2023 14:55:53 +0900 Subject: [PATCH 09/91] Fix out-of-bound read in gtsvector_picksplit() This could lead to an imprecise choice when splitting an index page of a GiST index on a tsvector, deciding which entries should remain on the old page and which entries should move to a new page. This is wrong since tsearch2 has been moved into core with commit 140d4ebcb46e, so backpatch all the way down. This error has been spotted by valgrind. Author: Alexander Lakhin Discussion: https://postgr.es/m/17950-6c80a8d2b94ec695@postgresql.org Backpatch-through: 11 --- src/backend/utils/adt/tsgistidx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/adt/tsgistidx.c b/src/backend/utils/adt/tsgistidx.c index c09eefdda23..157cc4536bd 100644 --- a/src/backend/utils/adt/tsgistidx.c +++ b/src/backend/utils/adt/tsgistidx.c @@ -728,7 +728,7 @@ gtsvector_picksplit(PG_FUNCTION_ARGS) size_alpha = SIGLENBIT(siglen) - sizebitvec((cache[j].allistrue) ? GETSIGN(datum_l) : - GETSIGN(cache[j].sign), + cache[j].sign, siglen); } else @@ -742,7 +742,7 @@ gtsvector_picksplit(PG_FUNCTION_ARGS) size_beta = SIGLENBIT(siglen) - sizebitvec((cache[j].allistrue) ? GETSIGN(datum_r) : - GETSIGN(cache[j].sign), + cache[j].sign, siglen); } else From a4e3f9781fa34d18764eb8e7acb85c815aa90bbc Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 5 Sep 2023 13:05:27 -0400 Subject: [PATCH 10/91] doc: mention libpq regression tests Reported-by: Ryo Matsumura Discussion: https://postgr.es/m/TYCPR01MB11316B3FB56EE54D70BF0CEF6E8E4A@TYCPR01MB11316.jpnprd01.prod.outlook.com Backpatch-through: 11 --- doc/src/sgml/regress.sgml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 724ef566e76..98a702ef1af 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -194,8 +194,9 @@ make check-world -j8 >/dev/null - Regression tests for the ECPG interface library, - located in src/interfaces/ecpg/test. + Regression tests for the interface libraries, + located in src/interfaces/libpq/test and + src/interfaces/ecpg/test. From cb82456cb6a25c6a03aabdfebbf40109315c8711 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Wed, 6 Sep 2023 16:52:24 -0400 Subject: [PATCH 11/91] doc: mention that to_char() values are rounded Reported-by: barsikdacat@gmail.com Diagnosed-by: Laurenz Albe Discussion: https://postgr.es/m/168991536429.626.9957835774751337210@wrigleys.postgresql.org Author: Laurenz Albe Backpatch-through: 11 --- doc/src/sgml/func.sgml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index cb4f44e7e7f..8578c4d81aa 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -8034,6 +8034,14 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}'); + + + If the format provides fewer fractional digits than the number being + formatted, to_char() will round the number to + the specified number of fractional digits. + + + The pattern characters S, L, D, From 92ec683cf68ab3ef21738a71bc224cadb6d1142b Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 7 Sep 2023 14:12:31 +0900 Subject: [PATCH 12/91] pg_basebackup: Generate valid temporary slot names under PQbackendPID() pgbouncer can cause PQbackendPID() to return negative values due to it filling be_pid with random bytes (even these days pid_max can only be set up to 2^22 on 64b machines on Linux, for example, so this cannot happen with normal PID numbers). When this happens, pg_basebackup may generate a temporary slot name that may not be accepted by the parser, leading to spurious failures, like: pg_basebackup: error: could not send replication command ERROR: replication slot name "pg_basebackup_-1201966863" contains invalid character This commit fixes that problem by formatting the result from PQbackendPID() as an unsigned integer when creating the temporary replication slot name, so as the invalid character is gone and the command can be parsed. Author: Jelte Fennema Reviewed-by: Daniel Gustafsson, Nishant Sharma Discussion: https://postgr.es/m/CAGECzQQOGvYfp8ziF4fWQ_o8s2K7ppaoWBQnTmdakn3s-4Z=5g@mail.gmail.com Backpatch-through: 11 --- src/bin/pg_basebackup/pg_basebackup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index c7dbe5da069..f68cfd0b123 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -671,7 +671,8 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) * Create replication slot if requested */ if (temp_replication_slot && !replication_slot) - replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn)); + replication_slot = psprintf("pg_basebackup_%u", + (unsigned int) PQbackendPID(param->bgconn)); if (temp_replication_slot || create_slot) { if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, From 858c7af16b40bd02927e1029142b554d6c322193 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Fri, 8 Sep 2023 17:25:15 -0400 Subject: [PATCH 13/91] doc: remove mention of backslash doubling in strings Reported-by: Laurenz Albe Discussion: https://postgr.es/m/0b03f91a875fb44182f5bed9e1d404ed6d138066.camel@cybertec.at Author: Laurenz Albe Backpatch-through: 11 --- doc/src/sgml/syntax.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml index d66560b587c..bcb67b74592 100644 --- a/doc/src/sgml/syntax.sgml +++ b/doc/src/sgml/syntax.sgml @@ -555,7 +555,7 @@ U&'d!0061t!+000061' UESCAPE '!' While the standard syntax for specifying string constants is usually convenient, it can be difficult to understand when the desired string - contains many single quotes or backslashes, since each of those must + contains many single quotes, since each of those must be doubled. To allow more readable queries in such situations, PostgreSQL provides another way, called dollar quoting, to write string constants. From 40fefddc3a2606e035e878631715971eef17d9e2 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Wed, 6 Oct 2021 00:16:03 +0900 Subject: [PATCH 14/91] Make recovery report error message when invalid page header is found. Commit 0668719801 changed XLogPageRead() so that it validated the page header, if invalid page header was found reset the error message and retried reading the page, to fix the scenario where streaming standby got stuck at a continuation record. This change hid the error message about invalid page header, which would make it harder for users to investigate what the actual issue was found in WAL. To fix the issue, this commit makes XLogPageRead() report the error message when invalid page header is found. When not in standby mode, an invalid page header should cause recovery to end, not retry reading the page, so XLogPageRead() doesn't need to validate the page header for the retry. Instead, ReadPageInternal() should be responsible for the validation in that case. Therefore this commit changes XLogPageRead() so that if not in standby mode it doesn't validate the page header for the retry. This commit has been originally pushed as of 68601985e699 for 15 and newer versions, but not to the older branches. A recent investigation related to WAL replay failures has showed up that the lack of this patch in 12~14 is an issue, as we want to be able to improve the WAL reader to make a correct distinction between the end-of-wal and OOM cases when validating record headers. REL_11_STABLE is left out as it will be EOL'd soon. Reported-by: Yugo Nagata Author: Yugo Nagata, Kyotaro Horiguchi Reviewed-by: Ranier Vilela, Fujii Masao Discussion: https://postgr.es/m/20210718045505.32f463ed6c227111038d8ae4@sraoss.co.jp Discussion: https://postgr.es/m/17928-aa92416a70ff44a2@postgresql.org Backpatch-through: 12 --- src/backend/access/transam/xlog.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index cc465f296d8..69cbb5ee1c4 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -13287,7 +13287,7 @@ XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen, /* * Check the page header immediately, so that we can retry immediately if - * it's not valid. This may seem unnecessary, because XLogReadRecord() + * it's not valid. This may seem unnecessary, because ReadPageInternal() * validates the page header anyway, and would propagate the failure up to * ReadRecord(), which would retry. However, there's a corner case with * continuation records, if a record is split across two pages such that @@ -13311,9 +13311,23 @@ XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen, * * Validating the page header is cheap enough that doing it twice * shouldn't be a big deal from a performance point of view. + * + * When not in standby mode, an invalid page header should cause recovery + * to end, not retry reading the page, so we don't need to validate the + * page header here for the retry. Instead, ReadPageInternal() is + * responsible for the validation. */ - if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) + if (StandbyMode && + !XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf)) { + /* + * Emit this error right now then retry this page immediately. Use + * errmsg_internal() because the message was already translated. + */ + if (xlogreader->errormsg_buf[0]) + ereport(emode_for_corrupt_record(emode, EndRecPtr), + (errmsg_internal("%s", xlogreader->errormsg_buf))); + /* reset any error XLogReaderValidatePageHeader() might have set */ xlogreader->errormsg_buf[0] = '\0'; goto next_record_is_invalid; From b40ab017b9d200c37d80d11f5f9f57da39243519 Mon Sep 17 00:00:00 2001 From: Amit Kapila Date: Tue, 12 Sep 2023 10:12:51 +0530 Subject: [PATCH 15/91] Fix uninitialized access to InitialRunningXacts during decoding after ERROR. The transactions and subtransactions array that was allocated under snapshot builder memory context and recorded during decoding was not cleared in case of errors. This can result in an assertion failure if we attempt to retry logical decoding within the same session. To address this issue, we register a callback function under the snapshot builder memory context to clear the recorded transactions and subtransactions array along with the context. This problem doesn't exist in PG16 and HEAD as instead of using InitialRunningXacts, we added the list of transaction IDs and sub-transaction IDs, that have modified catalogs and are running during snapshot serialization, to the serialized snapshot (see commit 7f13ac8123). Author: Hou Zhijie Reviewed-by: Amit Kapila Backpatch-through: 11 Discussion: http://postgr.es/m/18055-ab3beed9f4b7b7d6@postgresql.org --- src/backend/replication/logical/snapbuild.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1df82bc9ddc..a5388563bea 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -300,6 +300,17 @@ static void SnapBuildWaitSnapshot(xl_running_xacts *running, TransactionId cutof static void SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn); static bool SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn); +/* + * Memory context reset callback for clearing the array of running transactions + * and subtransactions. + */ +static void +SnapBuildResetRunningXactsCallback(void *arg) +{ + NInitialRunningXacts = 0; + InitialRunningXacts = NULL; +} + /* * Allocate a new snapshot builder. * @@ -316,6 +327,7 @@ AllocateSnapshotBuilder(ReorderBuffer *reorder, MemoryContext context; MemoryContext oldcontext; SnapBuild *builder; + MemoryContextCallback *mcallback; /* allocate memory in own context, to have better accountability */ context = AllocSetContextCreate(CurrentMemoryContext, @@ -341,6 +353,10 @@ AllocateSnapshotBuilder(ReorderBuffer *reorder, builder->building_full_snapshot = need_full_snapshot; builder->initial_consistent_point = initial_consistent_point; + mcallback = palloc0(sizeof(MemoryContextCallback)); + mcallback->func = SnapBuildResetRunningXactsCallback; + MemoryContextRegisterResetCallback(CurrentMemoryContext, mcallback); + MemoryContextSwitchTo(oldcontext); /* The initial running transactions array must be empty. */ @@ -366,10 +382,6 @@ FreeSnapshotBuilder(SnapBuild *builder) /* other resources are deallocated via memory context reset */ MemoryContextDelete(context); - - /* InitialRunningXacts is freed along with the context */ - NInitialRunningXacts = 0; - InitialRunningXacts = NULL; } /* From e53625851b243c385cc671a4673e976461450648 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 14 Sep 2023 11:27:43 +1200 Subject: [PATCH 16/91] Fix incorrect logic in plan dependency recording Both 50e17ad28 and 29f45e299 mistakenly tried to record a plan dependency on a function but mistakenly inverted the OidIsValid test. This meant that we'd record a dependency only when the function's Oid was InvalidOid. Clearly this was meant to *not* record the dependency in that case. 50e17ad28 made this mistake first, then in v15 29f45e299 copied the same mistake. Reported-by: Tom Lane Backpatch-through: 14, where 50e17ad28 first made this mistake Discussion: https://postgr.es/m/2277537.1694301772@sss.pgh.pa.us --- src/backend/optimizer/plan/setrefs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 5ed6f9dacab..776f239ed4f 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -2190,7 +2190,7 @@ fix_expr_common(PlannerInfo *root, Node *node) set_sa_opfuncid(saop); record_plan_function_dependency(root, saop->opfuncid); - if (!OidIsValid(saop->hashfuncid)) + if (OidIsValid(saop->hashfuncid)) record_plan_function_dependency(root, saop->hashfuncid); } else if (IsA(node, Const)) From 2407dbe655e008b4341dc250f2ff9de3aa630e49 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 14 Sep 2023 10:30:30 +0900 Subject: [PATCH 17/91] Improve error message on snapshot import in snapmgr.c When a snapshot file fails to be read in ImportSnapshot(), it would issue an ERROR as "invalid snapshot identifier" when opening a stream for it in read-only mode. This error message is reworded to be the same as all the other messages used in this case on failure, which is useful when debugging this area. Thinko introduced by bb446b689b66 where snapshot imports have been added. A backpatch down to 11 is done as this can improve any work related to snapshot imports in older branches. Author: Bharath Rupireddy Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CALj2ACWmr=3KdxDkm8h7Zn1XxBoF6hdzq8WQyMn2y1OL5RYFrg@mail.gmail.com Backpatch-through: 11 --- src/backend/utils/time/snapmgr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 34f45d5b59b..de6d6d19597 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -1611,8 +1611,9 @@ ImportSnapshot(const char *idstr) f = AllocateFile(path, PG_BINARY_R); if (!f) ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("invalid snapshot identifier: \"%s\"", idstr))); + (errcode_for_file_access(), + errmsg("could not open file \"%s\" for reading: %m", + path))); /* get the size of the file so that we know how much memory we need */ if (fstat(fileno(f), &stat_buf)) From fe5a63f7c07829492a22796b58b3963d527837e3 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 14 Sep 2023 16:00:41 +0900 Subject: [PATCH 18/91] Revert "Improve error message on snapshot import in snapmgr.c" This reverts commit a0d87bcd9b57, following a remark from Andres Frend that the new error can be triggered with an incorrect SET TRANSACTION SNAPSHOT command without being really helpful for the user as it uses the internal file name. Discussion: https://postgr.es/m/20230914020724.hlks7vunitvtbbz4@awork3.anarazel.de Backpatch-through: 11 --- src/backend/utils/time/snapmgr.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index de6d6d19597..34f45d5b59b 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -1611,9 +1611,8 @@ ImportSnapshot(const char *idstr) f = AllocateFile(path, PG_BINARY_R); if (!f) ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not open file \"%s\" for reading: %m", - path))); + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid snapshot identifier: \"%s\"", idstr))); /* get the size of the file so that we know how much memory we need */ if (fstat(fileno(f), &stat_buf)) From 1bf557a4434c138fab4ff036a71322078d27d1d9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 18 Sep 2023 14:27:47 -0400 Subject: [PATCH 19/91] Don't crash if cursor_to_xmlschema is used on a non-data-returning Portal. cursor_to_xmlschema() assumed that any Portal must have a tupDesc, which is not so. Add a defensive check. It's plausible that this mistake occurred because of the rather poorly chosen name of the lookup function SPI_cursor_find(), which in such cases is returning something that isn't very much like a cursor. Add some documentation to try to forestall future errors of the same ilk. Report and patch by Boyu Yang (docs changes by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/dd343010-c637-434c-a8cb-418f53bda3b8.yangboyu.yby@alibaba-inc.com --- doc/src/sgml/spi.sgml | 13 +++++++++++++ src/backend/utils/adt/xml.c | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml index 6c5ee2123aa..5f466d2f0b0 100644 --- a/doc/src/sgml/spi.sgml +++ b/doc/src/sgml/spi.sgml @@ -2718,6 +2718,19 @@ Portal SPI_cursor_find(const char * name) NULL if none was found + + + Notes + + + Beware that this function can return a Portal object + that does not have cursor-like properties; for example it might not + return tuples. If you simply pass the Portal pointer + to other SPI functions, they can defend themselves against such + cases, but caution is appropriate when directly inspecting + the Portal. + + diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index aafa6203b93..06562ec930c 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -2786,6 +2786,10 @@ cursor_to_xmlschema(PG_FUNCTION_ARGS) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_CURSOR), errmsg("cursor \"%s\" does not exist", name))); + if (portal->tupDesc == NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_CURSOR_STATE), + errmsg("portal \"%s\" does not return tuples", name))); xmlschema = _SPI_strdup(map_sql_table_to_xmlschema(portal->tupDesc, InvalidOid, nulls, From cfca60cf402722a1780218aedfbb6aee089096ec Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 19 Sep 2023 11:53:51 +0300 Subject: [PATCH 20/91] Fix GiST README's explanation of the NSN cross-check. The text got the condition backwards, it's "NSN > LSN", not "NSN < LSN". While we're at it, expand it a little for clarity. Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/4cb46e18-e688-524a-0f73-b1f03ed5d6ee@iki.fi --- src/backend/access/gist/README | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/access/gist/README b/src/backend/access/gist/README index 4558a5668ba..db92b852d3c 100644 --- a/src/backend/access/gist/README +++ b/src/backend/access/gist/README @@ -270,10 +270,10 @@ should be visited too. When split inserts the downlink to the parent, it clears the F_FOLLOW_RIGHT flag in the child, and sets the NSN field in the child page header to match the LSN of the insertion on the parent. If the F_FOLLOW_RIGHT flag is not set, a scan compares the NSN on the child and the -LSN it saw in the parent. If NSN < LSN, the scan looked at the parent page -before the downlink was inserted, so it should follow the rightlink. Otherwise -the scan saw the downlink in the parent page, and will/did follow that as -usual. +LSN it saw in the parent. If the child's NSN is greater than the LSN seen on +the parent, the scan looked at the parent page before the downlink was +inserted, so it should follow the rightlink. Otherwise the scan saw the +downlink in the parent page, and will/did follow that as usual. A scan can't normally see a page with the F_FOLLOW_RIGHT flag set, because a page split keeps the child pages locked until the downlink has been inserted From 587533e05b99ab7b07f71ced821d724633b0c45c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 21 Sep 2023 23:11:31 -0400 Subject: [PATCH 21/91] Fix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions. In older branches, COMMIT/ROLLBACK AND CHAIN failed to propagate the current transaction's properties to the new transaction if there was any open subtransaction (unreleased savepoint). Instead, some previous transaction's properties would be restored. This is because the "if (s->chain)" check in CommitTransactionCommand examined the wrong instance of the "chain" flag and falsely concluded that it didn't need to save transaction properties. Our regression tests would have noticed this, except they used identical transaction properties for multiple tests in a row, so that the faulty behavior was not distinguishable from correct behavior. Commit 12d768e70 fixed the problem in v15 and later, but only rather accidentally, because I removed the "if (s->chain)" test to avoid a compiler warning, while not realizing that the warning was flagging a real bug. In v14 and before, remove the if-test and save transaction properties unconditionally; just as in the newer branches, that's not expensive enough to justify thinking harder. Add the comment and extra regression test to v15 and later to forestall any future recurrence, but there's no live bug in those branches. Patch by me, per bug #18118 from Liu Xiang. Back-patch to v12 where the AND CHAIN feature was added. Discussion: https://postgr.es/m/18118-4b72fcbb903aace6@postgresql.org --- src/backend/access/transam/xact.c | 4 +-- src/test/regress/expected/transactions.out | 40 ++++++++++++++++++++++ src/test/regress/sql/transactions.sql | 11 ++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 62ddae7a649..3aaf2ef9cb5 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3952,8 +3952,8 @@ CommitTransactionCommand(void) elog(DEBUG1,"CommitTransactionCommand: called as segment Reader in state %s", BlockStateAsString(s->blockState)); - if (s->chain) - SaveTransactionCharacteristics(); + /* Must save in case we need to restore below */ + SaveTransactionCharacteristics(); switch (s->blockState) { diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index 5dcd21665d7..8771f85dc01 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -834,6 +834,46 @@ SHOW transaction_deferrable; on (1 row) +COMMIT; +START TRANSACTION ISOLATION LEVEL READ COMMITTED, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; + transaction_isolation +----------------------- + read committed +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +SAVEPOINT x; +COMMIT AND CHAIN; -- TBLOCK_SUBCOMMIT +SHOW transaction_isolation; + transaction_isolation +----------------------- + read committed +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + COMMIT; -- different mix of options just for fun START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE; diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index 5fd3c85ed95..54e72a67841 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -497,6 +497,17 @@ SHOW transaction_read_only; SHOW transaction_deferrable; COMMIT; +START TRANSACTION ISOLATION LEVEL READ COMMITTED, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +SAVEPOINT x; +COMMIT AND CHAIN; -- TBLOCK_SUBCOMMIT +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +COMMIT; + -- different mix of options just for fun START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE; SHOW transaction_isolation; From 990ff66bda0cc03c8bb7bfb38930b6fbb1bb2f79 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 22 Sep 2023 14:52:36 -0400 Subject: [PATCH 22/91] Doc: copy-edit the introductory para for the pg_class catalog. The previous wording had a faint archaic whiff to it, and more importantly used "catalogs" as a verb, which while cutely self-referential seems likely to provoke confusion in this particular context. Also consistently use "kind" not "type" to refer to the different kinds of relations distinguished by relkind. Per gripe from Martin Nash. Back-patch to supported versions. Discussion: https://postgr.es/m/169518739902.3727338.4793815593763320945@wrigleys.postgresql.org --- doc/src/sgml/catalogs.sgml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index b8b82208a40..d160d223ac5 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1852,8 +1852,8 @@ SCRAM-SHA-256$<iteration count>:&l - The catalog pg_class catalogs tables and most - everything else that has columns or is otherwise similar to a + The catalog pg_class describes tables and + other objects that have columns or are otherwise similar to a table. This includes indexes (but see also pg_index), sequences (but see also <iteration count>:&l views, materialized views, composite types, and TOAST tables; see relkind. Below, when we mean all of these kinds of objects we speak of - relations. Not all columns are meaningful for all relation - types. + relations. Not all of pg_class's + columns are meaningful for all relation kinds. From adccaa77679700ad7a999216b1a4d58089e3c269 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 25 Sep 2023 14:34:06 +0200 Subject: [PATCH 23/91] pg_upgrade: check for types removed in pg12 Commit cda6a8d01d39 removed a few datatypes, but didn't update pg_upgrade --check to throw error if these types are used. So the users find that pg_upgrade --check tells them that everything is fine, only to fail when the real upgrade is attempted. Reviewed-by: Tristan Partin Reviewed-by: Suraj Kharage Discussion: https://postgr.es/m/202309201654.ng4ksea25mti@alvherre.pgsql --- src/bin/pg_upgrade/check.c | 44 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 456d13c28a3..80a8f251126 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -131,6 +131,16 @@ check_and_dump_old_cluster(bool live_check, char **sequence_script_file_name) /* GPDB 7 removed support for SHA-256 hashed passwords */ if (GET_MAJOR_VERSION(old_cluster.major_version) <= 905) old_GPDB6_check_for_unsupported_sha256_password_hashes(); + /* + * PG 12 removed types abstime, reltime, tinterval. + */ + if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1100) + { + check_for_removed_data_type_usage(&old_cluster, "12", "abstime"); + check_for_removed_data_type_usage(&old_cluster, "12", "reltime"); + check_for_removed_data_type_usage(&old_cluster, "12", "tinterval"); + } + /* * PG 14 changed the function signature of encoding conversion functions. * Conversions from older versions cannot be upgraded automatically @@ -1551,6 +1561,40 @@ check_for_removed_data_type_usage(ClusterInfo *cluster, const char *version, check_ok(); } +/* + * check_for_removed_data_type_usage + * + * Check for in-core data types that have been removed. Callers know + * the exact list. + */ +static void +check_for_removed_data_type_usage(ClusterInfo *cluster, const char *version, + const char *datatype) +{ + char output_path[MAXPGPATH]; + char typename[NAMEDATALEN]; + + prep_status("Checking for removed \"%s\" data type in user tables", + datatype); + + snprintf(output_path, sizeof(output_path), "tables_using_%s.txt", + datatype); + snprintf(typename, sizeof(typename), "pg_catalog.%s", datatype); + + if (check_for_data_type_usage(cluster, typename, output_path)) + { + pg_log(PG_REPORT, "fatal"); + pg_fatal("Your installation contains the \"%s\" data type in user tables.\n" + "The \"%s\" type has been removed in PostgreSQL version %s,\n" + "so this cluster cannot currently be upgraded. You can drop the\n" + "problem columns, or change them to another data type, and restart\n" + "the upgrade. A list of the problem columns is in the file:\n" + " %s\n\n", datatype, datatype, version, output_path); + } + else + check_ok(); +} + /* * check_for_jsonb_9_4_usage() From 1c1af6eebd243d06deeaeabef6e6685a8b5ed29e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 25 Sep 2023 11:50:28 -0400 Subject: [PATCH 24/91] Limit to_tsvector_byid's initial array allocation to something sane. The initial estimate of the number of distinct ParsedWords is just that: an estimate. Don't let it exceed what palloc is willing to allocate. If in fact we need more entries, we'll eventually fail trying to enlarge the array. But if we don't, this allows success on inputs that currently draw "invalid memory alloc request size". Per bug #18080 from Uwe Binder. Back-patch to all supported branches. Discussion: https://postgr.es/m/18080-d5c5e58fef8c99b7@postgresql.org --- src/backend/tsearch/to_tsany.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c index c60b1bae341..7ad622c0f4f 100644 --- a/src/backend/tsearch/to_tsany.c +++ b/src/backend/tsearch/to_tsany.c @@ -265,6 +265,8 @@ to_tsvector_byid(PG_FUNCTION_ARGS) * number */ if (prs.lenwords < 2) prs.lenwords = 2; + else if (prs.lenwords > MaxAllocSize / sizeof(ParsedWord)) + prs.lenwords = MaxAllocSize / sizeof(ParsedWord); prs.curwords = 0; prs.pos = 0; prs.words = (ParsedWord *) palloc(sizeof(ParsedWord) * prs.lenwords); From 29622d53ca152864288cb16c09b48d166df8c3f2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 25 Sep 2023 14:41:57 -0400 Subject: [PATCH 25/91] Collect dependency information for parsed CallStmts. Parse analysis of a CallStmt will inject mutable information, for instance the OID of the called procedure, so that subsequent DDL may create a need to re-parse the CALL. We failed to detect this for CALLs in plpgsql routines, because no dependency information was collected when putting a CallStmt into the plan cache. That could lead to misbehavior or strange errors such as "cache lookup failed". Before commit ee895a655, the issue would only manifest for CALLs appearing in atomic contexts, because we re-planned non-atomic CALLs every time through anyway. It is now apparent that extract_query_dependencies() probably needs a special case for every utility statement type for which stmt_requires_parse_analysis() returns true. I wanted to add something like Assert(!stmt_requires_parse_analysis(...)) when falling out of extract_query_dependencies_walker without doing anything, but there are API issues as well as a more fundamental point: stmt_requires_parse_analysis is supposed to be applied to raw parser output, so it'd be cheating to assume it will give the correct answer for post-parse-analysis trees. I contented myself with adding a comment. Per bug #18131 from Christian Stork. Back-patch to all supported branches. Discussion: https://postgr.es/m/18131-576854e79c5cd264@postgresql.org --- src/backend/optimizer/plan/setrefs.c | 23 +++++++++- src/pl/plpgsql/src/expected/plpgsql_call.out | 47 ++++++++++++++++++++ src/pl/plpgsql/src/sql/plpgsql_call.sql | 38 ++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 776f239ed4f..fd271d68ffe 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -3847,8 +3847,27 @@ extract_query_dependencies_walker(Node *node, PlannerInfo *context) if (query->commandType == CMD_UTILITY) { /* - * Ignore utility statements, except those (such as EXPLAIN) that - * contain a parsed-but-not-planned query. + * This logic must handle any utility command for which parse + * analysis was nontrivial (cf. stmt_requires_parse_analysis). + * + * Notably, CALL requires its own processing. + */ + if (IsA(query->utilityStmt, CallStmt)) + { + CallStmt *callstmt = (CallStmt *) query->utilityStmt; + + /* We need not examine funccall, just the transformed exprs */ + (void) extract_query_dependencies_walker((Node *) callstmt->funcexpr, + context); + (void) extract_query_dependencies_walker((Node *) callstmt->outargs, + context); + return false; + } + + /* + * Ignore other utility statements, except those (such as EXPLAIN) + * that contain a parsed-but-not-planned query. For those, we + * just need to transfer our attention to the contained query. */ query = UtilityContainsQuery(query->utilityStmt); if (query == NULL) diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index f4481ce66db..892e0780b18 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -472,3 +472,50 @@ BEGIN END; $$; NOTICE: +-- check that we detect change of dependencies in CALL +-- atomic and non-atomic call sites used to do this differently, so check both +CREATE PROCEDURE inner_p (f1 int) +AS $$ +BEGIN + RAISE NOTICE 'inner_p(%)', f1; +END +$$ LANGUAGE plpgsql; +CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 1 $$ LANGUAGE sql; +CREATE PROCEDURE outer_p (f1 int) +AS $$ +BEGIN + RAISE NOTICE 'outer_p(%)', f1; + CALL inner_p(f(f1)); +END +$$ LANGUAGE plpgsql; +CREATE FUNCTION outer_f (f1 int) RETURNS void +AS $$ +BEGIN + RAISE NOTICE 'outer_f(%)', f1; + CALL inner_p(f(f1)); +END +$$ LANGUAGE plpgsql; +CALL outer_p(42); +NOTICE: outer_p(42) +NOTICE: inner_p(43) +SELECT outer_f(42); +NOTICE: outer_f(42) +NOTICE: inner_p(43) + outer_f +--------- + +(1 row) + +DROP FUNCTION f(int); +CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql; +CALL outer_p(42); +NOTICE: outer_p(42) +NOTICE: inner_p(44) +SELECT outer_f(42); +NOTICE: outer_f(42) +NOTICE: inner_p(44) + outer_f +--------- + +(1 row) + diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql index f93f28435fa..d80a5d2819e 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -442,3 +442,41 @@ BEGIN RAISE NOTICE '%', v_Text; END; $$; + + +-- check that we detect change of dependencies in CALL +-- atomic and non-atomic call sites used to do this differently, so check both + +CREATE PROCEDURE inner_p (f1 int) +AS $$ +BEGIN + RAISE NOTICE 'inner_p(%)', f1; +END +$$ LANGUAGE plpgsql; + +CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 1 $$ LANGUAGE sql; + +CREATE PROCEDURE outer_p (f1 int) +AS $$ +BEGIN + RAISE NOTICE 'outer_p(%)', f1; + CALL inner_p(f(f1)); +END +$$ LANGUAGE plpgsql; + +CREATE FUNCTION outer_f (f1 int) RETURNS void +AS $$ +BEGIN + RAISE NOTICE 'outer_f(%)', f1; + CALL inner_p(f(f1)); +END +$$ LANGUAGE plpgsql; + +CALL outer_p(42); +SELECT outer_f(42); + +DROP FUNCTION f(int); +CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql; + +CALL outer_p(42); +SELECT outer_f(42); From 7b26ac9a0ce62225d69d812aaa1dd04cd2cd65ad Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 25 Sep 2023 11:50:02 -0700 Subject: [PATCH 26/91] pg_dump: tests: Correct test condition for invalid databases For some reason I used not_like = { pg_dumpall_dbprivs => 1, } in the test condition of one of the tests added in in c66a7d75e65. That doesn't make sense for two reasons: 1) not_like isn't a valid test condition 2) the database should not be dumped in any of the tests. Due to 1), the test achieved its goal, but clearly the formulation is confusing. Instead use like => {}, with a comment explaining why. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/3ddf79f2-8b7b-a093-11d2-5c739bc64f86@eisentraut.org Backpatch: 11-, like c66a7d75e65 --- src/bin/pg_dump/t/002_pg_dump.pl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index c472d5d5cee..ec9bb8b8656 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1596,6 +1596,16 @@ regexp => qr/\n--[^\n]*\nattack/s, like => {}, }, + 'CREATE DATABASE regression_invalid...' => { + create_order => 1, + create_sql => q( + CREATE DATABASE regression_invalid; + UPDATE pg_database SET datconnlimit = -2 WHERE datname = 'regression_invalid'), + regexp => qr/^CREATE DATABASE regression_invalid/m, + + # invalid databases should never be dumped + like => {}, + }, 'CREATE ACCESS METHOD gist2' => { create_order => 52, From 77824022e9eecf8b710fde21fd4a475d0c921e0a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 26 Sep 2023 08:16:44 +0900 Subject: [PATCH 27/91] doc: Tell about "vcregress taptest" for regression tests on Windows There was no mention of this command in the documentation, and it is useful to run the TAP tests of a target source directory. Author: Yugo Nagata Discussion: https://postgr.es/m/20230925153204.926d685d347ee1c8f527090c@sraoss.co.jp Backpatch-through: 11 --- doc/src/sgml/install-windows.sgml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml index 176f15dea09..e4e1abdff39 100644 --- a/doc/src/sgml/install-windows.sgml +++ b/doc/src/sgml/install-windows.sgml @@ -462,6 +462,7 @@ $ENV{CONFIG}="Debug"; vcregress isolationcheck vcregress bincheck vcregress recoverycheck +vcregress taptest vcregress upgradecheck @@ -469,6 +470,12 @@ $ENV{CONFIG}="Debug"; command line like: vcregress check serial + + + vcregress taptest can be used to run the TAP tests + of a target directory, like: + +vcregress taptest src\bin\initdb\ For more information about the regression tests, see @@ -476,9 +483,10 @@ $ENV{CONFIG}="Debug"; - Running the regression tests on client programs, with - vcregress bincheck, or on recovery tests, with - vcregress recoverycheck, requires an additional Perl module + Running the regression tests on client programs with + vcregress bincheck, on recovery tests with + vcregress recoverycheck, or TAP tests specified with + vcregress taptest requires an additional Perl module to be installed: From fbd00da2eff94c8e04fb7cc4b11b5156d39a4cbc Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 26 Sep 2023 14:14:49 +0300 Subject: [PATCH 28/91] Fix another bug in parent page splitting during GiST index build. Yet another bug in the ilk of commits a7ee7c851 and 741b88435. In 741b88435, we took care to clear the memorized location of the downlink when we split the parent page, because splitting the parent page can move the downlink. But we missed that even *updating* a tuple on the parent can move it, because updating a tuple on a gist page is implemented as a delete+insert, so the updated tuple gets moved to the end of the page. This commit fixes the bug in two different ways (belt and suspenders): 1. Clear the downlink when we update a tuple on the parent page, even if it's not split. This the same approach as in commits a7ee7c851 and 741b88435. I also noticed that gistFindCorrectParent did not clear the 'downlinkoffnum' when it stepped to the right sibling. Fix that too, as it seems like a clear bug even though I haven't been able to find a test case to hit that. 2. Change gistFindCorrectParent so that it treats 'downlinkoffnum' merely as a hint. It now always first checks if the downlink is still at that location, and if not, it scans the page like before. That's more robust if there are still more cases where we fail to clear 'downlinkoffnum' that we haven't yet uncovered. With this, it's no longer necessary to meticulously clear 'downlinkoffnum', so this makes the previous fixes unnecessary, but I didn't revert them because it still seems nice to clear it when we know that the downlink has moved. Also add the test case using the same test data that Alexander posted. I tried to reduce it to a smaller test, and I also tried to reproduce this with different test data, but I was not able to, so let's just include what we have. Backpatch to v12, like the previous fixes. Reported-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/18129-caca016eaf0c3702@postgresql.org --- contrib/intarray/expected/_int.out | 91 +++++++++++++++ contrib/intarray/sql/_int.sql | 35 ++++++ src/backend/access/gist/gist.c | 180 ++++++++++++++++------------- 3 files changed, 226 insertions(+), 80 deletions(-) diff --git a/contrib/intarray/expected/_int.out b/contrib/intarray/expected/_int.out index d937b7ae8c5..b361d458912 100644 --- a/contrib/intarray/expected/_int.out +++ b/contrib/intarray/expected/_int.out @@ -859,4 +859,95 @@ SELECT count(*) from test__int WHERE a @@ '!20 & !21'; 6343 (1 row) +DROP INDEX text_idx; +-- Repeat the same queries with an extended data set. The data set is the +-- same that we used before, except that each element in the array is +-- repeated three times, offset by 1000 and 2000. For example, {1, 5} +-- becomes {1, 1001, 2001, 5, 1005, 2005}. +-- +-- That has proven to be unreasonably effective at exercising codepaths in +-- core GiST code related to splitting parent pages, which is not covered by +-- other tests. This is a bit out-of-place as the point is to test core GiST +-- code rather than this extension, but there is no suitable GiST opclass in +-- core that would reach the same codepaths. +CREATE TABLE more__int AS SELECT + -- Leave alone NULLs, empty arrays and the one row that we use to test + -- equality + CASE WHEN a IS NULL OR a = '{}' OR a = '{73,23,20}' THEN a ELSE + (select array_agg(u) || array_agg(u + 1000) || array_agg(u + 2000) from (select unnest(a) u) x) + END AS a, a as b + FROM test__int; +CREATE INDEX ON more__int using gist (a gist__int_ops(numranges = 252)); +SELECT count(*) from more__int WHERE a && '{23,50}'; + count +------- + 403 +(1 row) + +SELECT count(*) from more__int WHERE a @@ '23|50'; + count +------- + 403 +(1 row) + +SELECT count(*) from more__int WHERE a @> '{23,50}'; + count +------- + 12 +(1 row) + +SELECT count(*) from more__int WHERE a @@ '23&50'; + count +------- + 12 +(1 row) + +SELECT count(*) from more__int WHERE a @> '{20,23}'; + count +------- + 12 +(1 row) + +SELECT count(*) from more__int WHERE a <@ '{73,23,20}'; + count +------- + 10 +(1 row) + +SELECT count(*) from more__int WHERE a = '{73,23,20}'; + count +------- + 1 +(1 row) + +SELECT count(*) from more__int WHERE a @@ '50&68'; + count +------- + 9 +(1 row) + +SELECT count(*) from more__int WHERE a @> '{20,23}' or a @> '{50,68}'; + count +------- + 21 +(1 row) + +SELECT count(*) from more__int WHERE a @@ '(20&23)|(50&68)'; + count +------- + 21 +(1 row) + +SELECT count(*) from more__int WHERE a @@ '20 | !21'; + count +------- + 6566 +(1 row) + +SELECT count(*) from more__int WHERE a @@ '!20 & !21'; + count +------- + 6343 +(1 row) + RESET enable_seqscan; diff --git a/contrib/intarray/sql/_int.sql b/contrib/intarray/sql/_int.sql index 00bb5e87f32..93004d374ad 100644 --- a/contrib/intarray/sql/_int.sql +++ b/contrib/intarray/sql/_int.sql @@ -180,4 +180,39 @@ SELECT count(*) from test__int WHERE a @@ '(20&23)|(50&68)'; SELECT count(*) from test__int WHERE a @@ '20 | !21'; SELECT count(*) from test__int WHERE a @@ '!20 & !21'; +DROP INDEX text_idx; + +-- Repeat the same queries with an extended data set. The data set is the +-- same that we used before, except that each element in the array is +-- repeated three times, offset by 1000 and 2000. For example, {1, 5} +-- becomes {1, 1001, 2001, 5, 1005, 2005}. +-- +-- That has proven to be unreasonably effective at exercising codepaths in +-- core GiST code related to splitting parent pages, which is not covered by +-- other tests. This is a bit out-of-place as the point is to test core GiST +-- code rather than this extension, but there is no suitable GiST opclass in +-- core that would reach the same codepaths. +CREATE TABLE more__int AS SELECT + -- Leave alone NULLs, empty arrays and the one row that we use to test + -- equality + CASE WHEN a IS NULL OR a = '{}' OR a = '{73,23,20}' THEN a ELSE + (select array_agg(u) || array_agg(u + 1000) || array_agg(u + 2000) from (select unnest(a) u) x) + END AS a, a as b + FROM test__int; +CREATE INDEX ON more__int using gist (a gist__int_ops(numranges = 252)); + +SELECT count(*) from more__int WHERE a && '{23,50}'; +SELECT count(*) from more__int WHERE a @@ '23|50'; +SELECT count(*) from more__int WHERE a @> '{23,50}'; +SELECT count(*) from more__int WHERE a @@ '23&50'; +SELECT count(*) from more__int WHERE a @> '{20,23}'; +SELECT count(*) from more__int WHERE a <@ '{73,23,20}'; +SELECT count(*) from more__int WHERE a = '{73,23,20}'; +SELECT count(*) from more__int WHERE a @@ '50&68'; +SELECT count(*) from more__int WHERE a @> '{20,23}' or a @> '{50,68}'; +SELECT count(*) from more__int WHERE a @@ '(20&23)|(50&68)'; +SELECT count(*) from more__int WHERE a @@ '20 | !21'; +SELECT count(*) from more__int WHERE a @@ '!20 & !21'; + + RESET enable_seqscan; diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 3e021ac2527..42568c03829 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -1017,87 +1017,106 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum) * remain so at exit, but it might not be the same page anymore. */ static void -gistFindCorrectParent(Relation r, GISTInsertStack *child) +gistFindCorrectParent(Relation r, GISTInsertStack *child, bool is_build) { GISTInsertStack *parent = child->parent; + ItemId iid; + IndexTuple idxtuple; + OffsetNumber maxoff; + GISTInsertStack *ptr; gistcheckpage(r, parent->buffer); parent->page = (Page) BufferGetPage(parent->buffer); + maxoff = PageGetMaxOffsetNumber(parent->page); - /* here we don't need to distinguish between split and page update */ - if (child->downlinkoffnum == InvalidOffsetNumber || - parent->lsn != PageGetLSN(parent->page)) + /* Check if the downlink is still where it was before */ + if (child->downlinkoffnum != InvalidOffsetNumber && child->downlinkoffnum <= maxoff) { - /* parent is changed, look child in right links until found */ - OffsetNumber i, - maxoff; - ItemId iid; - IndexTuple idxtuple; - GISTInsertStack *ptr; + iid = PageGetItemId(parent->page, child->downlinkoffnum); + idxtuple = (IndexTuple) PageGetItem(parent->page, iid); + if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno) + return; /* still there */ + } - while (true) - { - maxoff = PageGetMaxOffsetNumber(parent->page); - for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i)) - { - iid = PageGetItemId(parent->page, i); - idxtuple = (IndexTuple) PageGetItem(parent->page, iid); - if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno) - { - /* yes!!, found */ - child->downlinkoffnum = i; - return; - } - } + /* + * The page has changed since we looked. During normal operation, every + * update of a page changes its LSN, so the LSN we memorized should have + * changed too. During index build, however, we don't WAL-log the changes + * until we have built the index, so the LSN doesn't change. There is no + * concurrent activity during index build, but we might have changed the + * parent ourselves. + */ + Assert(parent->lsn != PageGetLSN(parent->page) || is_build); + + /* + * Scan the page to re-find the downlink. If the page was split, it might + * have moved to a different page, so follow the right links until we find + * it. + */ + while (true) + { + OffsetNumber i; - parent->blkno = GistPageGetOpaque(parent->page)->rightlink; - UnlockReleaseBuffer(parent->buffer); - if (parent->blkno == InvalidBlockNumber) + maxoff = PageGetMaxOffsetNumber(parent->page); + for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i)) + { + iid = PageGetItemId(parent->page, i); + idxtuple = (IndexTuple) PageGetItem(parent->page, iid); + if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno) { - /* - * End of chain and still didn't find parent. It's a very-very - * rare situation when root splitted. - */ - break; + /* yes!!, found */ + child->downlinkoffnum = i; + return; } - parent->buffer = ReadBuffer(r, parent->blkno); - LockBuffer(parent->buffer, GIST_EXCLUSIVE); - gistcheckpage(r, parent->buffer); - parent->page = (Page) BufferGetPage(parent->buffer); } - /* - * awful!!, we need search tree to find parent ... , but before we - * should release all old parent - */ - - ptr = child->parent->parent; /* child->parent already released - * above */ - while (ptr) + parent->blkno = GistPageGetOpaque(parent->page)->rightlink; + parent->downlinkoffnum = InvalidOffsetNumber; + UnlockReleaseBuffer(parent->buffer); + if (parent->blkno == InvalidBlockNumber) { - ReleaseBuffer(ptr->buffer); - ptr = ptr->parent; + /* + * End of chain and still didn't find parent. It's a very-very + * rare situation when root splitted. + */ + break; } + parent->buffer = ReadBuffer(r, parent->blkno); + LockBuffer(parent->buffer, GIST_EXCLUSIVE); + gistcheckpage(r, parent->buffer); + parent->page = (Page) BufferGetPage(parent->buffer); + } - /* ok, find new path */ - ptr = parent = gistFindPath(r, child->blkno, &child->downlinkoffnum); + /* + * awful!!, we need search tree to find parent ... , but before we should + * release all old parent + */ - /* read all buffers as expected by caller */ - /* note we don't lock them or gistcheckpage them here! */ - while (ptr) - { - ptr->buffer = ReadBuffer(r, ptr->blkno); - ptr->page = (Page) BufferGetPage(ptr->buffer); - ptr = ptr->parent; - } + ptr = child->parent->parent; /* child->parent already released above */ + while (ptr) + { + ReleaseBuffer(ptr->buffer); + ptr = ptr->parent; + } - /* install new chain of parents to stack */ - child->parent = parent; + /* ok, find new path */ + ptr = parent = gistFindPath(r, child->blkno, &child->downlinkoffnum); - /* make recursive call to normal processing */ - LockBuffer(child->parent->buffer, GIST_EXCLUSIVE); - gistFindCorrectParent(r, child); + /* read all buffers as expected by caller */ + /* note we don't lock them or gistcheckpage them here! */ + while (ptr) + { + ptr->buffer = ReadBuffer(r, ptr->blkno); + ptr->page = (Page) BufferGetPage(ptr->buffer); + ptr = ptr->parent; } + + /* install new chain of parents to stack */ + child->parent = parent; + + /* make recursive call to normal processing */ + LockBuffer(child->parent->buffer, GIST_EXCLUSIVE); + gistFindCorrectParent(r, child, is_build); } /* @@ -1105,7 +1124,7 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) */ static IndexTuple gistformdownlink(Relation rel, Buffer buf, GISTSTATE *giststate, - GISTInsertStack *stack) + GISTInsertStack *stack, bool is_build) { Page page = BufferGetPage(buf); OffsetNumber maxoff; @@ -1146,7 +1165,7 @@ gistformdownlink(Relation rel, Buffer buf, GISTSTATE *giststate, ItemId iid; LockBuffer(stack->parent->buffer, GIST_EXCLUSIVE); - gistFindCorrectParent(rel, stack); + gistFindCorrectParent(rel, stack, is_build); iid = PageGetItemId(stack->parent->page, stack->downlinkoffnum); downlink = (IndexTuple) PageGetItem(stack->parent->page, iid); downlink = CopyIndexTuple(downlink); @@ -1192,7 +1211,7 @@ gistfixsplit(GISTInsertState *state, GISTSTATE *giststate) page = BufferGetPage(buf); /* Form the new downlink tuples to insert to parent */ - downlink = gistformdownlink(state->r, buf, giststate, stack); + downlink = gistformdownlink(state->r, buf, giststate, stack, state->is_build); si->buf = buf; si->downlink = downlink; @@ -1346,7 +1365,7 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, right = (GISTPageSplitInfo *) list_nth(splitinfo, pos); left = (GISTPageSplitInfo *) list_nth(splitinfo, pos - 1); - gistFindCorrectParent(state->r, stack); + gistFindCorrectParent(state->r, stack, state->is_build); if (gistinserttuples(state, stack->parent, giststate, &right->downlink, 1, InvalidOffsetNumber, @@ -1371,21 +1390,22 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, */ tuples[0] = left->downlink; tuples[1] = right->downlink; - gistFindCorrectParent(state->r, stack); - if (gistinserttuples(state, stack->parent, giststate, - tuples, 2, - stack->downlinkoffnum, - left->buf, right->buf, - true, /* Unlock parent */ - unlockbuf /* Unlock stack->buffer if caller wants - * that */ - )) - { - /* - * If the parent page was split, the downlink might have moved. - */ - stack->downlinkoffnum = InvalidOffsetNumber; - } + gistFindCorrectParent(state->r, stack, state->is_build); + (void) gistinserttuples(state, stack->parent, giststate, + tuples, 2, + stack->downlinkoffnum, + left->buf, right->buf, + true, /* Unlock parent */ + unlockbuf /* Unlock stack->buffer if caller + * wants that */ + ); + + /* + * The downlink might have moved when we updated it. Even if the page + * wasn't split, because gistinserttuples() implements updating the old + * tuple by removing and re-inserting it! + */ + stack->downlinkoffnum = InvalidOffsetNumber; Assert(left->buf == stack->buffer); From a281987825b39fae65c50bbc1089ace6a2a2b933 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 26 Sep 2023 17:31:06 -0400 Subject: [PATCH 29/91] doc: mention GROUP BY columns can reference target col numbers Reported-by: hape Discussion: https://postgr.es/m/168871536004.379168.9352636188330923805@wrigleys.postgresql.org Backpatch-through: 11 --- doc/src/sgml/ref/select.sgml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 7b8b24b9af8..27e4d12534d 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -131,6 +131,9 @@ TABLE [ ONLY ] table_name [ * ] eliminates groups that do not satisfy the given condition. (See and below.) + Although query output columns are nominally computed in the next + step, they can also be referenced (by name or ordinal number) + in the GROUP BY clause. From 31b492eee56f1e8a12b9c26976a2abb1f57c55b0 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 26 Sep 2023 18:54:10 -0400 Subject: [PATCH 30/91] doc: pg_upgrade, clarify standby servers must remain running Also mention that mismatching primary/standby LSNs should never happen. Reported-by: Nikolay Samokhvalov Discussion: https://postgr.es/m/CAM527d8heqkjG5VrvjU3Xjsqxg41ufUyabD9QZccdAxnpbRH-Q@mail.gmail.com Backpatch-through: 11 --- doc/src/sgml/ref/pgupgrade.sgml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 81cd4135eba..fc91ec31d9b 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -373,8 +373,8 @@ NET STOP postgresql-&majorversion; - Streaming replication and log-shipping standby servers can - remain running until a later step. + Streaming replication and log-shipping standby servers must be + running during this shutdown so they receive all changes. @@ -387,8 +387,6 @@ NET STOP postgresql-&majorversion; servers are caught up by running pg_controldata against the old primary and standby clusters. Verify that the Latest checkpoint location values match in all clusters. - (There will be a mismatch if old standby servers were shut down - before the old primary or if the old standby servers are still running.) Also, make sure wal_level is not set to minimal in the postgresql.conf file on the new primary cluster. From 747a4870d75ef995920f9f7e2fe9726a0f655343 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 26 Sep 2023 19:02:18 -0400 Subject: [PATCH 31/91] doc: clarify the behavior of unopenable listen_addresses Reported-by: Gurjeet Singh Discussion: https://postgr.es/m/CABwTF4WYPD9ov-kcSq1+J+ZJ5wYDQLXquY6Lu2cvb-Y7pTpSGA@mail.gmail.com Backpatch-through: 11 --- doc/src/sgml/config.sgml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index ec623af937f..80f9c35d771 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -653,10 +653,15 @@ include_dir 'conf.d' :: allows listening for all IPv6 addresses. If the list is empty, the server does not listen on any IP interface at all, in which case only Unix-domain sockets can be used to connect - to it. + to it. If the list is not empty, the server will start if it + can listen on at least one TCP/IP address. A warning will be + emitted for any TCP/IP address which cannot be opened. The default value is localhost, which allows only local TCP/IP loopback connections to be - made. While client authentication ( + + While client authentication () allows fine-grained control over who can access the server, listen_addresses controls which interfaces accept connection attempts, which From dd931e53ad421b1253bb4296f4c43bfd2faf0925 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 26 Sep 2023 19:23:59 -0400 Subject: [PATCH 32/91] doc: clarify handling of time zones with "time with time zone" Reported-by: davecramer@postgres.rocks Discussion: https://postgr.es/m/168451942371.714.9173574930845904336@wrigleys.postgresql.org Backpatch-through: 11 --- doc/src/sgml/datatype.sgml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index 692b6fe2c43..c17f3976b38 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -1986,7 +1986,8 @@ MINUTE TO SECOND America/New_York. In this case specifying the date is required in order to determine whether standard or daylight-savings time applies. The appropriate time zone offset is recorded in the - time with time zone value. + time with time zone value and is output as stored; + it is not adjusted to the active time zone.
From b16c2a691e162aabacf73d7c547bd57231848a07 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 26 Sep 2023 19:44:21 -0400 Subject: [PATCH 33/91] doc: clarify the effect of concurrent work_mem allocations Reported-by: Sami Imseih Discussion: https://postgr.es/m/66590882-F48C-4A25-83E3-73792CF8C51F@amazon.com Backpatch-through: 11 --- doc/src/sgml/config.sgml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 80f9c35d771..edbd5968ec7 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1829,9 +1829,10 @@ include_dir 'conf.d' (such as a sort or hash table) before writing to temporary disk files. If this value is specified without units, it is taken as kilobytes. The default value is four megabytes (4MB). - Note that for a complex query, several sort or hash operations might be - running in parallel; each operation will generally be allowed - to use as much memory as this value specifies before it starts + Note that a complex query might perform several sort and hash + operations at the same time, with each operation generally being + allowed to use as much memory as this value specifies before + it starts to write data into temporary files. Also, several running sessions could be doing such operations concurrently. Therefore, the total memory used could be many times the value @@ -1845,7 +1846,7 @@ include_dir 'conf.d' Hash-based operations are generally more sensitive to memory availability than equivalent sort-based operations. The - memory available for hash tables is computed by multiplying + memory limit for a hash table is computed by multiplying work_mem by hash_mem_multiplier. This makes it possible for hash-based operations to use an amount of memory From 0a3d0122f2fef2af31594f0b93c8e6afcb32cad2 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 26 Sep 2023 21:06:21 -0400 Subject: [PATCH 34/91] Stop using "-multiply_defined suppress" on macOS. We started to use this linker switch in commit 9df308697 of 2004-07-13, which was in the OS X 10.3 era. Apparently it's been a no-op since around OS X 10.9. Apple's most recent toolchain version actively complains about it, so it's time to get rid of it. Discussion: https://postgr.es/m/467042.1695766998@sss.pgh.pa.us --- src/Makefile.shlib | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Makefile.shlib b/src/Makefile.shlib index 50214226de6..4eb25cf6550 100644 --- a/src/Makefile.shlib +++ b/src/Makefile.shlib @@ -122,13 +122,13 @@ ifeq ($(PORTNAME), darwin) ifneq ($(SO_MAJOR_VERSION), 0) version_link = -compatibility_version $(SO_MAJOR_VERSION) -current_version $(SO_MAJOR_VERSION).$(SO_MINOR_VERSION) endif - LINK.shared = $(COMPILER) -dynamiclib -install_name '$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' $(version_link) $(exported_symbols_list) -multiply_defined suppress + LINK.shared = $(COMPILER) -dynamiclib -install_name '$(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)' $(version_link) $(exported_symbols_list) shlib = lib$(NAME).$(SO_MAJOR_VERSION).$(SO_MINOR_VERSION)$(DLSUFFIX) shlib_major = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX) else # loadable module DLSUFFIX = .so - LINK.shared = $(COMPILER) -bundle -multiply_defined suppress + LINK.shared = $(COMPILER) -bundle endif BUILD.exports = $(AWK) '/^[^\043]/ {printf "_%s\n",$$1}' $< >$@ exports_file = $(SHLIB_EXPORTS:%.txt=%.list) From 19587abbe60dd743bd2cbc39d75d4e31aeb57ea7 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 27 Sep 2023 14:41:23 +0900 Subject: [PATCH 35/91] unaccent: Tweak value of PYTHON when building without Python support As coded, the module's Makefile would fail to set a value for PYTHON as it checked if the variable is defined. When compiling without --with-python, PYTHON is defined and set to an empty value, so the existing check is not able to do its work. This commit switches the rule to check if the value is empty rather than defined, allowing the generation of unaccent.rules even if --with-python is not used as long as "python" exists. BISON and FLEX do the same in pgxs.mk, for instance. Thinko in f85a485f89e2. Author: Japin Li Discussion: https://postgr.es/m/MEYP282MB1669F86C0DC7B4DC48489CB0B6C3A@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM Backpatch-through: 13 --- contrib/unaccent/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/unaccent/Makefile b/contrib/unaccent/Makefile index b8307d1601e..9e42f3e638a 100644 --- a/contrib/unaccent/Makefile +++ b/contrib/unaccent/Makefile @@ -30,7 +30,9 @@ endif update-unicode: unaccent.rules # Allow running this even without --with-python -PYTHON ?= python +ifeq ($(PYTHON),) +PYTHON = python +endif unaccent.rules: generate_unaccent_rules.py ../../src/common/unicode/UnicodeData.txt Latin-ASCII.xml $(PYTHON) $< --unicode-data-file $(word 2,$^) --latin-ascii-file $(word 3,$^) >$@ From 5cf32f13eae4f69e6085f594a4aefe2b4d1d4837 Mon Sep 17 00:00:00 2001 From: Etsuro Fujita Date: Thu, 28 Sep 2023 19:45:05 +0900 Subject: [PATCH 36/91] Fix typo in src/backend/access/transam/README. --- src/backend/access/transam/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/access/transam/README b/src/backend/access/transam/README index efac0cb505e..fdfa7155ffd 100644 --- a/src/backend/access/transam/README +++ b/src/backend/access/transam/README @@ -324,7 +324,7 @@ changes much more often than its xid, having GetSnapshotData look at xmins can lead to a lot of unnecessary cacheline ping-pong. Instead GetSnapshotData updates approximate thresholds (one that guarantees that all deleted rows older than it can be removed, another determining that deleted -rows newer than it can not be removed). GlobalVisTest* uses those threshold +rows newer than it can not be removed). GlobalVisTest* uses those thresholds to make invisibility decision, falling back to ComputeXidHorizons if necessary. From c3f23ca61e379930256f0ba597aafe637b24108c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 29 Sep 2023 00:04:03 +1300 Subject: [PATCH 37/91] Add missing TidRangePath handling in print_path() Tid Range scans were added back in bb437f995. That commit forgot to add handling for TidRangePaths in print_path(). Only people building with OPTIMIZER_DEBUG might have noticed this, which likely is the reason it's taken 4 years for anyone to notice. Author: Andrey Lepikhov Reported-by: Andrey Lepikhov Discussion: https://postgr.es/m/379082d6-1b6a-4cd6-9ecf-7157d8c08635@postgrespro.ru Backpatch-through: 14, where bb437f995 was introduced --- src/backend/optimizer/path/allpaths.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 10e41de8cd9..daa9b9e998b 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -5022,6 +5022,9 @@ print_path(PlannerInfo *root, Path *path, int indent) case T_TidPath: ptype = "TidScan"; break; + case T_TidRangePath: + ptype = "TidRangePath"; + break; case T_SubqueryScanPath: ptype = "SubqueryScan"; break; From 8d55a58bd651be70f6c1eb34252133c29980ec79 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 28 Sep 2023 14:05:25 -0400 Subject: [PATCH 38/91] Fix checking of index expressions in CompareIndexInfo(). This code was sloppy about comparison of index columns that are expressions. It didn't reliably reject cases where one index has an expression where the other has a plain column, and it could index off the start of the attmap array, leading to a Valgrind complaint (though an actual crash seems unlikely). I'm not sure that the expression-vs-column sloppiness leads to any visible problem in practice, because the subsequent comparison of the two expression lists would reject cases where the indexes have different numbers of expressions overall. Maybe we could falsely match indexes having the same expressions in different column positions, but it'd require unlucky contents of the word before the attmap array. It's not too surprising that no problem has been reported from the field. Nonetheless, this code is clearly wrong. Per bug #18135 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18135-532f4a755e71e4d2@postgresql.org --- src/backend/catalog/index.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 2a1a93f5aa7..7035c4a74b2 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2709,7 +2709,7 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2, /* * and columns match through the attribute map (actual attribute numbers - * might differ!) Note that this implies that index columns that are + * might differ!) Note that this checks that index columns that are * expressions appear in the same positions. We will next compare the * expressions themselves. */ @@ -2718,13 +2718,22 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2, if (attmap->maplen < info2->ii_IndexAttrNumbers[i]) elog(ERROR, "incorrect attribute map"); - /* ignore expressions at this stage */ - if ((info1->ii_IndexAttrNumbers[i] != InvalidAttrNumber) && - (attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] != - info1->ii_IndexAttrNumbers[i])) - return false; + /* ignore expressions for now (but check their collation/opfamily) */ + if (!(info1->ii_IndexAttrNumbers[i] == InvalidAttrNumber && + info2->ii_IndexAttrNumbers[i] == InvalidAttrNumber)) + { + /* fail if just one index has an expression in this column */ + if (info1->ii_IndexAttrNumbers[i] == InvalidAttrNumber || + info2->ii_IndexAttrNumbers[i] == InvalidAttrNumber) + return false; + + /* both are columns, so check for match after mapping */ + if (attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] != + info1->ii_IndexAttrNumbers[i]) + return false; + } - /* collation and opfamily is not valid for including columns */ + /* collation and opfamily are not valid for included columns */ if (i >= info1->ii_NumIndexKeyAttrs) continue; From b60cdf165a22d0959c2d4d5ea9c0154cafe86847 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 28 Sep 2023 16:29:29 -0700 Subject: [PATCH 39/91] Fix btmarkpos/btrestrpos array key wraparound bug. nbtree's mark/restore processing failed to correctly handle an edge case involving array key advancement and related search-type scan key state. Scans with ScalarArrayScalarArrayOpExpr quals requiring mark/restore processing (for a merge join) could incorrectly conclude that an affected array/scan key must not have advanced during the time between marking and restoring the scan's position. As a result of all this, array key handling within btrestrpos could skip a required call to _bt_preprocess_keys(). This confusion allowed later primitive index scans to overlook tuples matching the true current array keys. The scan's search-type scan keys would still have spurious values corresponding to the final array element(s) -- not values matching the first/now-current array element(s). To fix, remember that "array key wraparound" has taken place during the ongoing btrescan in a flag variable stored in the scan's state, and use that information at the point where btrestrpos decides if another call to _bt_preprocess_keys is required. Oversight in commit 70bc5833, which taught nbtree to handle array keys during mark/restore processing, but missed this subtlety. That commit was itself a bug fix for an issue in commit 9e8da0f7, which taught nbtree to handle ScalarArrayOpExpr quals natively. Author: Peter Geoghegan Discussion: https://postgr.es/m/CAH2-WzkgP3DDRJxw6DgjCxo-cu-DKrvjEv_ArkP2ctBJatDCYg@mail.gmail.com Backpatch: 11- (all supported branches). --- src/backend/access/nbtree/nbtree.c | 1 + src/backend/access/nbtree/nbtutils.c | 17 ++++++++++++++++- src/include/access/nbtree.h | 4 +++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 8edf4694401..8d4a587899c 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -463,6 +463,7 @@ btbeginscan(Relation rel, int nkeys, int norderbys) so->keyData = NULL; so->arrayKeyData = NULL; /* assume no array keys for now */ + so->arraysStarted = false; so->numArrayKeys = 0; so->arrayKeys = NULL; so->arrayContext = NULL; diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index 733a97a76df..cee04bf0d1b 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -532,6 +532,8 @@ _bt_start_array_keys(IndexScanDesc scan, ScanDirection dir) curArrayKey->cur_elem = 0; skey->sk_argument = curArrayKey->elem_values[curArrayKey->cur_elem]; } + + so->arraysStarted = true; } /* @@ -591,6 +593,14 @@ _bt_advance_array_keys(IndexScanDesc scan, ScanDirection dir) if (scan->parallel_scan != NULL) _bt_parallel_advance_array_keys(scan); + /* + * When no new array keys were found, the scan is "past the end" of the + * array keys. _bt_start_array_keys can still "restart" the array keys if + * a rescan is required. + */ + if (!found) + so->arraysStarted = false; + return found; } @@ -644,8 +654,13 @@ _bt_restore_array_keys(IndexScanDesc scan) * If we changed any keys, we must redo _bt_preprocess_keys. That might * sound like overkill, but in cases with multiple keys per index column * it seems necessary to do the full set of pushups. + * + * Also do this whenever the scan's set of array keys "wrapped around" at + * the end of the last primitive index scan. There won't have been a call + * to _bt_preprocess_keys from some other place following wrap around, so + * we do it for ourselves. */ - if (changed) + if (changed || !so->arraysStarted) { _bt_preprocess_keys(scan); /* The mark should have been set on a consistent set of keys... */ diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index 816a0fe761d..8d7d972f715 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -1029,8 +1029,10 @@ typedef struct BTArrayKeyInfo typedef struct BTScanOpaqueData { - /* these fields are set by _bt_preprocess_keys(): */ + /* all fields (except arraysStarted) are set by _bt_preprocess_keys(): */ bool qual_ok; /* false if qual can never be satisfied */ + bool arraysStarted; /* Started array keys, but have yet to "reach + * past the end" of all arrays? */ int numberOfKeys; /* number of preprocessed scan keys */ ScanKey keyData; /* array of preprocessed scan keys */ From a6e1a97fa226a9ade8cea610a31d44f06bdad112 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 29 Sep 2023 15:55:37 +0200 Subject: [PATCH 40/91] doc: Change statistics function xref to the right target Commit 7d3b7011b added a link to the statistics functions, which at the time were anchored under the section for statistics views. aebe989477a added a separate section for statistics functions, but the link was not updated to point to the new anchor. Fix by changing the xref. Backpatch to all supported branches. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Ptr0jKzNNtWnssLq+3jNhbyaBseqf6NPrWHk08mQFRoTg@mail.gmail.com Backpatch-through: 11 --- doc/src/sgml/func.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 8578c4d81aa..a30c79abbbd 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -21767,7 +21767,7 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n); In addition to the functions listed in this section, there are a number of functions related to the statistics system that also provide system - information. See for more + information. See for more information. From 7220c7f2d5abdb7a657e60e6143b1014ff4274e6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 29 Sep 2023 13:13:54 -0400 Subject: [PATCH 41/91] Doc: improve description of dump/restore's --clean and --if-exists. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Try to make these option descriptions a little clearer for novices. Per gripe from Attila Gulyás. Discussion: https://postgr.es/m/169590536647.3727336.11070254203649648453@wrigleys.postgresql.org --- doc/src/sgml/ref/pg_dump.sgml | 17 ++++++++++------- doc/src/sgml/ref/pg_dumpall.sgml | 17 +++++++++++------ doc/src/sgml/ref/pg_restore.sgml | 18 +++++++++++------- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index 6a2ad26a17d..b317138cbef 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -180,11 +180,12 @@ PostgreSQL documentation - Output commands to clean (drop) + Output commands to DROP all the dumped database objects prior to outputting the commands for creating them. - (Unless is also specified, - restore might generate some harmless error messages, if any objects - were not present in the destination database.) + This option is useful when the restore is to overwrite an existing + database. If any of the objects do not exist in the destination + database, ignorable error messages will be reported during + restore, unless is also specified. @@ -830,9 +831,11 @@ PostgreSQL documentation - Use conditional commands (i.e., add an IF EXISTS - clause) when cleaning database objects. This option is not valid - unless is also specified. + Use DROP ... IF EXISTS commands to drop objects + in mode. This suppresses does not + exist errors that might otherwise be reported. This + option is not valid unless is also + specified. diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml index c025c4cf27f..f407aac497f 100644 --- a/doc/src/sgml/ref/pg_dumpall.sgml +++ b/doc/src/sgml/ref/pg_dumpall.sgml @@ -100,9 +100,12 @@ PostgreSQL documentation - Include SQL commands to clean (drop) databases before - recreating them. DROP commands for roles and - tablespaces are added as well. + Emit SQL commands to DROP all the dumped + databases, roles, and tablespaces before recreating them. + This option is useful when the restore is to overwrite an existing + cluster. If any of the objects do not exist in the destination + cluster, ignorable error messages will be reported during + restore, unless is also specified. @@ -368,9 +371,11 @@ PostgreSQL documentation - Use conditional commands (i.e., add an IF EXISTS - clause) to drop databases and other objects. This option is not valid - unless is also specified. + Use DROP ... IF EXISTS commands to drop objects + in mode. This suppresses does not + exist errors that might otherwise be reported. This + option is not valid unless is also + specified. diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml index 40f136670cb..71ea95a9c7b 100644 --- a/doc/src/sgml/ref/pg_restore.sgml +++ b/doc/src/sgml/ref/pg_restore.sgml @@ -123,10 +123,12 @@ PostgreSQL documentation - Clean (drop) database objects before recreating them. - (Unless is used, - this might generate some harmless error messages, if any objects - were not present in the destination database.) + Before restoring database objects, issue commands + to DROP all the objects that will be restored. + This option is useful for overwriting an existing database. + If any of the objects do not exist in the destination database, + ignorable error messages will be reported, + unless is also specified. @@ -592,9 +594,11 @@ PostgreSQL documentation - Use conditional commands (i.e., add an IF EXISTS - clause) to drop database objects. This option is not valid - unless is also specified. + Use DROP ... IF EXISTS commands to drop objects + in mode. This suppresses does not + exist errors that might otherwise be reported. This + option is not valid unless is also + specified. From 7c233109eafe67cbef30bfa0a4933e97360db6ec Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 29 Sep 2023 14:07:30 -0400 Subject: [PATCH 42/91] Suppress macOS warnings about duplicate libraries in link commands. As of Xcode 15 (macOS Sonoma), the linker complains about duplicate references to the same library. We see warnings about libpgport and libpgcommon being duplicated in many client executables. This is a consequence of the hack introduced in commit 6b7ef076b to list libpgport before libpq while not removing it from $(LIBS). (Commit 8396447cd later applied the same rule to libpgcommon.) The concern in 6b7ef076b was to ensure that the client executable wouldn't unintentionally depend on pgport functions from libpq. That concern is obsolete on any platform for which we can do symbol export control, because if we can then the pgport functions in libpq won't be exposed anyway. Hence, we can fix this problem by just removing libpgport and libpgcommon from $(libpq_pgport), and letting clients depend on the occurrences in $(LIBS). In the back branches, do that only on macOS (which we know has symbol export control). In HEAD, let's be more aggressive and remove the extra libraries everywhere. The only still-supported platforms that lack export control are MinGW/Cygwin, and it doesn't seem worth sweating over ABI stability details for those (or if somebody does care, it'd probably be possible to perform symbol export control for those too). As well as being simpler, this might give some microscopic improvement in build time. The meson build system is not changed here, as it doesn't have this particular disease, though it does have some related issues that we'll fix separately. Discussion: https://postgr.es/m/467042.1695766998@sss.pgh.pa.us --- src/Makefile.global.in | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 457a4a0944e..5b58f241964 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -663,19 +663,32 @@ endif libpq = -L$(libpq_builddir) -lpq # libpq_pgport is for use by client executables (not libraries) that use libpq. -# We force clients to pull symbols from the non-shared libraries libpgport +# We want clients to pull symbols from the non-shared libraries libpgport # and libpgcommon rather than pulling some libpgport symbols from libpq just # because libpq uses those functions too. This makes applications less -# dependent on changes in libpq's usage of pgport (on platforms where we -# don't have symbol export control for libpq). To do this we link to +# dependent on changes in libpq's usage of pgport. To do this we link to # pgport before libpq. This does cause duplicate -lpgport's to appear -# on client link lines, since that also appears in $(LIBS). +# on client link lines, since that also appears in $(LIBS). On platforms +# where we have symbol export control for libpq, the whole exercise is +# unnecessary because libpq won't expose any of these symbols. Currently, +# only macOS warns about duplicate library references, so we only suppress +# the duplicates on macOS. +ifeq ($(PORTNAME),darwin) +libpq_pgport = $(libpq) +else ifdef PGXS +libpq_pgport = -L$(libdir) -lpgcommon -lpgport $(libpq) +else +libpq_pgport = -L$(top_builddir)/src/common -lpgcommon -L$(top_builddir)/src/port -lpgport $(libpq) +endif + # libpq_pgport_shlib is the same idea, but for use in client shared libraries. +# We need those clients to use the shlib variants. (Ideally, users of this +# macro would strip libpgport and libpgcommon from $(LIBS), but no harm is +# done if they don't, since they will have satisfied all their references +# from these libraries.) ifdef PGXS -libpq_pgport = -L$(libdir) -lpgcommon -lpgport $(libpq) libpq_pgport_shlib = -L$(libdir) -lpgcommon_shlib -lpgport_shlib $(libpq) else -libpq_pgport = -L$(top_builddir)/src/common -lpgcommon -L$(top_builddir)/src/port -lpgport $(libpq) libpq_pgport_shlib = -L$(top_builddir)/src/common -lpgcommon_shlib -L$(top_builddir)/src/port -lpgport_shlib $(libpq) endif From fe2a7d63b626535653948163ca1a757293155e2f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 29 Sep 2023 20:20:57 -0400 Subject: [PATCH 43/91] Remove environment sensitivity in pl/tcl regression test. Add "-gmt 1" to our test invocations of the Tcl "clock" command, so that they do not consult the timezone environment. While it doesn't really matter which timezone is used here, it does matter that the command not fall over entirely. We've now discovered that at least on FreeBSD, "clock scan" will fail if /etc/localtime is missing. It seems worth making the test insensitive to that. Per Tomas Vondras' buildfarm animal dikkop. Thanks to Thomas Munro for the diagnosis. Discussion: https://postgr.es/m/316d304a-1dcd-cea1-3d6c-27f794727a06@enterprisedb.com --- src/pl/tcl/expected/pltcl_setup.out | 2 +- src/pl/tcl/sql/pltcl_setup.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pl/tcl/expected/pltcl_setup.out b/src/pl/tcl/expected/pltcl_setup.out index ed809f02bfb..a8fdcf31256 100644 --- a/src/pl/tcl/expected/pltcl_setup.out +++ b/src/pl/tcl/expected/pltcl_setup.out @@ -119,7 +119,7 @@ CREATE OPERATOR CLASS tcl_int4_ops -- for initialization problems. -- create function tcl_date_week(int4,int4,int4) returns text as $$ - return [clock format [clock scan "$2/$3/$1"] -format "%U"] + return [clock format [clock scan "$2/$3/$1" -gmt 1] -format "%U" -gmt 1] $$ language pltcl immutable; select tcl_date_week(2010,1,26); tcl_date_week diff --git a/src/pl/tcl/sql/pltcl_setup.sql b/src/pl/tcl/sql/pltcl_setup.sql index e9f59989b5b..b9892ea4f76 100644 --- a/src/pl/tcl/sql/pltcl_setup.sql +++ b/src/pl/tcl/sql/pltcl_setup.sql @@ -142,7 +142,7 @@ CREATE OPERATOR CLASS tcl_int4_ops -- for initialization problems. -- create function tcl_date_week(int4,int4,int4) returns text as $$ - return [clock format [clock scan "$2/$3/$1"] -format "%U"] + return [clock format [clock scan "$2/$3/$1" -gmt 1] -format "%U" -gmt 1] $$ language pltcl immutable; select tcl_date_week(2010,1,26); From eb13c22bb958542a502dd97f856fedf4990317ff Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sat, 30 Sep 2023 17:03:50 +0300 Subject: [PATCH 44/91] Fix briefly showing old progress stats for ANALYZE on inherited tables. ANALYZE on a table with inheritance children analyzes all the child tables in a loop. When stepping to next child table, it updated the child rel ID value in the command progress stats, but did not reset the 'sample_blks_total' and 'sample_blks_scanned' counters. acquire_sample_rows() updates 'sample_blks_total' as soon as the scan starts and 'sample_blks_scanned' after processing the first block, but until then, pg_stat_progress_analyze would display a bogus combination of the new child table relid with old counter values from the previously processed child table. Fix by resetting 'sample_blks_total' and 'sample_blks_scanned' to zero at the same time that 'current_child_table_relid' is updated. Backpatch to v13, where pg_stat_progress_analyze view was introduced. Reported-by: Justin Pryzby Discussion: https://www.postgresql.org/message-id/20230122162345.GP13860%40telsasoft.com --- src/backend/commands/analyze.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index a7d5025124e..fbd26f2ee9c 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -2168,8 +2168,25 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, AcquireSampleRowsFunc acquirefunc = acquirefuncs[i]; double childblocks = relblocks[i]; - pgstat_progress_update_param(PROGRESS_ANALYZE_CURRENT_CHILD_TABLE_RELID, - RelationGetRelid(childrel)); + /* + * Report progress. The sampling function will normally report blocks + * done/total, but we need to reset them to 0 here, so that they don't + * show an old value until that. + */ + { + const int progress_index[] = { + PROGRESS_ANALYZE_CURRENT_CHILD_TABLE_RELID, + PROGRESS_ANALYZE_BLOCKS_DONE, + PROGRESS_ANALYZE_BLOCKS_TOTAL + }; + const int64 progress_vals[] = { + RelationGetRelid(childrel), + 0, + 0, + }; + + pgstat_progress_update_multi_param(3, progress_index, progress_vals); + } if (childblocks > 0) { From cab9442eaf119c4b89710df596904105d3b78b94 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 1 Oct 2023 12:09:26 -0400 Subject: [PATCH 45/91] In COPY FROM, fail cleanly when unsupported encoding conversion is needed. In recent releases, such cases fail with "cache lookup failed for function 0" rather than complaining that the conversion function doesn't exist as prior versions did. Seems to be a consequence of sloppy refactoring in commit f82de5c46. Add the missing error check. Per report from Pierre Fortin. Back-patch to v14 where the oversight crept in. Discussion: https://postgr.es/m/20230929163739.3bea46e5.pfortin@pfortin.com --- src/backend/commands/copyfrom.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 1fcd9643db2..a60266b8d1d 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -2731,6 +2731,12 @@ BeginCopyFrom(ParseState *pstate, { cstate->need_transcoding = true; cstate->conversion_proc = FindDefaultConversionProc(cstate->file_encoding, GetDatabaseEncoding()); + if (!OidIsValid(cstate->conversion_proc)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("default conversion function for encoding \"%s\" to \"%s\" does not exist", + pg_encoding_to_char(cstate->file_encoding), + pg_encoding_to_char(GetDatabaseEncoding())))); } /* From c3c268dae2eaba68ad77dfafc9f11637a8304f09 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 1 Oct 2023 13:16:47 -0400 Subject: [PATCH 46/91] Fix datalen calculation in tsvectorrecv(). After receiving position data for a lexeme, tsvectorrecv() advanced its "datalen" value by (npos+1)*sizeof(WordEntry) where the correct calculation is (npos+1)*sizeof(WordEntryPos). This accidentally failed to render the constructed tsvector invalid, but it did result in leaving some wasted space approximately equal to the space consumed by the position data. That could have several bad effects: * Disk space is wasted if the received tsvector is stored into a table as-is. * A legal tsvector could get rejected with "maximum total lexeme length exceeded" if the extra space pushes it over the MAXSTRPOS limit. * In edge cases, the finished tsvector could be assigned a length larger than the allocated size of its palloc chunk, conceivably leading to SIGSEGV when the tsvector gets copied somewhere else. The odds of a field failure of this sort seem low, though valgrind testing could probably have found this. While we're here, let's express the calculation as "sizeof(uint16) + npos * sizeof(WordEntryPos)" to avoid the type pun implicit in the "npos + 1" formulation. It's not wrong given that WordEntryPos had better be 2 bytes to avoid padding problems, but it seems clearer this way. Report and patch by Denis Erokhin. Back-patch to all supported versions. Discussion: https://postgr.es/m/009801d9f2d9$f29730c0$d7c59240$@datagile.ru --- src/backend/utils/adt/tsvector.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/adt/tsvector.c b/src/backend/utils/adt/tsvector.c index 5f52e6d7595..49db4146c2e 100644 --- a/src/backend/utils/adt/tsvector.c +++ b/src/backend/utils/adt/tsvector.c @@ -496,7 +496,7 @@ tsvectorrecv(PG_FUNCTION_ARGS) * But make sure the buffer is large enough first. */ while (hdrlen + SHORTALIGN(datalen + lex_len) + - (npos + 1) * sizeof(WordEntryPos) >= len) + sizeof(uint16) + npos * sizeof(WordEntryPos) >= len) { len *= 2; vec = (TSVector) repalloc(vec, len); @@ -542,7 +542,7 @@ tsvectorrecv(PG_FUNCTION_ARGS) elog(ERROR, "position information is misordered"); } - datalen += (npos + 1) * sizeof(WordEntry); + datalen += sizeof(uint16) + npos * sizeof(WordEntryPos); } } From 242de0105eb76b79de57ef65cf89f7f6b7c3164b Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 2 Oct 2023 12:39:35 +0300 Subject: [PATCH 47/91] Flush WAL stats in bgwriter bgwriter can write out WAL, but did not flush the WAL pgstat counters, so the writes were not seen in pg_stat_wal. Back-patch to v14, where pg_stat_wal was introduced. Author: Nazir Bilal Yavuz Reviewed-by: Matthias van de Meent, Kyotaro Horiguchi Discussion: https://www.postgresql.org/message-id/CAN55FZ2FPYngovZstr%3D3w1KSEHe6toiZwrurbhspfkXe5UDocg%40mail.gmail.com --- src/backend/postmaster/bgwriter.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index d59f3d284ae..dfe4e6dc53f 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -250,6 +250,7 @@ BackgroundWriterMain(void) * Send off activity statistics to the stats collector */ pgstat_send_bgwriter(); + pgstat_send_wal(true); if (FirstCallSinceLastCheckpoint()) { From 937705c5cb07e48b64d690ac92c35759cb8d2dc0 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 3 Oct 2023 15:37:21 +0900 Subject: [PATCH 48/91] Avoid memory size overflow when allocating backend activity buffer The code in charge of copying the contents of PgBackendStatus to local memory could fail on memory allocation because of an overflow on the amount of memory to use. The overflow can happen when combining a high value track_activity_query_size (max at 1MB) with a large max_connections, when both multiplied get higher than INT32_MAX as both parameters treated as signed integers. This could for example trigger with the following functions, all calling pgstat_read_current_status(): - pg_stat_get_backend_subxact() - pg_stat_get_backend_idset() - pg_stat_get_progress_info() - pg_stat_get_activity() - pg_stat_get_db_numbackends() The change to use MemoryContextAllocHuge() has been introduced in 8d0ddccec636, so backpatch down to 12. Author: Jakub Wartak Discussion: https://postgr.es/m/CAKZiRmw8QSNVw2qNK-dznsatQqz+9DkCquxP0GHbbv1jMkGHMA@mail.gmail.com Backpatch-through: 12 --- src/backend/utils/activity/backend_status.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 217483c1c61..2c60349b808 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -777,7 +777,8 @@ pgstat_read_current_status(void) NAMEDATALEN * NumBackendStatSlots); localactivity = (char *) MemoryContextAllocHuge(backendStatusSnapContext, - pgstat_track_activity_query_size * NumBackendStatSlots); + (Size) pgstat_track_activity_query_size * + (Size) NumBackendStatSlots); #ifdef USE_SSL localsslstatus = (PgBackendSSLStatus *) MemoryContextAlloc(backendStatusSnapContext, From c149ceb053a6cb487aa25aa6fd9fba66e1126d9a Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 5 Oct 2023 20:32:14 +1300 Subject: [PATCH 49/91] Fix memory leak in Memoize code Ensure we switch to the per-tuple memory context to prevent any memory leaks of detoasted Datums in MemoizeHash_hash() and MemoizeHash_equal(). Reported-by: Orlov Aleksej Author: Orlov Aleksej, David Rowley Discussion: https://postgr.es/m/83281eed63c74e4f940317186372abfd%40cft.ru Backpatch-through: 14, where Memoize was added --- src/backend/executor/nodeMemoize.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c index c078b68740b..6a387ba8fc5 100644 --- a/src/backend/executor/nodeMemoize.c +++ b/src/backend/executor/nodeMemoize.c @@ -158,10 +158,14 @@ static uint32 MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key) { MemoizeState *mstate = (MemoizeState *) tb->private_data; + ExprContext *econtext = mstate->ss.ps.ps_ExprContext; + MemoryContext oldcontext; TupleTableSlot *pslot = mstate->probeslot; uint32 hashkey = 0; int numkeys = mstate->nkeys; + oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + if (mstate->binary_mode) { for (int i = 0; i < numkeys; i++) @@ -203,6 +207,8 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key) } } + ResetExprContext(econtext); + MemoryContextSwitchTo(oldcontext); return murmurhash32(hashkey); } @@ -226,7 +232,11 @@ MemoizeHash_equal(struct memoize_hash *tb, const MemoizeKey *key1, if (mstate->binary_mode) { + MemoryContext oldcontext; int numkeys = mstate->nkeys; + bool match = true; + + oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); slot_getallattrs(tslot); slot_getallattrs(pslot); @@ -236,7 +246,10 @@ MemoizeHash_equal(struct memoize_hash *tb, const MemoizeKey *key1, FormData_pg_attribute *attr; if (tslot->tts_isnull[i] != pslot->tts_isnull[i]) - return false; + { + match = false; + break; + } /* both NULL? they're equal */ if (tslot->tts_isnull[i]) @@ -246,9 +259,15 @@ MemoizeHash_equal(struct memoize_hash *tb, const MemoizeKey *key1, attr = &tslot->tts_tupleDescriptor->attrs[i]; if (!datum_image_eq(tslot->tts_values[i], pslot->tts_values[i], attr->attbyval, attr->attlen)) - return false; + { + match = false; + break; + } } - return true; + + ResetExprContext(econtext); + MemoryContextSwitchTo(oldcontext); + return match; } else { From 2b19fe7ab49390063dfd53c307fb63499b781a97 Mon Sep 17 00:00:00 2001 From: Etsuro Fujita Date: Fri, 6 Oct 2023 18:30:05 +0900 Subject: [PATCH 50/91] Remove extra parenthesis from comment. --- src/backend/storage/ipc/procarray.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 8eea86a16ad..09afe6c5378 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2869,7 +2869,7 @@ GetSnapshotDataReuse(Snapshot snapshot) * GetSnapshotData() cannot change while ProcArrayLock is held. Snapshot * contents only depend on transactions with xids and xactCompletionCount * is incremented whenever a transaction with an xid finishes (while - * holding ProcArrayLock) exclusively). Thus the xactCompletionCount check + * holding ProcArrayLock exclusively). Thus the xactCompletionCount check * ensures we would detect if the snapshot would have changed. * * As the snapshot contents are the same as it was before, it is safe to From 3dba7673ba0cb0594e688cdf7919511a437e3a2a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 9 Oct 2023 11:29:21 -0400 Subject: [PATCH 51/91] Doc: use CURRENT_USER not USER in plpgsql trigger examples. While these two built-in functions do exactly the same thing, CURRENT_USER seems preferable to use in documentation examples. It's easier to look up if the reader is unsure what it is. Also, this puts these examples in sync with an adjacent example that already used CURRENT_USER. Per question from Kirk Parker. Discussion: https://postgr.es/m/CANwZ8rmN_Eb0h0hoMRS8Feftaik0z89PxVsKg+cP+PctuOq=Qg@mail.gmail.com --- doc/src/sgml/plpgsql.sgml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 22fa317f7b5..8017ba392a4 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -4300,11 +4300,11 @@ CREATE OR REPLACE FUNCTION process_emp_audit() RETURNS TRIGGER AS $emp_audit$ -- making use of the special variable TG_OP to work out the operation. -- IF (TG_OP = 'DELETE') THEN - INSERT INTO emp_audit SELECT 'D', now(), user, OLD.*; + INSERT INTO emp_audit SELECT 'D', now(), current_user, OLD.*; ELSIF (TG_OP = 'UPDATE') THEN - INSERT INTO emp_audit SELECT 'U', now(), user, NEW.*; + INSERT INTO emp_audit SELECT 'U', now(), current_user, NEW.*; ELSIF (TG_OP = 'INSERT') THEN - INSERT INTO emp_audit SELECT 'I', now(), user, NEW.*; + INSERT INTO emp_audit SELECT 'I', now(), current_user, NEW.*; END IF; RETURN NULL; -- result is ignored since this is an AFTER trigger END; @@ -4370,20 +4370,20 @@ CREATE OR REPLACE FUNCTION update_emp_view() RETURNS TRIGGER AS $$ IF NOT FOUND THEN RETURN NULL; END IF; OLD.last_updated = now(); - INSERT INTO emp_audit VALUES('D', user, OLD.*); + INSERT INTO emp_audit VALUES('D', current_user, OLD.*); RETURN OLD; ELSIF (TG_OP = 'UPDATE') THEN UPDATE emp SET salary = NEW.salary WHERE empname = OLD.empname; IF NOT FOUND THEN RETURN NULL; END IF; NEW.last_updated = now(); - INSERT INTO emp_audit VALUES('U', user, NEW.*); + INSERT INTO emp_audit VALUES('U', current_user, NEW.*); RETURN NEW; ELSIF (TG_OP = 'INSERT') THEN INSERT INTO emp VALUES(NEW.empname, NEW.salary); NEW.last_updated = now(); - INSERT INTO emp_audit VALUES('I', user, NEW.*); + INSERT INTO emp_audit VALUES('I', current_user, NEW.*); RETURN NEW; END IF; END; @@ -4598,13 +4598,13 @@ CREATE OR REPLACE FUNCTION process_emp_audit() RETURNS TRIGGER AS $emp_audit$ -- IF (TG_OP = 'DELETE') THEN INSERT INTO emp_audit - SELECT 'D', now(), user, o.* FROM old_table o; + SELECT 'D', now(), current_user, o.* FROM old_table o; ELSIF (TG_OP = 'UPDATE') THEN INSERT INTO emp_audit - SELECT 'U', now(), user, n.* FROM new_table n; + SELECT 'U', now(), current_user, n.* FROM new_table n; ELSIF (TG_OP = 'INSERT') THEN INSERT INTO emp_audit - SELECT 'I', now(), user, n.* FROM new_table n; + SELECT 'I', now(), current_user, n.* FROM new_table n; END IF; RETURN NULL; -- result is ignored since this is an AFTER trigger END; From bcfc7eae6a3b9c44534645a85cda8b243f322d57 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Tue, 10 Oct 2023 11:01:13 -0700 Subject: [PATCH 52/91] Fix bug in GenericXLogFinish(). Mark the buffers dirty before writing WAL. Discussion: https://postgr.es/m/25104133-7df8-cae3-b9a2-1c0aaa1c094a@iki.fi Reviewed-by: Heikki Linnakangas Backpatch-through: 11 --- src/backend/access/transam/generic_xlog.c | 56 +++++++++++------------ 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c index 63301a1ab16..c6759fd59eb 100644 --- a/src/backend/access/transam/generic_xlog.c +++ b/src/backend/access/transam/generic_xlog.c @@ -342,6 +342,10 @@ GenericXLogFinish(GenericXLogState *state) START_CRIT_SECTION(); + /* + * Compute deltas if necessary, write changes to buffers, mark + * buffers dirty, and register changes. + */ for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++) { PageData *pageData = &state->pages[i]; @@ -354,41 +358,34 @@ GenericXLogFinish(GenericXLogState *state) page = BufferGetPage(pageData->buffer); pageHeader = (PageHeader) pageData->image; + /* + * Compute delta while we still have both the unmodified page and + * the new image. Not needed if we are logging the full image. + */ + if (!(pageData->flags & GENERIC_XLOG_FULL_IMAGE)) + computeDelta(pageData, page, (Page) pageData->image); + + /* + * Apply the image, being careful to zero the "hole" between + * pd_lower and pd_upper in order to avoid divergence between + * actual page state and what replay would produce. + */ + memcpy(page, pageData->image, pageHeader->pd_lower); + memset(page + pageHeader->pd_lower, 0, + pageHeader->pd_upper - pageHeader->pd_lower); + memcpy(page + pageHeader->pd_upper, + pageData->image + pageHeader->pd_upper, + BLCKSZ - pageHeader->pd_upper); + + MarkBufferDirty(pageData->buffer); + if (pageData->flags & GENERIC_XLOG_FULL_IMAGE) { - /* - * A full-page image does not require us to supply any xlog - * data. Just apply the image, being careful to zero the - * "hole" between pd_lower and pd_upper in order to avoid - * divergence between actual page state and what replay would - * produce. - */ - memcpy(page, pageData->image, pageHeader->pd_lower); - memset(page + pageHeader->pd_lower, 0, - pageHeader->pd_upper - pageHeader->pd_lower); - memcpy(page + pageHeader->pd_upper, - pageData->image + pageHeader->pd_upper, - BLCKSZ - pageHeader->pd_upper); - XLogRegisterBuffer(i, pageData->buffer, REGBUF_FORCE_IMAGE | REGBUF_STANDARD); } else { - /* - * In normal mode, calculate delta and write it as xlog data - * associated with this page. - */ - computeDelta(pageData, page, (Page) pageData->image); - - /* Apply the image, with zeroed "hole" as above */ - memcpy(page, pageData->image, pageHeader->pd_lower); - memset(page + pageHeader->pd_lower, 0, - pageHeader->pd_upper - pageHeader->pd_lower); - memcpy(page + pageHeader->pd_upper, - pageData->image + pageHeader->pd_upper, - BLCKSZ - pageHeader->pd_upper); - XLogRegisterBuffer(i, pageData->buffer, REGBUF_STANDARD); XLogRegisterBufData(i, pageData->delta, pageData->deltaLen); } @@ -397,7 +394,7 @@ GenericXLogFinish(GenericXLogState *state) /* Insert xlog record */ lsn = XLogInsert(RM_GENERIC_ID, 0); - /* Set LSN and mark buffers dirty */ + /* Set LSN */ for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++) { PageData *pageData = &state->pages[i]; @@ -405,7 +402,6 @@ GenericXLogFinish(GenericXLogState *state) if (BufferIsInvalid(pageData->buffer)) continue; PageSetLSN(BufferGetPage(pageData->buffer), lsn); - MarkBufferDirty(pageData->buffer); } END_CRIT_SECTION(); } From 6fddb0acf9f2b27cd18f0a9be9dc82419cf2a928 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 10 Oct 2023 15:14:18 -0400 Subject: [PATCH 53/91] doc: document the need to analyze partitioned tables Autovacuum does not do it. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20210913035409.GA10647@telsasoft.com Backpatch-through: 11 --- doc/src/sgml/maintenance.sgml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml index eaffb5a6592..0b5860ebbbd 100644 --- a/doc/src/sgml/maintenance.sgml +++ b/doc/src/sgml/maintenance.sgml @@ -841,10 +841,15 @@ analyze threshold = analyze base threshold + analyze scale factor * number of tu - Partitioned tables are not processed by autovacuum. Statistics - should be collected by running a manual ANALYZE when it is - first populated, and again whenever the distribution of data in its - partitions changes significantly. + Partitioned tables do not directly store tuples and consequently + are not processed by autovacuum. (Autovacuum does process table + partitions just like other tables.) Unfortunately, this means that + autovacuum does not run ANALYZE on partitioned + tables, and this can cause suboptimal plans for queries that reference + partitioned table statistics. You can work around this problem by + manually running ANALYZE on partitioned tables + when they are first populated, and again whenever the distribution + of data in their partitions changes significantly. From f71fbb230582e3ed06dfe571321242f5e1b9e79a Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 10 Oct 2023 15:54:28 -0400 Subject: [PATCH 54/91] doc: add SSL configuration section reference Reported-by: Steve Atkins Discussion: https://postgr.es/m/B82E80DD-1452-4175-B19C-564FE46705BA@blighty.com Backpatch-through: 11 --- doc/src/sgml/client-auth.sgml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index eb5e9f48db1..e99bb5ae59f 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -2048,7 +2048,8 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" This authentication method uses SSL client certificates to perform - authentication. It is therefore only available for SSL connections. + authentication. It is therefore only available for SSL connections; + see for SSL configuration instructions. When using this authentication method, the server will require that the client provide a valid, trusted certificate. No password prompt will be sent to the client. The cn (Common Name) From 3d4ef0d73ec6550bdbfd6ac417071a64aa698360 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 10 Oct 2023 16:04:56 -0400 Subject: [PATCH 55/91] doc: foreign servers with pushdown need matching collation Reported-by: Pete Storer Discussion: https://postgr.es/m/BL0PR05MB66283C57D72E321591AE4EB1F3CE9@BL0PR05MB6628.namprd05.prod.outlook.com Backpatch-through: 11 --- doc/src/sgml/ref/create_server.sgml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/src/sgml/ref/create_server.sgml b/doc/src/sgml/ref/create_server.sgml index ca5be066f96..fe3098a079c 100644 --- a/doc/src/sgml/ref/create_server.sgml +++ b/doc/src/sgml/ref/create_server.sgml @@ -148,6 +148,11 @@ CREATE SERVER [ IF NOT EXISTS ] server_nameUSAGE privilege on the foreign server to be able to use it in this way. + + + If the foreign server supports sort pushdown, it is necessary for it + to have the same sort ordering as the local server. + From f29fa10bce6190e840d44f859fed702b79c8ca17 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 10 Oct 2023 16:51:08 -0400 Subject: [PATCH 56/91] doc: clarify that SSPI and GSSAPI are interchangeable Reported-by: tpo_deb@sourcepole.ch Discussion: https://postgr.es/m/167846222574.1803490.15815104179136215862@wrigleys.postgresql.org Backpatch-through: 11 --- doc/src/sgml/client-auth.sgml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index e99bb5ae59f..44f8fd02b0c 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -1388,10 +1388,12 @@ omicron bryanh guest1 negotiate mode, which will use Kerberos when possible and automatically fall back to NTLM in other cases. - SSPI authentication only works when both - server and client are running Windows, - or, on non-Windows platforms, when GSSAPI - is available. + SSPI and GSSAPI + interoperate as clients and servers, e.g., an + SSPI client can authenticate to an + GSSAPI server. It is recommended to use + SSPI on Windows clients and servers and + GSSAPI on non-Windows platforms. From d1617791a6ec6a70507a7268daab63da59c8574a Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 12 Oct 2023 21:16:43 +1300 Subject: [PATCH 57/91] Doc: fix grammatical errors for enable_partitionwise_aggregate Author: Andrew Atkinson Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/CAG6XLEnC%3DEgq0YHRic2kWWDs4xwQnQ_kBA6qhhzAq1-pO_9Tfw%40mail.gmail.com Backpatch-through: 11, where enable_partitionwise_aggregate was added --- doc/src/sgml/config.sgml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index edbd5968ec7..0396704f3d8 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -5175,13 +5175,14 @@ ANY num_sync ( fds.fd_count + 1 >= FD_SETSIZE) + { + pg_log_fatal("too many concurrent database clients for this platform: %d", + sa->fds.fd_count + 1); + exit(1); + } +#else if (fd < 0 || fd >= FD_SETSIZE) { - /* - * Doing a hard exit here is a bit grotty, but it doesn't seem worth - * complicating the API to make it less grotty. - */ - pg_log_fatal("too many client connections for select()"); + pg_log_fatal("socket file descriptor out of range for select(): %d", + fd); exit(1); } +#endif FD_SET(fd, &sa->fds); if (fd > sa->maxfd) sa->maxfd = fd; diff --git a/src/fe_utils/parallel_slot.c b/src/fe_utils/parallel_slot.c index dcdad9e30c5..8d558064343 100644 --- a/src/fe_utils/parallel_slot.c +++ b/src/fe_utils/parallel_slot.c @@ -297,11 +297,40 @@ connect_slot(ParallelSlotArray *sa, int slotno, const char *dbname) slot->connection = connectDatabase(sa->cparams, sa->progname, sa->echo, false, true); sa->cparams->override_dbname = old_override; - if (PQsocket(slot->connection) >= FD_SETSIZE) + /* + * POSIX defines FD_SETSIZE as the highest file descriptor acceptable to + * FD_SET() and allied macros. Windows defines it as a ceiling on the + * count of file descriptors in the set, not a ceiling on the value of + * each file descriptor; see + * https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select + * and + * https://learn.microsoft.com/en-us/windows/win32/api/winsock/ns-winsock-fd_set. + * We can't ignore that, because Windows starts file descriptors at a + * higher value, delays reuse, and skips values. With less than ten + * concurrent file descriptors, opened and closed rapidly, one can reach + * file descriptor 1024. + * + * Doing a hard exit here is a bit grotty, but it doesn't seem worth + * complicating the API to make it less grotty. + */ +#ifdef WIN32 + if (slotno >= FD_SETSIZE) { - pg_log_fatal("too many jobs for this platform"); + pg_log_fatal("too many jobs for this platform: %d", slotno); exit(1); } +#else + { + int fd = PQsocket(slot->connection); + + if (fd >= FD_SETSIZE) + { + pg_log_fatal("socket file descriptor out of range for select(): %d", + fd); + exit(1); + } + } +#endif /* Setup the connection using the supplied command, if any. */ if (sa->initcmd) From b7686621a115904c80788ca656cf4eb32620f431 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 16 Oct 2023 10:43:47 +1300 Subject: [PATCH 59/91] Acquire ControlFileLock in relevant SQL functions. Commit dc7d70ea added functions that read the control file, but didn't acquire ControlFileLock. With unlucky timing, file systems that have weak interlocking like ext4 and ntfs could expose partially overwritten contents, and the checksum would fail. Back-patch to all supported releases. Reviewed-by: David Steele Reviewed-by: Anton A. Melnikov Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de --- src/backend/utils/misc/pg_controldata.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c index 908179cdf8b..6b695064e82 100644 --- a/src/backend/utils/misc/pg_controldata.c +++ b/src/backend/utils/misc/pg_controldata.c @@ -24,6 +24,7 @@ #include "common/controldata_utils.h" #include "funcapi.h" #include "miscadmin.h" +#include "storage/lwlock.h" #include "utils/builtins.h" #include "utils/pg_lsn.h" #include "utils/timestamp.h" @@ -56,7 +57,9 @@ pg_control_system(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* read the control file */ + LWLockAcquire(ControlFileLock, LW_SHARED); ControlFile = get_controlfile(DataDir, &crc_ok); + LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); @@ -134,7 +137,9 @@ pg_control_checkpoint(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* Read the control file. */ + LWLockAcquire(ControlFileLock, LW_SHARED); ControlFile = get_controlfile(DataDir, &crc_ok); + LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); @@ -237,7 +242,9 @@ pg_control_recovery(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* read the control file */ + LWLockAcquire(ControlFileLock, LW_SHARED); ControlFile = get_controlfile(DataDir, &crc_ok); + LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); @@ -304,7 +311,9 @@ pg_control_init(PG_FUNCTION_ARGS) tupdesc = BlessTupleDesc(tupdesc); /* read the control file */ + LWLockAcquire(ControlFileLock, LW_SHARED); ControlFile = get_controlfile(DataDir, &crc_ok); + LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, (errmsg("calculated CRC checksum does not match value stored in file"))); From 8317eafb5af7895eeef09810011359cd97151b35 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 16 Oct 2023 17:10:13 +1300 Subject: [PATCH 60/91] Try to handle torn reads of pg_control in frontend. Some of our src/bin tools read the control file without any kind of interlocking against concurrent writes from the server. At least ext4 and ntfs can expose partially modified contents when you do that. For now, we'll try to tolerate this by retrying up to 10 times if the checksum doesn't match, until we get two reads in a row with the same bad checksum. This is not guaranteed to reach the right conclusion, but it seems very likely to. Thanks to Tom Lane for this suggestion. Various ideas for interlocking or atomicity were considered too complicated, unportable or expensive given the lack of field reports, but remain open for future reconsideration. Back-patch as far as 12. It doesn't seem like a good idea to put a heuristic change for a very rare problem into the final release of 11. Reviewed-by: Anton A. Melnikov Reviewed-by: David Steele Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de --- src/common/controldata_utils.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c index 7d4af7881ea..c6f133fe0c0 100644 --- a/src/common/controldata_utils.c +++ b/src/common/controldata_utils.c @@ -55,12 +55,22 @@ get_controlfile(const char *DataDir, bool *crc_ok_p) char ControlFilePath[MAXPGPATH]; pg_crc32c crc; int r; +#ifdef FRONTEND + pg_crc32c last_crc; + int retries = 0; +#endif AssertArg(crc_ok_p); ControlFile = palloc(sizeof(ControlFileData)); snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir); +#ifdef FRONTEND + INIT_CRC32C(last_crc); + +retry: +#endif + #ifndef FRONTEND if ((fd = OpenTransientFile(ControlFilePath, O_RDONLY | PG_BINARY)) == -1) ereport(ERROR, @@ -128,6 +138,26 @@ get_controlfile(const char *DataDir, bool *crc_ok_p) *crc_ok_p = EQ_CRC32C(crc, ControlFile->crc); +#ifdef FRONTEND + + /* + * If the server was writing at the same time, it is possible that we read + * partially updated contents on some systems. If the CRC doesn't match, + * retry a limited number of times until we compute the same bad CRC twice + * in a row with a short sleep in between. Then the failure is unlikely + * to be due to a concurrent write. + */ + if (!*crc_ok_p && + (retries == 0 || !EQ_CRC32C(crc, last_crc)) && + retries < 10) + { + retries++; + last_crc = crc; + pg_usleep(10000); + goto retry; + } +#endif + /* Make sure the control file is valid byte order. */ if (ControlFile->pg_control_version % 65536 == 0 && ControlFile->pg_control_version / 65536 != 0) From 9df98ec9473164f5fdb6f569920abfb2e67d8385 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 16 Oct 2023 12:57:39 -0400 Subject: [PATCH 61/91] Update the documentation on recovering from (M)XID exhaustion. The old documentation encourages entering single-user mode for no reason, which is a bad plan in most cases. Instead, discourage users from doing that, and explain the limited cases in which it may be desirable. The old documentation claims that running VACUUM as anyone but the superuser can't possibly work, which is not really true, because it might be that some other user has enough permissions to VACUUM all the tables that matter. Weaken the language just a bit. The old documentation claims that you can't run any commands when near XID exhaustion, which is false because you can still run commands that don't require an XID, like a SELECT without a locking clause. The old documentation doesn't clearly explain that it's a good idea to get rid of prepared transactons, long-running transactions, and replication slots that are preventing (M)XID horizon advancement. Spell out the steps to do that. Also, discourage the use of VACUUM FULL and VACUUM FREEZE in this type of scenario. Back-patch to v14. Much of this is good advice on all supported versions, but before 60f1f09ff44308667ef6c72fbafd68235e55ae27 the chances of VACUUM failing in multi-user mode were much higher. Alexander Alekseev, John Naylor, Robert Haas, reviewed at various times by Peter Geoghegan, Hannu Krosing, and Andres Freund. Discussion: http://postgr.es/m/CA+TgmoYtsUDrzaHcmjFhLzTk1VEv29mO_u-MT+XWHrBJ_4nD8A@mail.gmail.com --- doc/src/sgml/maintenance.sgml | 112 +++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml index 0b5860ebbbd..4cd243f361e 100644 --- a/doc/src/sgml/maintenance.sgml +++ b/doc/src/sgml/maintenance.sgml @@ -641,29 +641,79 @@ HINT: To avoid a database shutdown, execute a database-wide VACUUM in that data (A manual VACUUM should fix the problem, as suggested by the - hint; but note that the VACUUM must be performed by a - superuser, else it will fail to process system catalogs and thus not - be able to advance the database's datfrozenxid.) - If these warnings are - ignored, the system will shut down and refuse to start any new - transactions once there are fewer than three million transactions left - until wraparound: + hint; but note that the VACUUM should be performed by a + superuser, else it will fail to process system catalogs, which prevent it from + being able to advance the database's datfrozenxid.) + If these warnings are ignored, the system will refuse to assign new XIDs once + there are fewer than three million transactions left until wraparound: ERROR: database is not accepting commands to avoid wraparound data loss in database "mydb" HINT: Stop the postmaster and vacuum that database in single-user mode. - The three-million-transaction safety margin exists to let the - administrator recover without data loss, by manually executing the - required VACUUM commands. However, since the system will not - execute commands once it has gone into the safety shutdown mode, - the only way to do this is to stop the server and start the server in single-user - mode to execute VACUUM. The shutdown mode is not enforced - in single-user mode. See the reference - page for details about using single-user mode. + In this condition any transactions already in progress can continue, + but only read-only transactions can be started. Operations that + modify database records or truncate relations will fail. + The VACUUM command can still be run normally. + Contrary to what the hint states, it is not necessary or desirable to stop the + postmaster or enter single user-mode in order to restore normal operation. + Instead, follow these steps: + + + + Resolve old prepared transactions. You can find these by checking + pg_prepared_xacts for rows where + age(transactionid) is large. Such transactions should be + committed or rolled back. + + + End long-running open transactions. You can find these by checking + pg_stat_activity for rows where + age(backend_xid) or age(backend_xmin) is + large. Such transactions should be committed or rolled back, or the session + can be terminated using pg_terminate_backend. + + + Drop any old replication slots. Use + pg_stat_replication to + find slots where age(xmin) or age(catalog_xmin) + is large. In many cases, such slots were created for replication to servers that no + longer exist, or that have been down for a long time. If you drop a slot for a server + that still exists and might still try to connect to that slot, that replica may + need to be rebuilt. + + + Execute VACUUM in the target database. A database-wide + VACUUM is simplest; to reduce the time required, it as also possible + to issue manual VACUUM commands on the tables where + relminxid is oldest. Do not use VACUUM FULL + in this scenario, because it requires an XID and will therefore fail, except in super-user + mode, where it will instead consume an XID and thus increase the risk of transaction ID + wraparound. Do not use VACUUM FREEZE either, because it will do + more than the minimum amount of work required to restore normal operation. + + + Once normal operation is restored, ensure that autovacuum is properly configured + in the target database in order to avoid future problems. + + + + + In earlier versions, it was sometimes necessary to stop the postmaster and + VACUUM the database in a single-user mode. In typical scenarios, this + is no longer necessary, and should be avoided whenever possible, since it involves taking + the system down. It is also riskier, since it disables transaction ID wraparound safeguards + that are designed to prevent data loss. The only reason to use single-user mode in this + scenario is if you wish to TRUNCATE or DROP unneeded + tables to avoid needing to VACUUM them. The three-million-transaction + safety margin exists to let the administrator do this. See the + reference page for details about using single-user mode. + + + Multixacts and Wraparound @@ -727,6 +777,38 @@ HINT: Stop the postmaster and vacuum that database in single-user mode. have the oldest multixact-age. Both of these kinds of aggressive scans will occur even if autovacuum is nominally disabled. + + + Similar to the XID case, if autovacuum fails to clear old MXIDs from a table, the + system will begin to emit warning messages when the database's oldest MXIDs reach forty + million transactions from the wraparound point. And, just as an the XID case, if these + warnings are ignored, the system will refuse to generate new MXIDs once there are fewer + than three million left until wraparound. + + + + Normal operation when MXIDs are exhausted can be restored in much the same way as + when XIDs are exhausted. Follow the same steps in the previous section, but with the + following differences: + + + + Running transactions and prepared transactions can be ignored if there + is no chance that they might appear in a multixact. + + + MXID information is not directly visible in system views such as + pg_stat_activity; however, looking for old XIDs is still a good + way of determining which transactions are causing MXID wraparound problems. + + + XID exhaustion will block all write transactions, but MXID exhaustion will + only block a subset of write transactions, specifically those that involve + row locks that require an MXID. + + + + From 0a13a7d75fbfa30d69166eb447ec62d8f5ec1c89 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 16 Oct 2023 14:06:11 -0400 Subject: [PATCH 62/91] Ensure we have a snapshot while dropping ON COMMIT DROP temp tables. Dropping a temp table could entail TOAST table access to clean out toasted catalog entries, such as large pg_constraint.conbin strings for complex CHECK constraints. If we did that via ON COMMIT DROP, we triggered the assertion in init_toast_snapshot(), because there was no provision for setting up a snapshot for the drop actions. Fix that. (I assume here that the adjacent truncation actions for ON COMMIT DELETE ROWS don't have a similar problem: it doesn't seem like nontransactional truncations would need to touch any toasted fields. If that proves wrong, we could refactor a bit to have the same snapshot acquisition cover that too.) The test case added here does not fail before v15, because that assertion was added in 277692220 which was not back-patched. However, the race condition the assertion warns of surely exists further back, so back-patch to all supported branches. Per report from Richard Guo. Discussion: https://postgr.es/m/CAMbWs4-x26=_QxxgdJyNbiCDzvtr2WV5ZDso_v-CukKEe6cBZw@mail.gmail.com --- src/backend/commands/tablecmds.c | 8 ++++++++ src/test/regress/expected/temp.out | 20 ++++++++++++++++++++ src/test/regress/sql/temp.sql | 16 ++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c51443b77cc..d21b2800577 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -20659,6 +20659,12 @@ PreCommit_on_commit_actions(void) add_exact_object_address(&object, targetObjects); } + /* + * Object deletion might involve toast table access (to clean up + * toasted catalog entries), so ensure we have a valid snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + /* * Since this is an automatic drop, rather than one directly initiated * by the user, we pass the PERFORM_DELETION_INTERNAL flag. @@ -20666,6 +20672,8 @@ PreCommit_on_commit_actions(void) performMultipleDeletions(targetObjects, DROP_CASCADE, PERFORM_DELETION_INTERNAL | PERFORM_DELETION_QUIETLY); + PopActiveSnapshot(); + #ifdef USE_ASSERT_CHECKING /* diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out index 46186faa221..ec705636482 100644 --- a/src/test/regress/expected/temp.out +++ b/src/test/regress/expected/temp.out @@ -108,6 +108,26 @@ SELECT * FROM temptest; 1 (1 row) +COMMIT; +SELECT * FROM temptest; +ERROR: relation "temptest" does not exist +LINE 1: SELECT * FROM temptest; + ^ +-- Test it with a CHECK condition that produces a toasted pg_constraint entry +BEGIN; +do $$ +begin + execute format($cmd$ + CREATE TEMP TABLE temptest (col text CHECK (col < %L)) ON COMMIT DROP + $cmd$, + (SELECT string_agg(g.i::text || ':' || random()::text, '|') + FROM generate_series(1, 100) g(i))); +end$$; +SELECT * FROM temptest; + col +----- +(0 rows) + COMMIT; SELECT * FROM temptest; ERROR: relation "temptest" does not exist diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql index 2eeeb4a65e2..75616b53e1a 100644 --- a/src/test/regress/sql/temp.sql +++ b/src/test/regress/sql/temp.sql @@ -101,6 +101,22 @@ COMMIT; SELECT * FROM temptest; +-- Test it with a CHECK condition that produces a toasted pg_constraint entry +BEGIN; +do $$ +begin + execute format($cmd$ + CREATE TEMP TABLE temptest (col text CHECK (col < %L)) ON COMMIT DROP + $cmd$, + (SELECT string_agg(g.i::text || ':' || random()::text, '|') + FROM generate_series(1, 100) g(i))); +end$$; + +SELECT * FROM temptest; +COMMIT; + +SELECT * FROM temptest; + -- ON COMMIT is only allowed for TEMP CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS; From bb23da733bb49b521d4a350043a52e28013c945d Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 17 Oct 2023 10:42:12 -0500 Subject: [PATCH 63/91] Avoid calling proc_exit() in processes forked by system(). The SIGTERM handler for the startup process immediately calls proc_exit() for the duration of the restore_command, i.e., a call to system(). This system() call forks a new process to execute the shell command, and this child process inherits the parent's signal handlers. If both the parent and child processes receive SIGTERM, both will attempt to call proc_exit(). This can end badly. For example, both processes will try to remove themselves from the PGPROC shared array. To fix this problem, this commit adds a check in StartupProcShutdownHandler() to see whether MyProcPid == getpid(). If they match, this is the parent process, and we can proc_exit() like before. If they do not match, this is a child process, and we just emit a message to STDERR (in a signal safe manner) and _exit(), thereby skipping any problematic exit callbacks. This commit also adds checks in proc_exit(), ProcKill(), and AuxiliaryProcKill() that verify they are not being called within such child processes. Suggested-by: Andres Freund Reviewed-by: Thomas Munro, Andres Freund Discussion: https://postgr.es/m/Y9nGDSgIm83FHcad%40paquier.xyz Discussion: https://postgr.es/m/20230223231503.GA743455%40nathanxps13 Backpatch-through: 11 --- src/backend/postmaster/startup.c | 17 ++++++++++++++++- src/backend/utils/error/elog.c | 28 ++++++++++++++++++++++++++++ src/include/utils/elog.h | 5 +++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index c8752b5cc79..a9fbf6d52d8 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -19,6 +19,8 @@ */ #include "postgres.h" +#include + #include "access/xlog.h" #include "libpq/pqsignal.h" #include "miscadmin.h" @@ -103,7 +105,20 @@ StartupProcShutdownHandler(SIGNAL_ARGS) int save_errno = errno; if (in_restore_command) - proc_exit(1); + { + /* + * If we are in a child process (e.g., forked by system() in + * RestoreArchivedFile()), we don't want to call any exit callbacks. + * The parent will take care of that. + */ + if (MyProcPid == (int) getpid()) + proc_exit(1); + else + { + write_stderr_signal_safe("StartupProcShutdownHandler() called in child process\n"); + _exit(1); + } + } else shutdown_requested = true; WakeupRecovery(); diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 859ac9e56fa..2f78f5750c2 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -5056,6 +5056,34 @@ write_stderr(const char *fmt,...) va_end(ap); } +/* + * Write a message to STDERR using only async-signal-safe functions. This can + * be used to safely emit a message from a signal handler. + * + * TODO: It is likely possible to safely do a limited amount of string + * interpolation (e.g., %s and %d), but that is not presently supported. + */ +void +write_stderr_signal_safe(const char *str) +{ + int nwritten = 0; + int ntotal = strlen(str); + + while (nwritten < ntotal) + { + int rc; + + rc = write(STDERR_FILENO, str + nwritten, ntotal - nwritten); + + /* Just give up on error. There isn't much else we can do. */ + if (rc == -1) + return; + + nwritten += rc; + } +} + + /* * Adjust the level of a recovery-related message per trace_recovery_messages. * diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 0b39f8e760d..001bc08a3a5 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -666,5 +666,10 @@ extern bool gp_log_stack_trace_lines; /* session GUC, controls line info in st extern const char *SegvBusIllName(int signal); extern void StandardHandlerForSigillSigsegvSigbus_OnMainThread(char * processName, SIGNAL_ARGS); +/* + * Write a message to STDERR using only async-signal-safe functions. This can + * be used to safely emit a message from a signal handler. + */ +extern void write_stderr_signal_safe(const char *fmt); #endif /* ELOG_H */ From 0e959240e308fbc754f9ae0cc9f6327a85109f26 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 17 Oct 2023 13:55:45 -0400 Subject: [PATCH 64/91] Back-patch test cases for timetz_zone/timetz_izone. Per code coverage reports, we had zero regression test coverage of these functions. That came back to bite us, as apparently that's allowed us to miss discovering misbehavior of this code with AIX's xlc compiler. Install relevant portions of the test cases added in 97957fdba, 2f0472030, 19fa97731. (Assuming the expected outcome that the xlc problem does appear in back branches, a code fix will follow.) Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com --- src/test/regress/expected/timetz.out | 60 ++++++++++++++++++++++++++++ src/test/regress/sql/timetz.sql | 20 ++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expected/timetz.out index 62084eb5f64..7f0e5e64ac8 100644 --- a/src/test/regress/expected/timetz.out +++ b/src/test/regress/expected/timetz.out @@ -275,3 +275,63 @@ SELECT date_part('epoch', TIME WITH TIME ZONE '2020-05-26 13:30:25.575401- 63025.575401 (1 row) +-- +-- Test timetz_zone, timetz_izone +-- +BEGIN; +SET LOCAL TimeZone TO 'UTC'; +CREATE VIEW timetz_local_view AS + SELECT f1 AS dat, + f1 AT TIME ZONE current_setting('TimeZone') AS dat_at_tz, + f1 AT TIME ZONE INTERVAL '00:00' AS dat_at_int + FROM TIMETZ_TBL + ORDER BY f1; +SELECT pg_get_viewdef('timetz_local_view', true); + pg_get_viewdef +---------------------------------------------------------------------------------- + SELECT timetz_tbl.f1 AS dat, + + (timetz_tbl.f1 AT TIME ZONE current_setting('TimeZone'::text)) AS dat_at_tz,+ + (timetz_tbl.f1 AT TIME ZONE '@ 0'::interval) AS dat_at_int + + FROM timetz_tbl + + ORDER BY timetz_tbl.f1; +(1 row) + +TABLE timetz_local_view; + dat | dat_at_tz | dat_at_int +----------------+----------------+---------------- + 00:01:00-07 | 07:01:00+00 | 07:01:00+00 + 01:00:00-07 | 08:00:00+00 | 08:00:00+00 + 02:03:00-07 | 09:03:00+00 | 09:03:00+00 + 08:08:00-04 | 12:08:00+00 | 12:08:00+00 + 07:07:00-08 | 15:07:00+00 | 15:07:00+00 + 11:59:00-07 | 18:59:00+00 | 18:59:00+00 + 12:00:00-07 | 19:00:00+00 | 19:00:00+00 + 12:01:00-07 | 19:01:00+00 | 19:01:00+00 + 15:36:39-04 | 19:36:39+00 | 19:36:39+00 + 15:36:39-05 | 20:36:39+00 | 20:36:39+00 + 23:59:00-07 | 06:59:00+00 | 06:59:00+00 + 23:59:59.99-07 | 06:59:59.99+00 | 06:59:59.99+00 +(12 rows) + +SELECT f1 AS dat, + f1 AT TIME ZONE 'UTC+10' AS dat_at_tz, + f1 AT TIME ZONE INTERVAL '-10:00' AS dat_at_int + FROM TIMETZ_TBL + ORDER BY f1; + dat | dat_at_tz | dat_at_int +----------------+----------------+---------------- + 00:01:00-07 | 21:01:00-10 | 21:01:00-10 + 01:00:00-07 | 22:00:00-10 | 22:00:00-10 + 02:03:00-07 | 23:03:00-10 | 23:03:00-10 + 08:08:00-04 | 02:08:00-10 | 02:08:00-10 + 07:07:00-08 | 05:07:00-10 | 05:07:00-10 + 11:59:00-07 | 08:59:00-10 | 08:59:00-10 + 12:00:00-07 | 09:00:00-10 | 09:00:00-10 + 12:01:00-07 | 09:01:00-10 | 09:01:00-10 + 15:36:39-04 | 09:36:39-10 | 09:36:39-10 + 15:36:39-05 | 10:36:39-10 | 10:36:39-10 + 23:59:00-07 | 20:59:00-10 | 20:59:00-10 + 23:59:59.99-07 | 20:59:59.99-10 | 20:59:59.99-10 +(12 rows) + +ROLLBACK; diff --git a/src/test/regress/sql/timetz.sql b/src/test/regress/sql/timetz.sql index 308e095eb42..eca8ceab12b 100644 --- a/src/test/regress/sql/timetz.sql +++ b/src/test/regress/sql/timetz.sql @@ -92,3 +92,23 @@ SELECT date_part('microsecond', TIME WITH TIME ZONE '2020-05-26 13:30:25.575401- SELECT date_part('millisecond', TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); SELECT date_part('second', TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); SELECT date_part('epoch', TIME WITH TIME ZONE '2020-05-26 13:30:25.575401-04'); + +-- +-- Test timetz_zone, timetz_izone +-- +BEGIN; +SET LOCAL TimeZone TO 'UTC'; +CREATE VIEW timetz_local_view AS + SELECT f1 AS dat, + f1 AT TIME ZONE current_setting('TimeZone') AS dat_at_tz, + f1 AT TIME ZONE INTERVAL '00:00' AS dat_at_int + FROM TIMETZ_TBL + ORDER BY f1; +SELECT pg_get_viewdef('timetz_local_view', true); +TABLE timetz_local_view; +SELECT f1 AS dat, + f1 AT TIME ZONE 'UTC+10' AS dat_at_tz, + f1 AT TIME ZONE INTERVAL '-10:00' AS dat_at_int + FROM TIMETZ_TBL + ORDER BY f1; +ROLLBACK; From dcb839ef8684062f9c31e855252d5d3ee4214b05 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 17 Oct 2023 16:11:03 -0500 Subject: [PATCH 65/91] windows: msvc: Define STDIN/OUT/ERR_FILENO. This commit (c290e79cf0) was originally back-patched to v15. Commit 97550c0711 added a new use of STDERR_FILENO, and it was back-patched all the way to v11, thus breaking MSVC builds for v11 through v14. Since STDERR_FILENO is now needed on older versions, let's back-patch c290e79cf0 down to v11, too. Here follows the original commit message describing the change: Because they are not available we've used _fileno(stdin) in some places, but that doesn't reliably work, because stdin might be closed. This is the prerequisite of the subsequent commit, fixing a failure introduced in 76e38b37a5. Author: Andres Freund Reported-By: Sandeep Thakkar Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers) Discussion: https://postgr.es/m/20231017164517.GA613565%40nathanxps13 Backpatch-through: 11 --- src/include/port/win32_msvc/unistd.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/include/port/win32_msvc/unistd.h b/src/include/port/win32_msvc/unistd.h index b63f4770a1c..b7795ba03c4 100644 --- a/src/include/port/win32_msvc/unistd.h +++ b/src/include/port/win32_msvc/unistd.h @@ -1 +1,9 @@ /* src/include/port/win32_msvc/unistd.h */ + +/* + * MSVC does not define these, nor does _fileno(stdin) etc reliably work + * (returns -1 if stdin/out/err are closed). + */ +#define STDIN_FILENO 0 +#define STDOUT_FILENO 1 +#define STDERR_FILENO 2 From 0a64fbafd19984cdaa4fbd8608a0bff3020accef Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 18 Oct 2023 22:09:05 +1300 Subject: [PATCH 66/91] jit: Support opaque pointers in LLVM 16. Remove use of LLVMGetElementType() and provide the type of all pointers to LLVMBuildXXX() functions when emitting IR, as required by modern LLVM versions[1]. * For LLVM <= 14, we'll still use the old LLVMBuildXXX() functions. * For LLVM == 15, we'll continue to do the same, explicitly opting out of opaque pointer mode. * For LLVM >= 16, we'll use the new LLVMBuildXXX2() functions that take the extra type argument. The difference is hidden behind some new IR emitting wrapper functions l_load(), l_gep(), l_call() etc. The change is mostly mechanical, except that at each site the correct type had to be provided. In some places we needed to do some extra work to get functions types, including some new wrappers for C++ APIs that are not yet exposed by in LLVM's C API, and some new "example" functions in llvmjit_types.c because it's no longer possible to start from the function pointer type and ask for the function type. Back-patch to 12, because it's a little tricker in 11 and we agreed not to put the latest LLVM support into the upcoming final release of 11. [1] https://llvm.org/docs/OpaquePointers.html Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Ronan Dunklau Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CA%2BhUKGKNX_%3Df%2B1C4r06WETKTq0G4Z_7q4L4Fxn5WWpMycDj9Fw%40mail.gmail.com --- src/backend/jit/llvm/llvmjit.c | 57 ++-- src/backend/jit/llvm/llvmjit_deform.c | 119 ++++---- src/backend/jit/llvm/llvmjit_expr.c | 397 ++++++++++++++++---------- src/backend/jit/llvm/llvmjit_types.c | 39 ++- src/backend/jit/llvm/llvmjit_wrap.cpp | 12 + src/include/jit/llvmjit.h | 6 + src/include/jit/llvmjit_emit.h | 106 +++++-- 7 files changed, 475 insertions(+), 261 deletions(-) diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index a649deab4b8..a58ac6d11db 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -85,8 +85,11 @@ LLVMTypeRef StructExprState; LLVMTypeRef StructAggState; LLVMTypeRef StructAggStatePerGroupData; LLVMTypeRef StructAggStatePerTransData; +LLVMTypeRef StructPlanState; LLVMValueRef AttributeTemplate; +LLVMValueRef ExecEvalSubroutineTemplate; +LLVMValueRef ExecEvalBoolSubroutineTemplate; LLVMModuleRef llvm_types_module = NULL; @@ -384,11 +387,7 @@ llvm_pg_var_type(const char *varname) if (!v_srcvar) elog(ERROR, "variable %s not in llvmjit_types.c", varname); - /* look at the contained type */ - typ = LLVMTypeOf(v_srcvar); - Assert(typ != NULL && LLVMGetTypeKind(typ) == LLVMPointerTypeKind); - typ = LLVMGetElementType(typ); - Assert(typ != NULL); + typ = LLVMGlobalGetValueType(v_srcvar); return typ; } @@ -400,12 +399,14 @@ llvm_pg_var_type(const char *varname) LLVMTypeRef llvm_pg_var_func_type(const char *varname) { - LLVMTypeRef typ = llvm_pg_var_type(varname); + LLVMValueRef v_srcvar; + LLVMTypeRef typ; + + v_srcvar = LLVMGetNamedFunction(llvm_types_module, varname); + if (!v_srcvar) + elog(ERROR, "function %s not in llvmjit_types.c", varname); - /* look at the contained type */ - Assert(LLVMGetTypeKind(typ) == LLVMPointerTypeKind); - typ = LLVMGetElementType(typ); - Assert(typ != NULL && LLVMGetTypeKind(typ) == LLVMFunctionTypeKind); + typ = LLVMGetFunctionType(v_srcvar); return typ; } @@ -435,7 +436,7 @@ llvm_pg_func(LLVMModuleRef mod, const char *funcname) v_fn = LLVMAddFunction(mod, funcname, - LLVMGetElementType(LLVMTypeOf(v_srcfn))); + LLVMGetFunctionType(v_srcfn)); llvm_copy_attributes(v_srcfn, v_fn); return v_fn; @@ -531,7 +532,7 @@ llvm_function_reference(LLVMJitContext *context, fcinfo->flinfo->fn_oid); v_fn = LLVMGetNamedGlobal(mod, funcname); if (v_fn != 0) - return LLVMBuildLoad(builder, v_fn, ""); + return l_load(builder, TypePGFunction, v_fn, ""); v_fn_addr = l_ptr_const(fcinfo->flinfo->fn_addr, TypePGFunction); @@ -541,7 +542,7 @@ llvm_function_reference(LLVMJitContext *context, LLVMSetLinkage(v_fn, LLVMPrivateLinkage); LLVMSetUnnamedAddr(v_fn, true); - return LLVMBuildLoad(builder, v_fn, ""); + return l_load(builder, TypePGFunction, v_fn, ""); } /* check if function already has been added */ @@ -549,7 +550,7 @@ llvm_function_reference(LLVMJitContext *context, if (v_fn != 0) return v_fn; - v_fn = LLVMAddFunction(mod, funcname, LLVMGetElementType(TypePGFunction)); + v_fn = LLVMAddFunction(mod, funcname, LLVMGetFunctionType(AttributeTemplate)); return v_fn; } @@ -801,12 +802,15 @@ llvm_session_initialize(void) LLVMInitializeNativeAsmParser(); /* - * When targeting an LLVM version with opaque pointers enabled by - * default, turn them off for the context we build our code in. We don't - * need to do so for other contexts (e.g. llvm_ts_context). Once the IR is - * generated, it carries the necessary information. + * When targeting LLVM 15, turn off opaque pointers for the context we + * build our code in. We don't need to do so for other contexts (e.g. + * llvm_ts_context). Once the IR is generated, it carries the necessary + * information. + * + * For 16 and above, opaque pointers must be used, and we have special + * code for that. */ -#if LLVM_VERSION_MAJOR > 14 +#if LLVM_VERSION_MAJOR == 15 LLVMContextSetOpaquePointers(LLVMGetGlobalContext(), false); #endif @@ -968,15 +972,7 @@ load_return_type(LLVMModuleRef mod, const char *name) if (!value) elog(ERROR, "function %s is unknown", name); - /* get type of function pointer */ - typ = LLVMTypeOf(value); - Assert(typ != NULL); - /* dereference pointer */ - typ = LLVMGetElementType(typ); - Assert(typ != NULL); - /* and look at return type */ - typ = LLVMGetReturnType(typ); - Assert(typ != NULL); + typ = LLVMGetFunctionReturnType(value); /* in llvmjit_wrap.cpp */ return typ; } @@ -1031,12 +1027,17 @@ llvm_create_types(void) StructHeapTupleTableSlot = llvm_pg_var_type("StructHeapTupleTableSlot"); StructMinimalTupleTableSlot = llvm_pg_var_type("StructMinimalTupleTableSlot"); StructHeapTupleData = llvm_pg_var_type("StructHeapTupleData"); + StructHeapTupleHeaderData = llvm_pg_var_type("StructHeapTupleHeaderData"); StructTupleDescData = llvm_pg_var_type("StructTupleDescData"); StructAggState = llvm_pg_var_type("StructAggState"); StructAggStatePerGroupData = llvm_pg_var_type("StructAggStatePerGroupData"); StructAggStatePerTransData = llvm_pg_var_type("StructAggStatePerTransData"); + StructPlanState = llvm_pg_var_type("StructPlanState"); + StructMinimalTupleData = llvm_pg_var_type("StructMinimalTupleData"); AttributeTemplate = LLVMGetNamedFunction(llvm_types_module, "AttributeTemplate"); + ExecEvalSubroutineTemplate = LLVMGetNamedFunction(llvm_types_module, "ExecEvalSubroutineTemplate"); + ExecEvalBoolSubroutineTemplate = LLVMGetNamedFunction(llvm_types_module, "ExecEvalBoolSubroutineTemplate"); } /* diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c index 008cd617f6c..60a677ba439 100644 --- a/src/backend/jit/llvm/llvmjit_deform.c +++ b/src/backend/jit/llvm/llvmjit_deform.c @@ -171,13 +171,13 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, v_slot = LLVMGetParam(v_deform_fn, 0); v_tts_values = - l_load_struct_gep(b, v_slot, FIELDNO_TUPLETABLESLOT_VALUES, + l_load_struct_gep(b, StructTupleTableSlot, v_slot, FIELDNO_TUPLETABLESLOT_VALUES, "tts_values"); v_tts_nulls = - l_load_struct_gep(b, v_slot, FIELDNO_TUPLETABLESLOT_ISNULL, + l_load_struct_gep(b, StructTupleTableSlot, v_slot, FIELDNO_TUPLETABLESLOT_ISNULL, "tts_ISNULL"); - v_flagsp = LLVMBuildStructGEP(b, v_slot, FIELDNO_TUPLETABLESLOT_FLAGS, ""); - v_nvalidp = LLVMBuildStructGEP(b, v_slot, FIELDNO_TUPLETABLESLOT_NVALID, ""); + v_flagsp = l_struct_gep(b, StructTupleTableSlot, v_slot, FIELDNO_TUPLETABLESLOT_FLAGS, ""); + v_nvalidp = l_struct_gep(b, StructTupleTableSlot, v_slot, FIELDNO_TUPLETABLESLOT_NVALID, ""); if (ops == &TTSOpsHeapTuple || ops == &TTSOpsBufferHeapTuple) { @@ -188,9 +188,9 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, v_slot, l_ptr(StructHeapTupleTableSlot), "heapslot"); - v_slotoffp = LLVMBuildStructGEP(b, v_heapslot, FIELDNO_HEAPTUPLETABLESLOT_OFF, ""); + v_slotoffp = l_struct_gep(b, StructHeapTupleTableSlot, v_heapslot, FIELDNO_HEAPTUPLETABLESLOT_OFF, ""); v_tupleheaderp = - l_load_struct_gep(b, v_heapslot, FIELDNO_HEAPTUPLETABLESLOT_TUPLE, + l_load_struct_gep(b, StructHeapTupleTableSlot, v_heapslot, FIELDNO_HEAPTUPLETABLESLOT_TUPLE, "tupleheader"); } @@ -203,9 +203,15 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, v_slot, l_ptr(StructMinimalTupleTableSlot), "minimalslot"); - v_slotoffp = LLVMBuildStructGEP(b, v_minimalslot, FIELDNO_MINIMALTUPLETABLESLOT_OFF, ""); + v_slotoffp = l_struct_gep(b, + StructMinimalTupleTableSlot, + v_minimalslot, + FIELDNO_MINIMALTUPLETABLESLOT_OFF, ""); v_tupleheaderp = - l_load_struct_gep(b, v_minimalslot, FIELDNO_MINIMALTUPLETABLESLOT_TUPLE, + l_load_struct_gep(b, + StructMinimalTupleTableSlot, + v_minimalslot, + FIELDNO_MINIMALTUPLETABLESLOT_TUPLE, "tupleheader"); } else @@ -215,21 +221,29 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, } v_tuplep = - l_load_struct_gep(b, v_tupleheaderp, FIELDNO_HEAPTUPLEDATA_DATA, + l_load_struct_gep(b, + StructHeapTupleData, + v_tupleheaderp, + FIELDNO_HEAPTUPLEDATA_DATA, "tuple"); v_bits = LLVMBuildBitCast(b, - LLVMBuildStructGEP(b, v_tuplep, - FIELDNO_HEAPTUPLEHEADERDATA_BITS, - ""), + l_struct_gep(b, + StructHeapTupleHeaderData, + v_tuplep, + FIELDNO_HEAPTUPLEHEADERDATA_BITS, + ""), l_ptr(LLVMInt8Type()), "t_bits"); v_infomask1 = - l_load_struct_gep(b, v_tuplep, + l_load_struct_gep(b, + StructHeapTupleHeaderData, + v_tuplep, FIELDNO_HEAPTUPLEHEADERDATA_INFOMASK, "infomask1"); v_infomask2 = l_load_struct_gep(b, + StructHeapTupleHeaderData, v_tuplep, FIELDNO_HEAPTUPLEHEADERDATA_INFOMASK2, "infomask2"); @@ -254,19 +268,21 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, */ v_hoff = LLVMBuildZExt(b, - l_load_struct_gep(b, v_tuplep, + l_load_struct_gep(b, + StructHeapTupleHeaderData, + v_tuplep, FIELDNO_HEAPTUPLEHEADERDATA_HOFF, ""), LLVMInt32Type(), "t_hoff"); - v_tupdata_base = - LLVMBuildGEP(b, - LLVMBuildBitCast(b, - v_tuplep, - l_ptr(LLVMInt8Type()), - ""), - &v_hoff, 1, - "v_tupdata_base"); + v_tupdata_base = l_gep(b, + LLVMInt8Type(), + LLVMBuildBitCast(b, + v_tuplep, + l_ptr(LLVMInt8Type()), + ""), + &v_hoff, 1, + "v_tupdata_base"); /* * Load tuple start offset from slot. Will be reset below in case there's @@ -275,7 +291,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, { LLVMValueRef v_off_start; - v_off_start = LLVMBuildLoad(b, v_slotoffp, "v_slot_off"); + v_off_start = l_load(b, LLVMInt32Type(), v_slotoffp, "v_slot_off"); v_off_start = LLVMBuildZExt(b, v_off_start, TypeSizeT, ""); LLVMBuildStore(b, v_off_start, v_offp); } @@ -315,6 +331,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, else { LLVMValueRef v_params[3]; + LLVMValueRef f; /* branch if not all columns available */ LLVMBuildCondBr(b, @@ -331,14 +348,16 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, v_params[0] = v_slot; v_params[1] = LLVMBuildZExt(b, v_maxatt, LLVMInt32Type(), ""); v_params[2] = l_int32_const(natts); - LLVMBuildCall(b, llvm_pg_func(mod, "slot_getmissingattrs"), - v_params, lengthof(v_params), ""); + f = llvm_pg_func(mod, "slot_getmissingattrs"); + l_call(b, + LLVMGetFunctionType(f), f, + v_params, lengthof(v_params), ""); LLVMBuildBr(b, b_find_start); } LLVMPositionBuilderAtEnd(b, b_find_start); - v_nvalid = LLVMBuildLoad(b, v_nvalidp, ""); + v_nvalid = l_load(b, LLVMInt16Type(), v_nvalidp, ""); /* * Build switch to go from nvalid to the right startblock. Callers @@ -440,7 +459,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, v_nullbyteno = l_int32_const(attnum >> 3); v_nullbytemask = l_int8_const(1 << ((attnum) & 0x07)); - v_nullbyte = l_load_gep1(b, v_bits, v_nullbyteno, "attnullbyte"); + v_nullbyte = l_load_gep1(b, LLVMInt8Type(), v_bits, v_nullbyteno, "attnullbyte"); v_nullbit = LLVMBuildICmp(b, LLVMIntEQ, @@ -457,11 +476,11 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, /* store null-byte */ LLVMBuildStore(b, l_int8_const(1), - LLVMBuildGEP(b, v_tts_nulls, &l_attno, 1, "")); + l_gep(b, LLVMInt8Type(), v_tts_nulls, &l_attno, 1, "")); /* store zero datum */ LLVMBuildStore(b, l_sizet_const(0), - LLVMBuildGEP(b, v_tts_values, &l_attno, 1, "")); + l_gep(b, TypeSizeT, v_tts_values, &l_attno, 1, "")); LLVMBuildBr(b, b_next); attguaranteedalign = false; @@ -520,10 +539,10 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, /* don't know if short varlena or not */ attguaranteedalign = false; - v_off = LLVMBuildLoad(b, v_offp, ""); + v_off = l_load(b, TypeSizeT, v_offp, ""); v_possible_padbyte = - l_load_gep1(b, v_tupdata_base, v_off, "padbyte"); + l_load_gep1(b, LLVMInt8Type(), v_tupdata_base, v_off, "padbyte"); v_ispad = LLVMBuildICmp(b, LLVMIntEQ, v_possible_padbyte, l_int8_const(0), @@ -542,7 +561,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, /* translation of alignment code (cf TYPEALIGN()) */ { LLVMValueRef v_off_aligned; - LLVMValueRef v_off = LLVMBuildLoad(b, v_offp, ""); + LLVMValueRef v_off = l_load(b, TypeSizeT, v_offp, ""); /* ((ALIGNVAL) - 1) */ LLVMValueRef v_alignval = l_sizet_const(alignto - 1); @@ -631,18 +650,18 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, /* compute address to load data from */ { - LLVMValueRef v_off = LLVMBuildLoad(b, v_offp, ""); + LLVMValueRef v_off = l_load(b, TypeSizeT, v_offp, ""); v_attdatap = - LLVMBuildGEP(b, v_tupdata_base, &v_off, 1, ""); + l_gep(b, LLVMInt8Type(), v_tupdata_base, &v_off, 1, ""); } /* compute address to store value at */ - v_resultp = LLVMBuildGEP(b, v_tts_values, &l_attno, 1, ""); + v_resultp = l_gep(b, TypeSizeT, v_tts_values, &l_attno, 1, ""); /* store null-byte (false) */ LLVMBuildStore(b, l_int8_const(0), - LLVMBuildGEP(b, v_tts_nulls, &l_attno, 1, "")); + l_gep(b, TypeStorageBool, v_tts_nulls, &l_attno, 1, "")); /* * Store datum. For byval: datums copy the value, extend to Datum's @@ -651,12 +670,12 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, if (att->attbyval) { LLVMValueRef v_tmp_loaddata; - LLVMTypeRef vartypep = - LLVMPointerType(LLVMIntType(att->attlen * 8), 0); + LLVMTypeRef vartype = LLVMIntType(att->attlen * 8); + LLVMTypeRef vartypep = LLVMPointerType(vartype, 0); v_tmp_loaddata = LLVMBuildPointerCast(b, v_attdatap, vartypep, ""); - v_tmp_loaddata = LLVMBuildLoad(b, v_tmp_loaddata, "attr_byval"); + v_tmp_loaddata = l_load(b, vartype, v_tmp_loaddata, "attr_byval"); v_tmp_loaddata = LLVMBuildZExt(b, v_tmp_loaddata, TypeSizeT, ""); LLVMBuildStore(b, v_tmp_loaddata, v_resultp); @@ -681,18 +700,20 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, } else if (att->attlen == -1) { - v_incby = LLVMBuildCall(b, - llvm_pg_func(mod, "varsize_any"), - &v_attdatap, 1, - "varsize_any"); + v_incby = l_call(b, + llvm_pg_var_func_type("varsize_any"), + llvm_pg_func(mod, "varsize_any"), + &v_attdatap, 1, + "varsize_any"); l_callsite_ro(v_incby); l_callsite_alwaysinline(v_incby); } else if (att->attlen == -2) { - v_incby = LLVMBuildCall(b, - llvm_pg_func(mod, "strlen"), - &v_attdatap, 1, "strlen"); + v_incby = l_call(b, + llvm_pg_var_func_type("strlen"), + llvm_pg_func(mod, "strlen"), + &v_attdatap, 1, "strlen"); l_callsite_ro(v_incby); @@ -712,7 +733,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, } else { - LLVMValueRef v_off = LLVMBuildLoad(b, v_offp, ""); + LLVMValueRef v_off = l_load(b, TypeSizeT, v_offp, ""); v_off = LLVMBuildAdd(b, v_off, v_incby, "increment_offset"); LLVMBuildStore(b, v_off, v_offp); @@ -738,13 +759,13 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, LLVMPositionBuilderAtEnd(b, b_out); { - LLVMValueRef v_off = LLVMBuildLoad(b, v_offp, ""); + LLVMValueRef v_off = l_load(b, TypeSizeT, v_offp, ""); LLVMValueRef v_flags; LLVMBuildStore(b, l_int16_const(natts), v_nvalidp); v_off = LLVMBuildTrunc(b, v_off, LLVMInt32Type(), ""); LLVMBuildStore(b, v_off, v_slotoffp); - v_flags = LLVMBuildLoad(b, v_flagsp, "tts_flags"); + v_flags = l_load(b, LLVMInt16Type(), v_flagsp, "tts_flags"); v_flags = LLVMBuildOr(b, v_flags, l_int16_const(TTS_FLAG_SLOW), ""); LLVMBuildStore(b, v_flags, v_flagsp); LLVMBuildRetVoid(b); diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 84c5f5980ab..b1c29039468 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -150,7 +150,7 @@ llvm_compile_expr(ExprState *state) /* create function */ eval_fn = LLVMAddFunction(mod, funcname, - llvm_pg_var_func_type("TypeExprStateEvalFunc")); + llvm_pg_var_func_type("ExecInterpExprStillValid")); LLVMSetLinkage(eval_fn, LLVMExternalLinkage); LLVMSetVisibility(eval_fn, LLVMDefaultVisibility); llvm_copy_attributes(AttributeTemplate, eval_fn); @@ -164,61 +164,95 @@ llvm_compile_expr(ExprState *state) LLVMPositionBuilderAtEnd(b, entry); - v_tmpvaluep = LLVMBuildStructGEP(b, v_state, - FIELDNO_EXPRSTATE_RESVALUE, - "v.state.resvalue"); - v_tmpisnullp = LLVMBuildStructGEP(b, v_state, - FIELDNO_EXPRSTATE_RESNULL, - "v.state.resnull"); - v_parent = l_load_struct_gep(b, v_state, + v_tmpvaluep = l_struct_gep(b, + StructExprState, + v_state, + FIELDNO_EXPRSTATE_RESVALUE, + "v.state.resvalue"); + v_tmpisnullp = l_struct_gep(b, + StructExprState, + v_state, + FIELDNO_EXPRSTATE_RESNULL, + "v.state.resnull"); + v_parent = l_load_struct_gep(b, + StructExprState, + v_state, FIELDNO_EXPRSTATE_PARENT, "v.state.parent"); /* build global slots */ - v_scanslot = l_load_struct_gep(b, v_econtext, + v_scanslot = l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_SCANTUPLE, "v_scanslot"); - v_innerslot = l_load_struct_gep(b, v_econtext, + v_innerslot = l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_INNERTUPLE, "v_innerslot"); - v_outerslot = l_load_struct_gep(b, v_econtext, + v_outerslot = l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_OUTERTUPLE, "v_outerslot"); - v_resultslot = l_load_struct_gep(b, v_state, + v_resultslot = l_load_struct_gep(b, + StructExprState, + v_state, FIELDNO_EXPRSTATE_RESULTSLOT, "v_resultslot"); /* build global values/isnull pointers */ - v_scanvalues = l_load_struct_gep(b, v_scanslot, + v_scanvalues = l_load_struct_gep(b, + StructTupleTableSlot, + v_scanslot, FIELDNO_TUPLETABLESLOT_VALUES, "v_scanvalues"); - v_scannulls = l_load_struct_gep(b, v_scanslot, + v_scannulls = l_load_struct_gep(b, + StructTupleTableSlot, + v_scanslot, FIELDNO_TUPLETABLESLOT_ISNULL, "v_scannulls"); - v_innervalues = l_load_struct_gep(b, v_innerslot, + v_innervalues = l_load_struct_gep(b, + StructTupleTableSlot, + v_innerslot, FIELDNO_TUPLETABLESLOT_VALUES, "v_innervalues"); - v_innernulls = l_load_struct_gep(b, v_innerslot, + v_innernulls = l_load_struct_gep(b, + StructTupleTableSlot, + v_innerslot, FIELDNO_TUPLETABLESLOT_ISNULL, "v_innernulls"); - v_outervalues = l_load_struct_gep(b, v_outerslot, + v_outervalues = l_load_struct_gep(b, + StructTupleTableSlot, + v_outerslot, FIELDNO_TUPLETABLESLOT_VALUES, "v_outervalues"); - v_outernulls = l_load_struct_gep(b, v_outerslot, + v_outernulls = l_load_struct_gep(b, + StructTupleTableSlot, + v_outerslot, FIELDNO_TUPLETABLESLOT_ISNULL, "v_outernulls"); - v_resultvalues = l_load_struct_gep(b, v_resultslot, + v_resultvalues = l_load_struct_gep(b, + StructTupleTableSlot, + v_resultslot, FIELDNO_TUPLETABLESLOT_VALUES, "v_resultvalues"); - v_resultnulls = l_load_struct_gep(b, v_resultslot, + v_resultnulls = l_load_struct_gep(b, + StructTupleTableSlot, + v_resultslot, FIELDNO_TUPLETABLESLOT_ISNULL, "v_resultnulls"); /* aggvalues/aggnulls */ - v_aggvalues = l_load_struct_gep(b, v_econtext, + v_aggvalues = l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_AGGVALUES, "v.econtext.aggvalues"); - v_aggnulls = l_load_struct_gep(b, v_econtext, + v_aggnulls = l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_AGGNULLS, "v.econtext.aggnulls"); @@ -252,8 +286,8 @@ llvm_compile_expr(ExprState *state) LLVMValueRef v_tmpisnull; LLVMValueRef v_tmpvalue; - v_tmpvalue = LLVMBuildLoad(b, v_tmpvaluep, ""); - v_tmpisnull = LLVMBuildLoad(b, v_tmpisnullp, ""); + v_tmpvalue = l_load(b, TypeSizeT, v_tmpvaluep, ""); + v_tmpisnull = l_load(b, TypeStorageBool, v_tmpisnullp, ""); LLVMBuildStore(b, v_tmpisnull, v_isnullp); @@ -296,7 +330,9 @@ llvm_compile_expr(ExprState *state) * whether deforming is required. */ v_nvalid = - l_load_struct_gep(b, v_slot, + l_load_struct_gep(b, + StructTupleTableSlot, + v_slot, FIELDNO_TUPLETABLESLOT_NVALID, ""); LLVMBuildCondBr(b, @@ -327,8 +363,10 @@ llvm_compile_expr(ExprState *state) params[0] = v_slot; - LLVMBuildCall(b, l_jit_deform, - params, lengthof(params), ""); + l_call(b, + LLVMGetFunctionType(l_jit_deform), + l_jit_deform, + params, lengthof(params), ""); } else { @@ -337,9 +375,10 @@ llvm_compile_expr(ExprState *state) params[0] = v_slot; params[1] = l_int32_const(op->d.fetch.last_var); - LLVMBuildCall(b, - llvm_pg_func(mod, "slot_getsomeattrs_int"), - params, lengthof(params), ""); + l_call(b, + llvm_pg_var_func_type("slot_getsomeattrs_int"), + llvm_pg_func(mod, "slot_getsomeattrs_int"), + params, lengthof(params), ""); } LLVMBuildBr(b, opblocks[opno + 1]); @@ -373,8 +412,8 @@ llvm_compile_expr(ExprState *state) } v_attnum = l_int32_const(op->d.var.attnum); - value = l_load_gep1(b, v_values, v_attnum, ""); - isnull = l_load_gep1(b, v_nulls, v_attnum, ""); + value = l_load_gep1(b, TypeSizeT, v_values, v_attnum, ""); + isnull = l_load_gep1(b, TypeStorageBool, v_nulls, v_attnum, ""); LLVMBuildStore(b, value, v_resvaluep); LLVMBuildStore(b, isnull, v_resnullp); @@ -439,15 +478,19 @@ llvm_compile_expr(ExprState *state) /* load data */ v_attnum = l_int32_const(op->d.assign_var.attnum); - v_value = l_load_gep1(b, v_values, v_attnum, ""); - v_isnull = l_load_gep1(b, v_nulls, v_attnum, ""); + v_value = l_load_gep1(b, TypeSizeT, v_values, v_attnum, ""); + v_isnull = l_load_gep1(b, TypeStorageBool, v_nulls, v_attnum, ""); /* compute addresses of targets */ v_resultnum = l_int32_const(op->d.assign_var.resultnum); - v_rvaluep = LLVMBuildGEP(b, v_resultvalues, - &v_resultnum, 1, ""); - v_risnullp = LLVMBuildGEP(b, v_resultnulls, - &v_resultnum, 1, ""); + v_rvaluep = l_gep(b, + TypeSizeT, + v_resultvalues, + &v_resultnum, 1, ""); + v_risnullp = l_gep(b, + TypeStorageBool, + v_resultnulls, + &v_resultnum, 1, ""); /* and store */ LLVMBuildStore(b, v_value, v_rvaluep); @@ -468,15 +511,15 @@ llvm_compile_expr(ExprState *state) size_t resultnum = op->d.assign_tmp.resultnum; /* load data */ - v_value = LLVMBuildLoad(b, v_tmpvaluep, ""); - v_isnull = LLVMBuildLoad(b, v_tmpisnullp, ""); + v_value = l_load(b, TypeSizeT, v_tmpvaluep, ""); + v_isnull = l_load(b, TypeStorageBool, v_tmpisnullp, ""); /* compute addresses of targets */ v_resultnum = l_int32_const(resultnum); v_rvaluep = - LLVMBuildGEP(b, v_resultvalues, &v_resultnum, 1, ""); + l_gep(b, TypeSizeT, v_resultvalues, &v_resultnum, 1, ""); v_risnullp = - LLVMBuildGEP(b, v_resultnulls, &v_resultnum, 1, ""); + l_gep(b, TypeStorageBool, v_resultnulls, &v_resultnum, 1, ""); /* store nullness */ LLVMBuildStore(b, v_isnull, v_risnullp); @@ -500,9 +543,10 @@ llvm_compile_expr(ExprState *state) LLVMPositionBuilderAtEnd(b, b_notnull); v_params[0] = v_value; v_value = - LLVMBuildCall(b, - llvm_pg_func(mod, "MakeExpandedObjectReadOnlyInternal"), - v_params, lengthof(v_params), ""); + l_call(b, + llvm_pg_var_func_type("MakeExpandedObjectReadOnlyInternal"), + llvm_pg_func(mod, "MakeExpandedObjectReadOnlyInternal"), + v_params, lengthof(v_params), ""); /* * Falling out of the if () with builder in b_notnull, @@ -665,8 +709,8 @@ llvm_compile_expr(ExprState *state) if (opcode == EEOP_BOOL_AND_STEP_FIRST) LLVMBuildStore(b, l_sbool_const(0), v_boolanynullp); - v_boolnull = LLVMBuildLoad(b, v_resnullp, ""); - v_boolvalue = LLVMBuildLoad(b, v_resvaluep, ""); + v_boolnull = l_load(b, TypeStorageBool, v_resnullp, ""); + v_boolvalue = l_load(b, TypeSizeT, v_resvaluep, ""); /* set resnull to boolnull */ LLVMBuildStore(b, v_boolnull, v_resnullp); @@ -707,7 +751,7 @@ llvm_compile_expr(ExprState *state) /* Build block that continues if bool is TRUE. */ LLVMPositionBuilderAtEnd(b, b_boolcont); - v_boolanynull = LLVMBuildLoad(b, v_boolanynullp, ""); + v_boolanynull = l_load(b, TypeStorageBool, v_boolanynullp, ""); /* set value to NULL if any previous values were NULL */ LLVMBuildCondBr(b, @@ -761,8 +805,8 @@ llvm_compile_expr(ExprState *state) if (opcode == EEOP_BOOL_OR_STEP_FIRST) LLVMBuildStore(b, l_sbool_const(0), v_boolanynullp); - v_boolnull = LLVMBuildLoad(b, v_resnullp, ""); - v_boolvalue = LLVMBuildLoad(b, v_resvaluep, ""); + v_boolnull = l_load(b, TypeStorageBool, v_resnullp, ""); + v_boolvalue = l_load(b, TypeSizeT, v_resvaluep, ""); /* set resnull to boolnull */ LLVMBuildStore(b, v_boolnull, v_resnullp); @@ -802,7 +846,7 @@ llvm_compile_expr(ExprState *state) /* build block that continues if bool is FALSE */ LLVMPositionBuilderAtEnd(b, b_boolcont); - v_boolanynull = LLVMBuildLoad(b, v_boolanynullp, ""); + v_boolanynull = l_load(b, TypeStorageBool, v_boolanynullp, ""); /* set value to NULL if any previous values were NULL */ LLVMBuildCondBr(b, @@ -826,8 +870,8 @@ llvm_compile_expr(ExprState *state) LLVMValueRef v_boolnull; LLVMValueRef v_negbool; - v_boolnull = LLVMBuildLoad(b, v_resnullp, ""); - v_boolvalue = LLVMBuildLoad(b, v_resvaluep, ""); + v_boolnull = l_load(b, TypeStorageBool, v_resnullp, ""); + v_boolvalue = l_load(b, TypeSizeT, v_resvaluep, ""); v_negbool = LLVMBuildZExt(b, LLVMBuildICmp(b, LLVMIntEQ, @@ -854,8 +898,8 @@ llvm_compile_expr(ExprState *state) b_qualfail = l_bb_before_v(opblocks[opno + 1], "op.%d.qualfail", opno); - v_resvalue = LLVMBuildLoad(b, v_resvaluep, ""); - v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + v_resvalue = l_load(b, TypeSizeT, v_resvaluep, ""); + v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); v_nullorfalse = LLVMBuildOr(b, @@ -893,7 +937,7 @@ llvm_compile_expr(ExprState *state) /* Transfer control if current result is null */ - v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); LLVMBuildCondBr(b, LLVMBuildICmp(b, LLVMIntEQ, v_resnull, @@ -909,7 +953,7 @@ llvm_compile_expr(ExprState *state) /* Transfer control if current result is non-null */ - v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); LLVMBuildCondBr(b, LLVMBuildICmp(b, LLVMIntEQ, v_resnull, @@ -928,8 +972,8 @@ llvm_compile_expr(ExprState *state) /* Transfer control if current result is null or false */ - v_resvalue = LLVMBuildLoad(b, v_resvaluep, ""); - v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + v_resvalue = l_load(b, TypeSizeT, v_resvaluep, ""); + v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); v_nullorfalse = LLVMBuildOr(b, @@ -948,7 +992,7 @@ llvm_compile_expr(ExprState *state) case EEOP_NULLTEST_ISNULL: { - LLVMValueRef v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + LLVMValueRef v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); LLVMValueRef v_resvalue; v_resvalue = @@ -967,7 +1011,7 @@ llvm_compile_expr(ExprState *state) case EEOP_NULLTEST_ISNOTNULL: { - LLVMValueRef v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + LLVMValueRef v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); LLVMValueRef v_resvalue; v_resvalue = @@ -1003,7 +1047,7 @@ llvm_compile_expr(ExprState *state) { LLVMBasicBlockRef b_isnull, b_notnull; - LLVMValueRef v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + LLVMValueRef v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); b_isnull = l_bb_before_v(opblocks[opno + 1], "op.%d.isnull", opno); @@ -1047,7 +1091,7 @@ llvm_compile_expr(ExprState *state) else { LLVMValueRef v_value = - LLVMBuildLoad(b, v_resvaluep, ""); + l_load(b, TypeSizeT, v_resvaluep, ""); v_value = LLVMBuildZExt(b, LLVMBuildICmp(b, LLVMIntEQ, @@ -1075,20 +1119,19 @@ llvm_compile_expr(ExprState *state) case EEOP_PARAM_CALLBACK: { - LLVMTypeRef v_functype; LLVMValueRef v_func; LLVMValueRef v_params[3]; - v_functype = llvm_pg_var_func_type("TypeExecEvalSubroutine"); v_func = l_ptr_const(op->d.cparam.paramfunc, - LLVMPointerType(v_functype, 0)); + llvm_pg_var_type("TypeExecEvalSubroutine")); v_params[0] = v_state; v_params[1] = l_ptr_const(op, l_ptr(StructExprEvalStep)); v_params[2] = v_econtext; - LLVMBuildCall(b, - v_func, - v_params, lengthof(v_params), ""); + l_call(b, + LLVMGetFunctionType(ExecEvalSubroutineTemplate), + v_func, + v_params, lengthof(v_params), ""); LLVMBuildBr(b, opblocks[opno + 1]); break; @@ -1097,21 +1140,20 @@ llvm_compile_expr(ExprState *state) case EEOP_SBSREF_SUBSCRIPTS: { int jumpdone = op->d.sbsref_subscript.jumpdone; - LLVMTypeRef v_functype; LLVMValueRef v_func; LLVMValueRef v_params[3]; LLVMValueRef v_ret; - v_functype = llvm_pg_var_func_type("TypeExecEvalBoolSubroutine"); v_func = l_ptr_const(op->d.sbsref_subscript.subscriptfunc, - LLVMPointerType(v_functype, 0)); + llvm_pg_var_type("TypeExecEvalBoolSubroutine")); v_params[0] = v_state; v_params[1] = l_ptr_const(op, l_ptr(StructExprEvalStep)); v_params[2] = v_econtext; - v_ret = LLVMBuildCall(b, - v_func, - v_params, lengthof(v_params), ""); + v_ret = l_call(b, + LLVMGetFunctionType(ExecEvalBoolSubroutineTemplate), + v_func, + v_params, lengthof(v_params), ""); v_ret = LLVMBuildZExt(b, v_ret, TypeStorageBool, ""); LLVMBuildCondBr(b, @@ -1126,20 +1168,19 @@ llvm_compile_expr(ExprState *state) case EEOP_SBSREF_ASSIGN: case EEOP_SBSREF_FETCH: { - LLVMTypeRef v_functype; LLVMValueRef v_func; LLVMValueRef v_params[3]; - v_functype = llvm_pg_var_func_type("TypeExecEvalSubroutine"); v_func = l_ptr_const(op->d.sbsref.subscriptfunc, - LLVMPointerType(v_functype, 0)); + llvm_pg_var_type("TypeExecEvalSubroutine")); v_params[0] = v_state; v_params[1] = l_ptr_const(op, l_ptr(StructExprEvalStep)); v_params[2] = v_econtext; - LLVMBuildCall(b, - v_func, - v_params, lengthof(v_params), ""); + l_call(b, + LLVMGetFunctionType(ExecEvalSubroutineTemplate), + v_func, + v_params, lengthof(v_params), ""); LLVMBuildBr(b, opblocks[opno + 1]); break; @@ -1174,8 +1215,8 @@ llvm_compile_expr(ExprState *state) /* if casetest != NULL */ LLVMPositionBuilderAtEnd(b, b_avail); - v_casevalue = LLVMBuildLoad(b, v_casevaluep, ""); - v_casenull = LLVMBuildLoad(b, v_casenullp, ""); + v_casevalue = l_load(b, TypeSizeT, v_casevaluep, ""); + v_casenull = l_load(b, TypeStorageBool, v_casenullp, ""); LLVMBuildStore(b, v_casevalue, v_resvaluep); LLVMBuildStore(b, v_casenull, v_resnullp); LLVMBuildBr(b, opblocks[opno + 1]); @@ -1183,10 +1224,14 @@ llvm_compile_expr(ExprState *state) /* if casetest == NULL */ LLVMPositionBuilderAtEnd(b, b_notavail); v_casevalue = - l_load_struct_gep(b, v_econtext, + l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_CASEDATUM, ""); v_casenull = - l_load_struct_gep(b, v_econtext, + l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_CASENULL, ""); LLVMBuildStore(b, v_casevalue, v_resvaluep); LLVMBuildStore(b, v_casenull, v_resnullp); @@ -1211,7 +1256,7 @@ llvm_compile_expr(ExprState *state) v_nullp = l_ptr_const(op->d.make_readonly.isnull, l_ptr(TypeStorageBool)); - v_null = LLVMBuildLoad(b, v_nullp, ""); + v_null = l_load(b, TypeStorageBool, v_nullp, ""); /* store null isnull value in result */ LLVMBuildStore(b, v_null, v_resnullp); @@ -1228,13 +1273,14 @@ llvm_compile_expr(ExprState *state) v_valuep = l_ptr_const(op->d.make_readonly.value, l_ptr(TypeSizeT)); - v_value = LLVMBuildLoad(b, v_valuep, ""); + v_value = l_load(b, TypeSizeT, v_valuep, ""); v_params[0] = v_value; v_ret = - LLVMBuildCall(b, - llvm_pg_func(mod, "MakeExpandedObjectReadOnlyInternal"), - v_params, lengthof(v_params), ""); + l_call(b, + llvm_pg_var_func_type("MakeExpandedObjectReadOnlyInternal"), + llvm_pg_func(mod, "MakeExpandedObjectReadOnlyInternal"), + v_params, lengthof(v_params), ""); LLVMBuildStore(b, v_ret, v_resvaluep); LLVMBuildBr(b, opblocks[opno + 1]); @@ -1280,12 +1326,14 @@ llvm_compile_expr(ExprState *state) v_fcinfo_in = l_ptr_const(fcinfo_in, l_ptr(StructFunctionCallInfoData)); v_fcinfo_in_isnullp = - LLVMBuildStructGEP(b, v_fcinfo_in, - FIELDNO_FUNCTIONCALLINFODATA_ISNULL, - "v_fcinfo_in_isnull"); + l_struct_gep(b, + StructFunctionCallInfoData, + v_fcinfo_in, + FIELDNO_FUNCTIONCALLINFODATA_ISNULL, + "v_fcinfo_in_isnull"); /* output functions are not called on nulls */ - v_resnull = LLVMBuildLoad(b, v_resnullp, ""); + v_resnull = l_load(b, TypeStorageBool, v_resnullp, ""); LLVMBuildCondBr(b, LLVMBuildICmp(b, LLVMIntEQ, v_resnull, l_sbool_const(1), ""), @@ -1297,7 +1345,7 @@ llvm_compile_expr(ExprState *state) LLVMBuildBr(b, b_input); LLVMPositionBuilderAtEnd(b, b_calloutput); - v_resvalue = LLVMBuildLoad(b, v_resvaluep, ""); + v_resvalue = l_load(b, TypeSizeT, v_resvaluep, ""); /* set arg[0] */ LLVMBuildStore(b, @@ -1307,8 +1355,10 @@ llvm_compile_expr(ExprState *state) l_sbool_const(0), l_funcnullp(b, v_fcinfo_out, 0)); /* and call output function (can never return NULL) */ - v_output = LLVMBuildCall(b, v_fn_out, &v_fcinfo_out, - 1, "funccall_coerce_out"); + v_output = l_call(b, + LLVMGetFunctionType(v_fn_out), + v_fn_out, &v_fcinfo_out, + 1, "funccall_coerce_out"); LLVMBuildBr(b, b_input); /* build block handling input function call */ @@ -1362,8 +1412,10 @@ llvm_compile_expr(ExprState *state) /* reset fcinfo_in->isnull */ LLVMBuildStore(b, l_sbool_const(0), v_fcinfo_in_isnullp); /* and call function */ - v_retval = LLVMBuildCall(b, v_fn_in, &v_fcinfo_in, 1, - "funccall_iocoerce_in"); + v_retval = l_call(b, + LLVMGetFunctionType(v_fn_in), + v_fn_in, &v_fcinfo_in, 1, + "funccall_iocoerce_in"); LLVMBuildStore(b, v_retval, v_resvaluep); @@ -1696,7 +1748,7 @@ llvm_compile_expr(ExprState *state) */ v_cmpresult = LLVMBuildTrunc(b, - LLVMBuildLoad(b, v_resvaluep, ""), + l_load(b, TypeSizeT, v_resvaluep, ""), LLVMInt32Type(), ""); switch (rctype) @@ -1789,8 +1841,8 @@ llvm_compile_expr(ExprState *state) /* if casetest != NULL */ LLVMPositionBuilderAtEnd(b, b_avail); - v_casevalue = LLVMBuildLoad(b, v_casevaluep, ""); - v_casenull = LLVMBuildLoad(b, v_casenullp, ""); + v_casevalue = l_load(b, TypeSizeT, v_casevaluep, ""); + v_casenull = l_load(b, TypeStorageBool, v_casenullp, ""); LLVMBuildStore(b, v_casevalue, v_resvaluep); LLVMBuildStore(b, v_casenull, v_resnullp); LLVMBuildBr(b, opblocks[opno + 1]); @@ -1798,11 +1850,15 @@ llvm_compile_expr(ExprState *state) /* if casetest == NULL */ LLVMPositionBuilderAtEnd(b, b_notavail); v_casevalue = - l_load_struct_gep(b, v_econtext, + l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_DOMAINDATUM, ""); v_casenull = - l_load_struct_gep(b, v_econtext, + l_load_struct_gep(b, + StructExprContext, + v_econtext, FIELDNO_EXPRCONTEXT_DOMAINNULL, ""); LLVMBuildStore(b, v_casevalue, v_resvaluep); @@ -1869,8 +1925,8 @@ llvm_compile_expr(ExprState *state) v_aggno = l_int32_const(op->d.aggref.aggno); /* load agg value / null */ - value = l_load_gep1(b, v_aggvalues, v_aggno, "aggvalue"); - isnull = l_load_gep1(b, v_aggnulls, v_aggno, "aggnull"); + value = l_load_gep1(b, TypeSizeT, v_aggvalues, v_aggno, "aggvalue"); + isnull = l_load_gep1(b, TypeStorageBool, v_aggnulls, v_aggno, "aggnull"); /* and store result */ LLVMBuildStore(b, value, v_resvaluep); @@ -1982,12 +2038,12 @@ llvm_compile_expr(ExprState *state) */ v_wfuncnop = l_ptr_const(&wfunc->wfuncno, l_ptr(LLVMInt32Type())); - v_wfuncno = LLVMBuildLoad(b, v_wfuncnop, "v_wfuncno"); + v_wfuncno = l_load(b, LLVMInt32Type(), v_wfuncnop, "v_wfuncno"); /* load window func value / null */ - value = l_load_gep1(b, v_aggvalues, v_wfuncno, + value = l_load_gep1(b, TypeSizeT, v_aggvalues, v_wfuncno, "windowvalue"); - isnull = l_load_gep1(b, v_aggnulls, v_wfuncno, + isnull = l_load_gep1(b, TypeStorageBool, v_aggnulls, v_wfuncno, "windownull"); LLVMBuildStore(b, value, v_resvaluep); @@ -2101,14 +2157,14 @@ llvm_compile_expr(ExprState *state) b_argnotnull = b_checknulls[argno + 1]; if (opcode == EEOP_AGG_STRICT_INPUT_CHECK_NULLS) - v_argisnull = l_load_gep1(b, v_nullsp, v_argno, ""); + v_argisnull = l_load_gep1(b, TypeStorageBool, v_nullsp, v_argno, ""); else { LLVMValueRef v_argn; - v_argn = LLVMBuildGEP(b, v_argsp, &v_argno, 1, ""); + v_argn = l_gep(b, StructNullableDatum, v_argsp, &v_argno, 1, ""); v_argisnull = - l_load_struct_gep(b, v_argn, + l_load_struct_gep(b, StructNullableDatum, v_argn, FIELDNO_NULLABLE_DATUM_ISNULL, ""); } @@ -2142,13 +2198,16 @@ llvm_compile_expr(ExprState *state) v_aggstatep = LLVMBuildBitCast(b, v_parent, l_ptr(StructAggState), ""); - v_allpergroupsp = l_load_struct_gep(b, v_aggstatep, + v_allpergroupsp = l_load_struct_gep(b, + StructAggState, + v_aggstatep, FIELDNO_AGGSTATE_ALL_PERGROUPS, "aggstate.all_pergroups"); v_setoff = l_int32_const(op->d.agg_plain_pergroup_nullcheck.setoff); - v_pergroup_allaggs = l_load_gep1(b, v_allpergroupsp, v_setoff, ""); + v_pergroup_allaggs = l_load_gep1(b, l_ptr(StructAggStatePerGroupData), + v_allpergroupsp, v_setoff, ""); LLVMBuildCondBr(b, LLVMBuildICmp(b, LLVMIntEQ, @@ -2212,15 +2271,19 @@ llvm_compile_expr(ExprState *state) * [op->d.agg_init_trans_check.transno]; */ v_allpergroupsp = - l_load_struct_gep(b, v_aggstatep, + l_load_struct_gep(b, + StructAggState, + v_aggstatep, FIELDNO_AGGSTATE_ALL_PERGROUPS, "aggstate.all_pergroups"); v_setoff = l_int32_const(op->d.agg_trans.setoff); v_transno = l_int32_const(op->d.agg_trans.transno); v_pergroupp = - LLVMBuildGEP(b, - l_load_gep1(b, v_allpergroupsp, v_setoff, ""), - &v_transno, 1, ""); + l_gep(b, + StructAggStatePerGroupData, + l_load_gep1(b, l_ptr(StructAggStatePerGroupData), + v_allpergroupsp, v_setoff, ""), + &v_transno, 1, ""); if (opcode == EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL || @@ -2231,7 +2294,9 @@ llvm_compile_expr(ExprState *state) LLVMBasicBlockRef b_no_init; v_notransvalue = - l_load_struct_gep(b, v_pergroupp, + l_load_struct_gep(b, + StructAggStatePerGroupData, + v_pergroupp, FIELDNO_AGGSTATEPERGROUPDATA_NOTRANSVALUE, "notransvalue"); @@ -2260,10 +2325,11 @@ llvm_compile_expr(ExprState *state) params[2] = v_pergroupp; params[3] = v_aggcontext; - LLVMBuildCall(b, - llvm_pg_func(mod, "ExecAggInitGroup"), - params, lengthof(params), - ""); + l_call(b, + llvm_pg_var_func_type("ExecAggInitGroup"), + llvm_pg_func(mod, "ExecAggInitGroup"), + params, lengthof(params), + ""); LLVMBuildBr(b, opblocks[opno + 1]); @@ -2283,7 +2349,9 @@ llvm_compile_expr(ExprState *state) b_strictpass = l_bb_before_v(opblocks[opno + 1], "op.%d.strictpass", opno); v_transnull = - l_load_struct_gep(b, v_pergroupp, + l_load_struct_gep(b, + StructAggStatePerGroupData, + v_pergroupp, FIELDNO_AGGSTATEPERGROUPDATA_TRANSVALUEISNULL, "transnull"); @@ -2303,20 +2371,23 @@ llvm_compile_expr(ExprState *state) l_ptr(StructExprContext)); v_current_setp = - LLVMBuildStructGEP(b, - v_aggstatep, - FIELDNO_AGGSTATE_CURRENT_SET, - "aggstate.current_set"); + l_struct_gep(b, + StructAggState, + v_aggstatep, + FIELDNO_AGGSTATE_CURRENT_SET, + "aggstate.current_set"); v_curaggcontext = - LLVMBuildStructGEP(b, - v_aggstatep, - FIELDNO_AGGSTATE_CURAGGCONTEXT, - "aggstate.curaggcontext"); + l_struct_gep(b, + StructAggState, + v_aggstatep, + FIELDNO_AGGSTATE_CURAGGCONTEXT, + "aggstate.curaggcontext"); v_current_pertransp = - LLVMBuildStructGEP(b, - v_aggstatep, - FIELDNO_AGGSTATE_CURPERTRANS, - "aggstate.curpertrans"); + l_struct_gep(b, + StructAggState, + v_aggstatep, + FIELDNO_AGGSTATE_CURPERTRANS, + "aggstate.curpertrans"); /* set aggstate globals */ LLVMBuildStore(b, v_aggcontext, v_curaggcontext); @@ -2332,19 +2403,25 @@ llvm_compile_expr(ExprState *state) /* store transvalue in fcinfo->args[0] */ v_transvaluep = - LLVMBuildStructGEP(b, v_pergroupp, - FIELDNO_AGGSTATEPERGROUPDATA_TRANSVALUE, - "transvalue"); + l_struct_gep(b, + StructAggStatePerGroupData, + v_pergroupp, + FIELDNO_AGGSTATEPERGROUPDATA_TRANSVALUE, + "transvalue"); v_transnullp = - LLVMBuildStructGEP(b, v_pergroupp, - FIELDNO_AGGSTATEPERGROUPDATA_TRANSVALUEISNULL, - "transnullp"); + l_struct_gep(b, + StructAggStatePerGroupData, + v_pergroupp, + FIELDNO_AGGSTATEPERGROUPDATA_TRANSVALUEISNULL, + "transnullp"); LLVMBuildStore(b, - LLVMBuildLoad(b, v_transvaluep, - "transvalue"), + l_load(b, + TypeSizeT, + v_transvaluep, + "transvalue"), l_funcvaluep(b, v_fcinfo, 0)); LLVMBuildStore(b, - LLVMBuildLoad(b, v_transnullp, "transnull"), + l_load(b, TypeStorageBool, v_transnullp, "transnull"), l_funcnullp(b, v_fcinfo, 0)); /* and invoke transition function */ @@ -2377,8 +2454,8 @@ llvm_compile_expr(ExprState *state) b_nocall = l_bb_before_v(opblocks[opno + 1], "op.%d.transnocall", opno); - v_transvalue = LLVMBuildLoad(b, v_transvaluep, ""); - v_transnull = LLVMBuildLoad(b, v_transnullp, ""); + v_transvalue = l_load(b, TypeSizeT, v_transvaluep, ""); + v_transnull = l_load(b, TypeStorageBool, v_transnullp, ""); /* * DatumGetPointer(newVal) != @@ -2404,9 +2481,11 @@ llvm_compile_expr(ExprState *state) v_fn = llvm_pg_func(mod, "ExecAggTransReparent"); v_newval = - LLVMBuildCall(b, v_fn, - params, lengthof(params), - ""); + l_call(b, + LLVMGetFunctionType(v_fn), + v_fn, + params, lengthof(params), + ""); /* store trans value */ LLVMBuildStore(b, v_newval, v_transvaluep); @@ -2516,15 +2595,17 @@ BuildV1Call(LLVMJitContext *context, LLVMBuilderRef b, v_fn = llvm_function_reference(context, b, mod, fcinfo); v_fcinfo = l_ptr_const(fcinfo, l_ptr(StructFunctionCallInfoData)); - v_fcinfo_isnullp = LLVMBuildStructGEP(b, v_fcinfo, - FIELDNO_FUNCTIONCALLINFODATA_ISNULL, - "v_fcinfo_isnull"); + v_fcinfo_isnullp = l_struct_gep(b, + StructFunctionCallInfoData, + v_fcinfo, + FIELDNO_FUNCTIONCALLINFODATA_ISNULL, + "v_fcinfo_isnull"); LLVMBuildStore(b, l_sbool_const(0), v_fcinfo_isnullp); - v_retval = LLVMBuildCall(b, v_fn, &v_fcinfo, 1, "funccall"); + v_retval = l_call(b, LLVMGetFunctionType(AttributeTemplate), v_fn, &v_fcinfo, 1, "funccall"); if (v_fcinfo_isnull) - *v_fcinfo_isnull = LLVMBuildLoad(b, v_fcinfo_isnullp, ""); + *v_fcinfo_isnull = l_load(b, TypeStorageBool, v_fcinfo_isnullp, ""); /* * Add lifetime-end annotation, signaling that writes to memory don't have @@ -2536,11 +2617,11 @@ BuildV1Call(LLVMJitContext *context, LLVMBuilderRef b, params[0] = l_int64_const(sizeof(NullableDatum) * fcinfo->nargs); params[1] = l_ptr_const(fcinfo->args, l_ptr(LLVMInt8Type())); - LLVMBuildCall(b, v_lifetime, params, lengthof(params), ""); + l_call(b, LLVMGetFunctionType(v_lifetime), v_lifetime, params, lengthof(params), ""); params[0] = l_int64_const(sizeof(fcinfo->isnull)); params[1] = l_ptr_const(&fcinfo->isnull, l_ptr(LLVMInt8Type())); - LLVMBuildCall(b, v_lifetime, params, lengthof(params), ""); + l_call(b, LLVMGetFunctionType(v_lifetime), v_lifetime, params, lengthof(params), ""); } return v_retval; @@ -2572,7 +2653,7 @@ build_EvalXFuncInt(LLVMBuilderRef b, LLVMModuleRef mod, const char *funcname, for (int i = 0; i < nargs; i++) params[argno++] = v_args[i]; - v_ret = LLVMBuildCall(b, v_fn, params, argno, ""); + v_ret = l_call(b, LLVMGetFunctionType(v_fn), v_fn, params, argno, ""); pfree(params); diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index 2deb65c5b5c..58c5d1359c1 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -48,7 +48,7 @@ PGFunction TypePGFunction; size_t TypeSizeT; bool TypeStorageBool; -ExprStateEvalFunc TypeExprStateEvalFunc; + ExecEvalSubroutine TypeExecEvalSubroutine; ExecEvalBoolSubroutine TypeExecEvalBoolSubroutine; @@ -61,11 +61,14 @@ ExprEvalStep StructExprEvalStep; ExprState StructExprState; FunctionCallInfoBaseData StructFunctionCallInfoData; HeapTupleData StructHeapTupleData; +HeapTupleHeaderData StructHeapTupleHeaderData; MemoryContextData StructMemoryContextData; TupleTableSlot StructTupleTableSlot; HeapTupleTableSlot StructHeapTupleTableSlot; MinimalTupleTableSlot StructMinimalTupleTableSlot; TupleDescData StructTupleDescData; +PlanState StructPlanState; +MinimalTupleData StructMinimalTupleData; /* @@ -77,9 +80,42 @@ extern Datum AttributeTemplate(PG_FUNCTION_ARGS); Datum AttributeTemplate(PG_FUNCTION_ARGS) { + AssertVariableIsOfType(&AttributeTemplate, PGFunction); + PG_RETURN_NULL(); } +/* + * And some more "templates" to give us examples of function types + * corresponding to function pointer types. + */ + +extern void ExecEvalSubroutineTemplate(ExprState *state, + struct ExprEvalStep *op, + ExprContext *econtext); +void +ExecEvalSubroutineTemplate(ExprState *state, + struct ExprEvalStep *op, + ExprContext *econtext) +{ + AssertVariableIsOfType(&ExecEvalSubroutineTemplate, + ExecEvalSubroutine); +} + +extern bool ExecEvalBoolSubroutineTemplate(ExprState *state, + struct ExprEvalStep *op, + ExprContext *econtext); +bool +ExecEvalBoolSubroutineTemplate(ExprState *state, + struct ExprEvalStep *op, + ExprContext *econtext) +{ + AssertVariableIsOfType(&ExecEvalBoolSubroutineTemplate, + ExecEvalBoolSubroutine); + + return false; +} + /* * Clang represents stdbool.h style booleans that are returned by functions * differently (as i1) than stored ones (as i8). Therefore we do not just need @@ -136,4 +172,5 @@ void *referenced_functions[] = slot_getsomeattrs_int, strlen, varsize_any, + ExecInterpExprStillValid, }; diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index 692483d3b93..ffda94a9938 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -76,3 +76,15 @@ LLVMGetAttributeCountAtIndexPG(LLVMValueRef F, uint32 Idx) */ return LLVMGetAttributeCountAtIndex(F, Idx); } + +LLVMTypeRef +LLVMGetFunctionReturnType(LLVMValueRef r) +{ + return llvm::wrap(llvm::unwrap(r)->getReturnType()); +} + +LLVMTypeRef +LLVMGetFunctionType(LLVMValueRef r) +{ + return llvm::wrap(llvm::unwrap(r)->getFunctionType()); +} diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h index 3560715e329..ffc2307ee14 100644 --- a/src/include/jit/llvmjit.h +++ b/src/include/jit/llvmjit.h @@ -67,6 +67,8 @@ extern LLVMTypeRef TypeStorageBool; extern LLVMTypeRef StructNullableDatum; extern LLVMTypeRef StructTupleDescData; extern LLVMTypeRef StructHeapTupleData; +extern LLVMTypeRef StructHeapTupleHeaderData; +extern LLVMTypeRef StructMinimalTupleData; extern LLVMTypeRef StructTupleTableSlot; extern LLVMTypeRef StructHeapTupleTableSlot; extern LLVMTypeRef StructMinimalTupleTableSlot; @@ -80,6 +82,8 @@ extern LLVMTypeRef StructAggStatePerTransData; extern LLVMTypeRef StructAggStatePerGroupData; extern LLVMValueRef AttributeTemplate; +extern LLVMValueRef ExecEvalBoolSubroutineTemplate; +extern LLVMValueRef ExecEvalSubroutineTemplate; extern void llvm_enter_fatal_on_oom(void); @@ -133,6 +137,8 @@ extern char *LLVMGetHostCPUFeatures(void); #endif extern unsigned LLVMGetAttributeCountAtIndexPG(LLVMValueRef F, uint32 Idx); +extern LLVMTypeRef LLVMGetFunctionReturnType(LLVMValueRef r); +extern LLVMTypeRef LLVMGetFunctionType(LLVMValueRef r); #ifdef __cplusplus } /* extern "C" */ diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h index a9e8d65dfee..c7e31249baa 100644 --- a/src/include/jit/llvmjit_emit.h +++ b/src/include/jit/llvmjit_emit.h @@ -16,6 +16,7 @@ #ifdef USE_LLVM #include +#include #include "jit/llvmjit.h" @@ -103,26 +104,65 @@ l_pbool_const(bool i) return LLVMConstInt(TypeParamBool, (int) i, false); } +static inline LLVMValueRef +l_struct_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, int32 idx, const char *name) +{ +#if LLVM_VERSION_MAJOR < 16 + return LLVMBuildStructGEP(b, v, idx, ""); +#else + return LLVMBuildStructGEP2(b, t, v, idx, ""); +#endif +} + +static inline LLVMValueRef +l_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, LLVMValueRef *indices, int32 nindices, const char *name) +{ +#if LLVM_VERSION_MAJOR < 16 + return LLVMBuildGEP(b, v, indices, nindices, name); +#else + return LLVMBuildGEP2(b, t, v, indices, nindices, name); +#endif +} + +static inline LLVMValueRef +l_load(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, const char *name) +{ +#if LLVM_VERSION_MAJOR < 16 + return LLVMBuildLoad(b, v, name); +#else + return LLVMBuildLoad2(b, t, v, name); +#endif +} + +static inline LLVMValueRef +l_call(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef fn, LLVMValueRef *args, int32 nargs, const char *name) +{ +#if LLVM_VERSION_MAJOR < 16 + return LLVMBuildCall(b, fn, args, nargs, name); +#else + return LLVMBuildCall2(b, t, fn, args, nargs, name); +#endif +} + /* * Load a pointer member idx from a struct. */ static inline LLVMValueRef -l_load_struct_gep(LLVMBuilderRef b, LLVMValueRef v, int32 idx, const char *name) +l_load_struct_gep(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, int32 idx, const char *name) { - LLVMValueRef v_ptr = LLVMBuildStructGEP(b, v, idx, ""); - - return LLVMBuildLoad(b, v_ptr, name); + return l_load(b, + LLVMStructGetTypeAtIndex(t, idx), + l_struct_gep(b, t, v, idx, ""), + name); } /* * Load value of a pointer, after applying one index operation. */ static inline LLVMValueRef -l_load_gep1(LLVMBuilderRef b, LLVMValueRef v, LLVMValueRef idx, const char *name) +l_load_gep1(LLVMBuilderRef b, LLVMTypeRef t, LLVMValueRef v, LLVMValueRef idx, const char *name) { - LLVMValueRef v_ptr = LLVMBuildGEP(b, v, &idx, 1, ""); - - return LLVMBuildLoad(b, v_ptr, name); + return l_load(b, t, l_gep(b, t, v, &idx, 1, ""), name); } /* separate, because pg_attribute_printf(2, 3) can't appear in definition */ @@ -210,7 +250,7 @@ l_mcxt_switch(LLVMModuleRef mod, LLVMBuilderRef b, LLVMValueRef nc) if (!(cur = LLVMGetNamedGlobal(mod, cmc))) cur = LLVMAddGlobal(mod, l_ptr(StructMemoryContextData), cmc); - ret = LLVMBuildLoad(b, cur, cmc); + ret = l_load(b, l_ptr(StructMemoryContextData), cur, cmc); LLVMBuildStore(b, nc, cur); return ret; @@ -225,13 +265,21 @@ l_funcnullp(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno) LLVMValueRef v_args; LLVMValueRef v_argn; - v_args = LLVMBuildStructGEP(b, - v_fcinfo, - FIELDNO_FUNCTIONCALLINFODATA_ARGS, - ""); - v_argn = LLVMBuildStructGEP(b, v_args, argno, ""); - - return LLVMBuildStructGEP(b, v_argn, FIELDNO_NULLABLE_DATUM_ISNULL, ""); + v_args = l_struct_gep(b, + StructFunctionCallInfoData, + v_fcinfo, + FIELDNO_FUNCTIONCALLINFODATA_ARGS, + ""); + v_argn = l_struct_gep(b, + LLVMArrayType(StructNullableDatum, 0), + v_args, + argno, + ""); + return l_struct_gep(b, + StructNullableDatum, + v_argn, + FIELDNO_NULLABLE_DATUM_ISNULL, + ""); } /* @@ -243,13 +291,21 @@ l_funcvaluep(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno) LLVMValueRef v_args; LLVMValueRef v_argn; - v_args = LLVMBuildStructGEP(b, - v_fcinfo, - FIELDNO_FUNCTIONCALLINFODATA_ARGS, - ""); - v_argn = LLVMBuildStructGEP(b, v_args, argno, ""); - - return LLVMBuildStructGEP(b, v_argn, FIELDNO_NULLABLE_DATUM_DATUM, ""); + v_args = l_struct_gep(b, + StructFunctionCallInfoData, + v_fcinfo, + FIELDNO_FUNCTIONCALLINFODATA_ARGS, + ""); + v_argn = l_struct_gep(b, + LLVMArrayType(StructNullableDatum, 0), + v_args, + argno, + ""); + return l_struct_gep(b, + StructNullableDatum, + v_argn, + FIELDNO_NULLABLE_DATUM_DATUM, + ""); } /* @@ -258,7 +314,7 @@ l_funcvaluep(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno) static inline LLVMValueRef l_funcnull(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno) { - return LLVMBuildLoad(b, l_funcnullp(b, v_fcinfo, argno), ""); + return l_load(b, TypeStorageBool, l_funcnullp(b, v_fcinfo, argno), ""); } /* @@ -267,7 +323,7 @@ l_funcnull(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno) static inline LLVMValueRef l_funcvalue(LLVMBuilderRef b, LLVMValueRef v_fcinfo, size_t argno) { - return LLVMBuildLoad(b, l_funcvaluep(b, v_fcinfo, argno), ""); + return l_load(b, TypeSizeT, l_funcvaluep(b, v_fcinfo, argno), ""); } #endif /* USE_LLVM */ From 95528a2adc80b1647c623d2c85caa67c8660ee13 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Thu, 19 Oct 2023 03:01:55 +1300 Subject: [PATCH 67/91] jit: Supply LLVMGlobalGetValueType() for LLVM < 8. Commit 37d5babb used this C API function while adding support for LLVM 16 and opaque pointers, but it's not available in LLVM 7 and older. Provide it in our own llvmjit_wrap.cpp. It just calls a C++ function that pre-dates LLVM 3.9, our minimum target. Back-patch to 12, like 37d5babb. Discussion: https://postgr.es/m/CA%2BhUKGKnLnJnWrkr%3D4mSGhE5FuTK55FY15uULR7%3Dzzc%3DwX4Nqw%40mail.gmail.com --- src/backend/jit/llvm/llvmjit_wrap.cpp | 8 ++++++++ src/include/jit/llvmjit.h | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index ffda94a9938..8008b9abd06 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -88,3 +88,11 @@ LLVMGetFunctionType(LLVMValueRef r) { return llvm::wrap(llvm::unwrap(r)->getFunctionType()); } + +#if LLVM_VERSION_MAJOR < 8 +LLVMTypeRef +LLVMGlobalGetValueType(LLVMValueRef g) +{ + return llvm::wrap(llvm::unwrap(g)->getValueType()); +} +#endif diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h index ffc2307ee14..60cb5d51f5a 100644 --- a/src/include/jit/llvmjit.h +++ b/src/include/jit/llvmjit.h @@ -140,6 +140,10 @@ extern unsigned LLVMGetAttributeCountAtIndexPG(LLVMValueRef F, uint32 Idx); extern LLVMTypeRef LLVMGetFunctionReturnType(LLVMValueRef r); extern LLVMTypeRef LLVMGetFunctionType(LLVMValueRef r); +#if LLVM_MAJOR_VERSION < 8 +extern LLVMTypeRef LLVMGlobalGetValueType(LLVMValueRef g); +#endif + #ifdef __cplusplus } /* extern "C" */ #endif From e9c7c36be50350596a24675962577fc7a60a96b8 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 18 Oct 2023 22:15:54 +1300 Subject: [PATCH 68/91] jit: Changes for LLVM 17. Changes required by https://llvm.org/docs/NewPassManager.html. Back-patch to 12, leaving the final release of 11 unchanged, consistent with earlier decision not to back-patch LLVM 16 support either. Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Andres Freund Reviewed-by: Thomas Munro Discussion: https://postgr.es/m/CA%2BhUKG%2BWXznXCyTgCADd%3DHWkP9Qksa6chd7L%3DGCnZo-MBgg9Lg%40mail.gmail.com --- src/backend/jit/llvm/llvmjit.c | 31 +++++++++++++++++++++++++++ src/backend/jit/llvm/llvmjit_wrap.cpp | 6 ++++++ 2 files changed, 37 insertions(+) diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index a58ac6d11db..b888fad028d 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -18,6 +18,9 @@ #include #include #include +#if LLVM_VERSION_MAJOR > 16 +#include +#endif #if LLVM_VERSION_MAJOR > 11 #include #include @@ -27,12 +30,14 @@ #endif #include #include +#if LLVM_VERSION_MAJOR < 17 #include #include #include #if LLVM_VERSION_MAJOR > 6 #include #endif +#endif #include "jit/llvmjit.h" #include "jit/llvmjit_emit.h" @@ -561,6 +566,7 @@ llvm_function_reference(LLVMJitContext *context, static void llvm_optimize_module(LLVMJitContext *context, LLVMModuleRef module) { +#if LLVM_VERSION_MAJOR < 17 LLVMPassManagerBuilderRef llvm_pmb; LLVMPassManagerRef llvm_mpm; LLVMPassManagerRef llvm_fpm; @@ -624,6 +630,31 @@ llvm_optimize_module(LLVMJitContext *context, LLVMModuleRef module) LLVMDisposePassManager(llvm_mpm); LLVMPassManagerBuilderDispose(llvm_pmb); +#else + LLVMPassBuilderOptionsRef options; + LLVMErrorRef err; + const char *passes; + + if (context->base.flags & PGJIT_OPT3) + passes = "default"; + else + passes = "default,mem2reg"; + + options = LLVMCreatePassBuilderOptions(); + +#ifdef LLVM_PASS_DEBUG + LLVMPassBuilderOptionsSetDebugLogging(options, 1); +#endif + + LLVMPassBuilderOptionsSetInlinerThreshold(options, 512); + + err = LLVMRunPasses(module, passes, NULL, options); + + if (err) + elog(ERROR, "failed to JIT module: %s", llvm_error_message(err)); + + LLVMDisposePassBuilderOptions(options); +#endif } /* diff --git a/src/backend/jit/llvm/llvmjit_wrap.cpp b/src/backend/jit/llvm/llvmjit_wrap.cpp index 8008b9abd06..2962083c50f 100644 --- a/src/backend/jit/llvm/llvmjit_wrap.cpp +++ b/src/backend/jit/llvm/llvmjit_wrap.cpp @@ -23,8 +23,14 @@ extern "C" #include #include +#if LLVM_VERSION_MAJOR < 17 #include +#endif +#if LLVM_VERSION_MAJOR > 16 +#include +#else #include +#endif #include "jit/llvmjit.h" From 556ec26ac2cb7c45d1216c6ea2bcb9709fa3f163 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 18 Oct 2023 20:43:17 -0400 Subject: [PATCH 69/91] Improve pglz_decompress's defenses against corrupt compressed data. When processing a match tag, check to see if the claimed "off" is more than the distance back to the output buffer start. If it is, then the data is corrupt, and what's more we would fetch from outside the buffer boundaries and potentially incur a SIGSEGV. (Although the odds of that seem relatively low, given that "off" can't be more than 4K.) Back-patch to v13; before that, this function wasn't really trying to protect against bad data. Report and fix by Flavien Guedez. Discussion: https://postgr.es/m/01fc0593-e31e-463d-902c-dd43174acee2@oopacity.net --- src/common/pg_lzcompress.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c index a30a2c2eb83..02e3b2a0087 100644 --- a/src/common/pg_lzcompress.c +++ b/src/common/pg_lzcompress.c @@ -735,11 +735,15 @@ pglz_decompress(const char *source, int32 slen, char *dest, /* * Check for corrupt data: if we fell off the end of the - * source, or if we obtained off = 0, we have problems. (We - * must check this, else we risk an infinite loop below in the - * face of corrupt data.) + * source, or if we obtained off = 0, or if off is more than + * the distance back to the buffer start, we have problems. + * (We must check for off = 0, else we risk an infinite loop + * below in the face of corrupt data. Likewise, the upper + * limit on off prevents accessing outside the buffer + * boundaries.) */ - if (unlikely(sp > srcend || off == 0)) + if (unlikely(sp > srcend || off == 0 || + off > (dp - (unsigned char *) dest))) return -1; /* From 7a0c8816d3554ecb04005242b233a8a3256d871d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 20 Oct 2023 13:01:02 -0400 Subject: [PATCH 70/91] Doc: update CREATE OPERATOR's statement about => as an operator. This doco said that use of => as an operator "is deprecated". It's been fully disallowed since 865f14a2d back in 9.5, but evidently that commit missed updating this statement. Do so now. --- doc/src/sgml/ref/create_operator.sgml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/create_operator.sgml b/doc/src/sgml/ref/create_operator.sgml index e27512ff391..d4d45ec357d 100644 --- a/doc/src/sgml/ref/create_operator.sgml +++ b/doc/src/sgml/ref/create_operator.sgml @@ -52,7 +52,8 @@ CREATE OPERATOR name ( There are a few restrictions on your choice of name: - -- and /* cannot appear anywhere in an operator name, + + -- and /* cannot appear anywhere in an operator name, since they will be taken as the start of a comment. @@ -72,8 +73,8 @@ CREATE OPERATOR name ( - The use of => as an operator name is deprecated. It may - be disallowed altogether in a future release. + The symbol => is reserved by the SQL grammar, + so it cannot be used as an operator name. From 477dd03de8ffe9274028a88797a04eef7dc3b76b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 20 Oct 2023 13:40:15 -0400 Subject: [PATCH 71/91] Dodge a compiler bug affecting timetz_zone/timetz_izone. This avoids a compiler bug occurring in AIX's xlc, even in pretty late-model revisions. Buildfarm testing has now confirmed that only 64-bit xlc is affected. Although we are contemplating dropping support for xlc in v17, it's still supported in the back branches, so we need this fix. Back-patch of code changes from HEAD commit 19fa97731. (The test cases were already back-patched, in 4a427b82c et al.) Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com --- src/backend/utils/adt/date.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index 70c8791b611..861364c7ab1 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -3176,10 +3176,11 @@ timetz_zone(PG_FUNCTION_ARGS) result = (TimeTzADT *) palloc(sizeof(TimeTzADT)); result->time = t->time + (t->zone - tz) * USECS_PER_SEC; + /* C99 modulo has the wrong sign convention for negative input */ while (result->time < INT64CONST(0)) result->time += USECS_PER_DAY; - while (result->time >= USECS_PER_DAY) - result->time -= USECS_PER_DAY; + if (result->time >= USECS_PER_DAY) + result->time %= USECS_PER_DAY; result->zone = tz; @@ -3209,10 +3210,11 @@ timetz_izone(PG_FUNCTION_ARGS) result = (TimeTzADT *) palloc(sizeof(TimeTzADT)); result->time = time->time + (time->zone - tz) * USECS_PER_SEC; + /* C99 modulo has the wrong sign convention for negative input */ while (result->time < INT64CONST(0)) result->time += USECS_PER_DAY; - while (result->time >= USECS_PER_DAY) - result->time -= USECS_PER_DAY; + if (result->time >= USECS_PER_DAY) + result->time %= USECS_PER_DAY; result->zone = tz; From 2e9109ec12aca4275655ef5e91f044a71afdb018 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sun, 22 Oct 2023 10:04:55 +1300 Subject: [PATCH 72/91] Fix min_dynamic_shared_memory on Windows. When min_dynamic_shared_memory is set above 0, we try to find space in a pre-allocated region of the main shared memory area instead of calling dsm_impl_XXX() routines to allocate more. The dsm_pin_segment() and dsm_unpin_segment() routines had a bug: they called dsm_impl_XXX() routines even for main region segments. Nobody noticed before now because those routines do nothing on Unix, but on Windows they'd fail while attempting to duplicate an invalid Windows HANDLE. Add the missing gating. Back-patch to 14, where commit 84b1c63a added this feature. Fixes pgsql-bugs bug #18165. Reported-by: Maxime Boyer Tested-by: Alexander Lakhin Discussion: https://postgr.es/m/18165-bf4f525cea6e51de%40postgresql.org --- src/backend/storage/ipc/dsm.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c index 02e344a46c3..0c10a2711a0 100644 --- a/src/backend/storage/ipc/dsm.c +++ b/src/backend/storage/ipc/dsm.c @@ -922,7 +922,7 @@ dsm_unpin_mapping(dsm_segment *seg) void dsm_pin_segment(dsm_segment *seg) { - void *handle; + void *handle = NULL; /* * Bump reference count for this segment in shared memory. This will @@ -933,7 +933,8 @@ dsm_pin_segment(dsm_segment *seg) LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE); if (dsm_control->item[seg->control_slot].pinned) elog(ERROR, "cannot pin a segment that is already pinned"); - dsm_impl_pin_segment(seg->handle, seg->impl_private, &handle); + if (!is_main_region_dsm_handle(seg->handle)) + dsm_impl_pin_segment(seg->handle, seg->impl_private, &handle); dsm_control->item[seg->control_slot].pinned = true; dsm_control->item[seg->control_slot].refcnt++; dsm_control->item[seg->control_slot].impl_private_pm_handle = handle; @@ -990,8 +991,9 @@ dsm_unpin_segment(dsm_handle handle) * releasing the lock, because impl_private_pm_handle may get modified by * dsm_impl_unpin_segment. */ - dsm_impl_unpin_segment(handle, - &dsm_control->item[control_slot].impl_private_pm_handle); + if (!is_main_region_dsm_handle(handle)) + dsm_impl_unpin_segment(handle, + &dsm_control->item[control_slot].impl_private_pm_handle); /* Note that 1 means no references (0 means unused slot). */ if (--dsm_control->item[control_slot].refcnt == 1) From be5b0e1c87b1e079ddb13540ffaa8c8c2a37e027 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sun, 22 Oct 2023 14:17:00 +1300 Subject: [PATCH 73/91] Log LLVM library version in configure output. When scanning build farm results, it's useful to be able to see which version is in use. For the Meson build system, this information was already displayed. Back-patch to all supported branches. Discussion: https://postgr.es/m/4022690.1697852728%40sss.pgh.pa.us --- config/llvm.m4 | 1 + configure | 2 ++ 2 files changed, 3 insertions(+) diff --git a/config/llvm.m4 b/config/llvm.m4 index 3a75cd8b4df..21d8cd4f90f 100644 --- a/config/llvm.m4 +++ b/config/llvm.m4 @@ -28,6 +28,7 @@ AC_DEFUN([PGAC_LLVM_SUPPORT], if echo $pgac_llvm_version | $AWK -F '.' '{ if ([$]1 >= 4 || ([$]1 == 3 && [$]2 >= 9)) exit 1; else exit 0;}';then AC_MSG_ERROR([$LLVM_CONFIG version is $pgac_llvm_version but at least 3.9 is required]) fi + AC_MSG_NOTICE([using llvm $pgac_llvm_version]) # need clang to create some bitcode files AC_ARG_VAR(CLANG, [path to clang compiler to generate bitcode]) diff --git a/configure b/configure index 304edf858f0..84dd5289a87 100755 --- a/configure +++ b/configure @@ -5458,6 +5458,8 @@ fi if echo $pgac_llvm_version | $AWK -F '.' '{ if ($1 >= 4 || ($1 == 3 && $2 >= 9)) exit 1; else exit 0;}';then as_fn_error $? "$LLVM_CONFIG version is $pgac_llvm_version but at least 3.9 is required" "$LINENO" 5 fi + { $as_echo "$as_me:${as_lineno-$LINENO}: using llvm $pgac_llvm_version" >&5 +$as_echo "$as_me: using llvm $pgac_llvm_version" >&6;} # need clang to create some bitcode files From 27999c35a2411fff417b6a46bebfb9e17e889386 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 24 Oct 2023 09:27:21 -0700 Subject: [PATCH 74/91] Doc: indexUnchanged is strictly a hint. Clearly spell out the limitations of aminsert()'s indexUnchanged hinting mechanism in the index AM documentation. Oversight in commit 9dc718bd, which added the "logically unchanged index" hint (which is used to trigger bottom-up index deletion). Author: Peter Geoghegan Reported-By: Tom Lane Reviewed-By: Tom Lane Discussion: https://postgr.es/m/CAH2-WzmU_BQ=-H9L+bxTSMQBqHMjp1DSwGypvL0gKs+dTOfkKg@mail.gmail.com Backpatch: 14-, where indexUnchanged hinting was introduced. --- doc/src/sgml/indexam.sgml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index 4f83970c851..1a787179a34 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -319,9 +319,13 @@ aminsert (Relation indexRelation, modify any columns covered by the index, but nevertheless requires a new version in the index. The index AM may use this hint to decide to apply bottom-up index deletion in parts of the index where many - versions of the same logical row accumulate. Note that updating a - non-key column does not affect the value of - indexUnchanged. + versions of the same logical row accumulate. Note that updating a non-key + column or a column that only appears in a partial index predicate does not + affect the value of indexUnchanged. The core code + determines each tuple's indexUnchanged value using a low + overhead approach that allows both false positives and false negatives. + Index AMs must not treat indexUnchanged as an + authoritative source of information about tuple visibility or versioning. From 39d097c07d19485f3827ae58e3c4b7966be273de Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 25 Oct 2023 09:41:13 +0900 Subject: [PATCH 75/91] doc: Fix some typos and grammar Author: Ekaterina Kiryanova, Elena Indrupskaya, Oleg Sibiryakov, Maxim Yablokov Discussion: https://postgr.es/m/7aad518b-3e6d-47f3-9184-b1d69cb412e7@postgrespro.ru Backpatch-through: 11 --- doc/src/sgml/ref/create_foreign_table.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/ref/create_foreign_table.sgml b/doc/src/sgml/ref/create_foreign_table.sgml index 7ad22ea0783..d1717fade90 100644 --- a/doc/src/sgml/ref/create_foreign_table.sgml +++ b/doc/src/sgml/ref/create_foreign_table.sgml @@ -423,7 +423,7 @@ int totalNumberOfSegments = getgpsegmentCount(); an UPDATE that changes the partition key value can cause a row to be moved from a local partition to a foreign-table partition, provided the foreign data wrapper supports tuple routing. - However it is not currently possible to move a row from a + However, it is not currently possible to move a row from a foreign-table partition to another partition. An UPDATE that would require doing that will fail due to the partitioning constraint, assuming that that is properly From b9f51e9d2048ff5f4098c7aba6b71a1bfb403f13 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 25 Oct 2023 17:34:47 -0400 Subject: [PATCH 76/91] Doc: remove misleading info about ecpg's CONNECT/DISCONNECT DEFAULT. As far as I can see, ecpg has no notion of a "default" open connection. You can do "CONNECT TO DEFAULT" but that just specifies letting libpq use all its default connection parameters --- the resulting connection is not special subsequently. In particular, SET CONNECTION = DEFAULT and DISCONNECT DEFAULT simply act on a connection named DEFAULT, if you've made one; they do not have special lookup rules. But the documentation of these commands makes it look like they do. Simplest fix, I think, is just to remove the paras suggesting that DEFAULT is special here. Also, SET CONNECTION *does* have one special lookup rule, which is that it recognizes CURRENT as an alias for the currently selected connection. SET CONNECTION = CURRENT is a no-op, so it's pretty useless, but nonetheless it does something different from selecting a connection by name; so we'd better document it. Per report from Sylvain Frandaz. Back-patch to all supported versions. Discussion: https://postgr.es/m/169824721149.1769274.1553568436817652238@wrigleys.postgresql.org --- doc/src/sgml/ecpg.sgml | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml index 9df09df4d77..46187a563be 100644 --- a/doc/src/sgml/ecpg.sgml +++ b/doc/src/sgml/ecpg.sgml @@ -412,12 +412,6 @@ EXEC SQL DISCONNECT connection; - - - DEFAULT - - - CURRENT @@ -7093,7 +7087,6 @@ EXEC SQL DEALLOCATE DESCRIPTOR mydesc; DISCONNECT connection_name DISCONNECT [ CURRENT ] -DISCONNECT DEFAULT DISCONNECT ALL @@ -7134,15 +7127,6 @@ DISCONNECT ALL - - DEFAULT - - - Close the default connection. - - - - ALL @@ -7161,13 +7145,11 @@ DISCONNECT ALL int main(void) { - EXEC SQL CONNECT TO testdb AS DEFAULT USER testuser; EXEC SQL CONNECT TO testdb AS con1 USER testuser; EXEC SQL CONNECT TO testdb AS con2 USER testuser; EXEC SQL CONNECT TO testdb AS con3 USER testuser; EXEC SQL DISCONNECT CURRENT; /* close con3 */ - EXEC SQL DISCONNECT DEFAULT; /* close DEFAULT */ EXEC SQL DISCONNECT ALL; /* close con2 and con1 */ return 0; @@ -7736,10 +7718,10 @@ SET CONNECTION [ TO | = ] connection_name - DEFAULT + CURRENT - Set the connection to the default connection. + Set the connection to the current connection (thus, nothing happens). From 790dc0eaa143d966b0d699abe33a7430fae9e351 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 28 Oct 2023 11:54:40 -0400 Subject: [PATCH 77/91] Remove PHOT from our default timezone abbreviations list. Debian recently decided to split out a bunch of "obsolete" timezone names into a new tzdata-legacy package, which isn't installed by default. One of these zone names is Pacific/Enderbury, and that breaks our regression tests (on --with-system-tzdata builds) because our default timezone abbreviations list defines PHOT as Pacific/Enderbury. Pacific/Enderbury got renamed to Pacific/Kanton in tzdata 2021b, so that in distros that still have this entry it's just a symlink to Pacific/Kanton anyway. So one answer would be to redefine PHOT as Pacific/Kanton. However, then things would fail if the installed tzdata predates 2021b, which is recent enough that that seems like a real problem. Instead, let's just remove PHOT from the default list. That seems likely to affect nobody in the real world, because (a) it was an abbreviation that the tzdb crew made up in the first place, with no evidence of real-world usage, and (b) the total human population of the Phoenix Islands is less than two dozen persons, per Wikipedia. If anyone does use this zone abbreviation they can easily put it back via a custom abbreviations file. We'll keep PHOT in the Pacific.txt reference file, but change it to Pacific/Kanton there, as that definition seems more likely to be useful to future readers of that file. Per report from Victor Wagner. Back-patch to all supported branches. Discussion: https://postgr.es/m/20231027152049.4b5c8044@wagner.wagner.home --- src/timezone/tznames/Default | 1 - src/timezone/tznames/Pacific.txt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/timezone/tznames/Default b/src/timezone/tznames/Default index b76d170b968..5e692423b5a 100644 --- a/src/timezone/tznames/Default +++ b/src/timezone/tznames/Default @@ -616,7 +616,6 @@ NZST 43200 # New Zealand Standard Time # (Antarctica/McMurdo) # (Pacific/Auckland) PGT 36000 # Papua New Guinea Time (obsolete) -PHOT Pacific/Enderbury # Phoenix Islands Time (Kiribati) (obsolete) PONT 39600 # Ponape Time (Micronesia) (obsolete) PWT 32400 # Palau Time (obsolete) TAHT -36000 # Tahiti Time (obsolete) diff --git a/src/timezone/tznames/Pacific.txt b/src/timezone/tznames/Pacific.txt index c30008cb049..556a370af58 100644 --- a/src/timezone/tznames/Pacific.txt +++ b/src/timezone/tznames/Pacific.txt @@ -50,7 +50,7 @@ NZST 43200 # New Zealand Standard Time # (Antarctica/McMurdo) # (Pacific/Auckland) PGT 36000 # Papua New Guinea Time (obsolete) -PHOT Pacific/Enderbury # Phoenix Islands Time (Kiribati) (obsolete) +PHOT Pacific/Kanton # Phoenix Islands Time (Kiribati) (obsolete) PONT 39600 # Ponape Time (Micronesia) (obsolete) # CONFLICT! PST is not unique # Other timezones: From 7fb2c238596610b476d81fab6b25e4c29c44019d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 28 Oct 2023 14:04:43 -0400 Subject: [PATCH 78/91] Fix intra-query memory leak when a SRF returns zero rows. When looping around after finding that the set-returning function returned zero rows for the current input tuple, ExecProjectSet neglected to reset either of the two memory contexts it's responsible for cleaning out. Typically this wouldn't cause much problem, because once the SRF does return at least one row, the contexts would get reset on the next call. However, if the SRF returns no rows for many input tuples in succession, quite a lot of memory could be transiently consumed. To fix, make sure we reset both contexts while looping around. Per bug #18172 from Sergei Kornilov. Back-patch to all supported branches. Discussion: https://postgr.es/m/18172-9b8c5fc1d676ded3@postgresql.org --- src/backend/executor/nodeProjectSet.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/backend/executor/nodeProjectSet.c b/src/backend/executor/nodeProjectSet.c index 07be814d7b5..d441a57cc0e 100644 --- a/src/backend/executor/nodeProjectSet.c +++ b/src/backend/executor/nodeProjectSet.c @@ -72,20 +72,22 @@ ExecProjectSet(PlanState *pstate) return resultSlot; } - /* - * Reset argument context to free any expression evaluation storage - * allocated in the previous tuple cycle. Note this can't happen until - * we're done projecting out tuples from a scan tuple, as ValuePerCall - * functions are allowed to reference the arguments for each returned - * tuple. - */ - MemoryContextReset(node->argcontext); - /* * Get another input tuple and project SRFs from it. */ for (;;) { + /* + * Reset argument context to free any expression evaluation storage + * allocated in the previous tuple cycle. Note this can't happen + * until we're done projecting out tuples from a scan tuple, as + * ValuePerCall functions are allowed to reference the arguments for + * each returned tuple. However, if we loop around after finding that + * no rows are produced from a scan tuple, we should reset, to avoid + * leaking memory when many successive scan tuples produce no rows. + */ + MemoryContextReset(node->argcontext); + /* * Retrieve tuples from the outer plan until there are no more. */ @@ -111,6 +113,12 @@ ExecProjectSet(PlanState *pstate) */ if (resultSlot) return resultSlot; + + /* + * When we do loop back, we'd better reset the econtext again, just in + * case the SRF leaked some memory there. + */ + ResetExprContext(econtext); } return NULL; From 5141f8b4f9f266e26d1fcfc5aa60e59851c4e9df Mon Sep 17 00:00:00 2001 From: Dean Rasheed Date: Sun, 29 Oct 2023 11:14:34 +0000 Subject: [PATCH 79/91] btree_gin: Fix calculation of leftmost interval value. Formerly, the value computed by leftmostvalue_interval() was a long way short of the minimum possible interval value. As a result, an index scan on a GIN index on an interval column with < or <= operators would miss large negative interval values. Fix by setting all fields of the leftmost interval to their minimum values, ensuring that the result is less than any other possible interval. Since this only affects index searches, no index rebuild is necessary. Back-patch to all supported branches. Dean Rasheed, reviewed by Heikki Linnakangas. Discussion: https://postgr.es/m/CAEZATCV80%2BgOfF8ehNUUfaKBZgZMDfCfL-g1HhWGb6kC3rpDfw%40mail.gmail.com --- contrib/btree_gin/btree_gin.c | 6 +++--- contrib/btree_gin/expected/interval.out | 16 +++++++++++----- contrib/btree_gin/sql/interval.sql | 4 +++- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/contrib/btree_gin/btree_gin.c b/contrib/btree_gin/btree_gin.c index c50d68ce186..b09bb8df964 100644 --- a/contrib/btree_gin/btree_gin.c +++ b/contrib/btree_gin/btree_gin.c @@ -306,9 +306,9 @@ leftmostvalue_interval(void) { Interval *v = palloc(sizeof(Interval)); - v->time = DT_NOBEGIN; - v->day = 0; - v->month = 0; + v->time = PG_INT64_MIN; + v->day = PG_INT32_MIN; + v->month = PG_INT32_MIN; return IntervalPGetDatum(v); } diff --git a/contrib/btree_gin/expected/interval.out b/contrib/btree_gin/expected/interval.out index 1f6ef54070e..8bb9806650d 100644 --- a/contrib/btree_gin/expected/interval.out +++ b/contrib/btree_gin/expected/interval.out @@ -3,30 +3,34 @@ CREATE TABLE test_interval ( i interval ); INSERT INTO test_interval VALUES + ( '-178000000 years' ), ( '03:55:08' ), ( '04:55:08' ), ( '05:55:08' ), ( '08:55:08' ), ( '09:55:08' ), - ( '10:55:08' ) + ( '10:55:08' ), + ( '178000000 years' ) ; CREATE INDEX idx_interval ON test_interval USING gin (i); SELECT * FROM test_interval WHERE i<'08:55:08'::interval ORDER BY i; i -------------------------- + @ 178000000 years ago @ 3 hours 55 mins 8 secs @ 4 hours 55 mins 8 secs @ 5 hours 55 mins 8 secs -(3 rows) +(4 rows) SELECT * FROM test_interval WHERE i<='08:55:08'::interval ORDER BY i; i -------------------------- + @ 178000000 years ago @ 3 hours 55 mins 8 secs @ 4 hours 55 mins 8 secs @ 5 hours 55 mins 8 secs @ 8 hours 55 mins 8 secs -(4 rows) +(5 rows) SELECT * FROM test_interval WHERE i='08:55:08'::interval ORDER BY i; i @@ -40,12 +44,14 @@ SELECT * FROM test_interval WHERE i>='08:55:08'::interval ORDER BY i; @ 8 hours 55 mins 8 secs @ 9 hours 55 mins 8 secs @ 10 hours 55 mins 8 secs -(3 rows) + @ 178000000 years +(4 rows) SELECT * FROM test_interval WHERE i>'08:55:08'::interval ORDER BY i; i --------------------------- @ 9 hours 55 mins 8 secs @ 10 hours 55 mins 8 secs -(2 rows) + @ 178000000 years +(3 rows) diff --git a/contrib/btree_gin/sql/interval.sql b/contrib/btree_gin/sql/interval.sql index e3851587833..7a2f3ac0d85 100644 --- a/contrib/btree_gin/sql/interval.sql +++ b/contrib/btree_gin/sql/interval.sql @@ -5,12 +5,14 @@ CREATE TABLE test_interval ( ); INSERT INTO test_interval VALUES + ( '-178000000 years' ), ( '03:55:08' ), ( '04:55:08' ), ( '05:55:08' ), ( '08:55:08' ), ( '09:55:08' ), - ( '10:55:08' ) + ( '10:55:08' ), + ( '178000000 years' ) ; CREATE INDEX idx_interval ON test_interval USING gin (i); From 196b2d9e57a136d94d373c49ff54c8f76b29f5ef Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 30 Oct 2023 14:46:05 -0700 Subject: [PATCH 80/91] Diagnose !indisvalid in more SQL functions. pgstatindex failed with ERRCODE_DATA_CORRUPTED, of the "can't-happen" class XX. The other functions succeeded on an empty index; they might have malfunctioned if the failed index build left torn I/O or other complex state. Report an ERROR in statistics functions pgstatindex, pgstatginindex, pgstathashindex, and pgstattuple. Report DEBUG1 and skip all index I/O in maintenance functions brin_desummarize_range, brin_summarize_new_values, brin_summarize_range, and gin_clean_pending_list. Back-patch to v11 (all supported versions). Discussion: https://postgr.es/m/20231001195309.a3@google.com --- contrib/pgstattuple/pgstatindex.c | 26 ++++++++++++++++++++++++++ contrib/pgstattuple/pgstattuple.c | 7 +++++++ src/backend/access/brin/brin.c | 27 +++++++++++++++++++++------ src/backend/access/gin/ginfast.c | 23 ++++++++++++++++++++--- 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index f7763b2e2fc..fdfaef925cd 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -237,6 +237,18 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary tables of other sessions"))); + /* + * A !indisready index could lead to ERRCODE_DATA_CORRUPTED later, so exit + * early. We're capable of assessing an indisready&&!indisvalid index, + * but the results could be confusing. For example, the index's size + * could be too low for a valid index of the table. + */ + if (!rel->rd_index->indisvalid) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(rel)))); + /* * Read metapage */ @@ -542,6 +554,13 @@ pgstatginindex_internal(Oid relid, FunctionCallInfo fcinfo) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary indexes of other sessions"))); + /* see pgstatindex_impl */ + if (!rel->rd_index->indisvalid) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(rel)))); + /* * Read metapage */ @@ -619,6 +638,13 @@ pgstathashindex(PG_FUNCTION_ARGS) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot access temporary indexes of other sessions"))); + /* see pgstatindex_impl */ + if (!rel->rd_index->indisvalid) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(rel)))); + /* Get the information we need from the metapage. */ memset(&stats, 0, sizeof(stats)); metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE); diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c index ae0f09fd05e..cafe43fec3c 100644 --- a/contrib/pgstattuple/pgstattuple.c +++ b/contrib/pgstattuple/pgstattuple.c @@ -264,6 +264,13 @@ pgstat_relation(Relation rel, FunctionCallInfo fcinfo) case RELKIND_DIRECTORY_TABLE: return pgstat_heap(rel, fcinfo); case RELKIND_INDEX: + /* see pgstatindex_impl */ + if (!rel->rd_index->indisvalid) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(rel)))); + switch (rel->rd_rel->relam) { case BTREE_AM_OID: diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index fc565a55cf3..2a9d6c22810 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -1241,8 +1241,14 @@ brin_summarize_range_internal(PG_FUNCTION_ARGS) errmsg("could not open parent table of index \"%s\"", RelationGetRelationName(indexRel)))); - /* OK, do it */ - brinsummarize(indexRel, heapRel, heapBlk, true, &numSummarized, NULL); + /* see gin_clean_pending_list() */ + if (indexRel->rd_index->indisvalid) + brinsummarize(indexRel, heapRel, heapBlk, true, &numSummarized, NULL); + else + ereport(DEBUG1, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(indexRel)))); /* Roll back any GUC changes executed by index functions */ AtEOXact_GUC(false, save_nestlevel); @@ -1327,12 +1333,21 @@ brin_desummarize_range(PG_FUNCTION_ARGS) errmsg("could not open parent table of index \"%s\"", RelationGetRelationName(indexRel)))); - /* the revmap does the hard work */ - do + /* see gin_clean_pending_list() */ + if (indexRel->rd_index->indisvalid) { - done = brinRevmapDesummarizeRange(indexRel, heapBlk); + /* the revmap does the hard work */ + do + { + done = brinRevmapDesummarizeRange(indexRel, heapBlk); + } + while (!done); } - while (!done); + else + ereport(DEBUG1, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(indexRel)))); relation_close(indexRel, ShareUpdateExclusiveLock); relation_close(heapRel, ShareUpdateExclusiveLock); diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c index 8679698b1f2..3f84e90b260 100644 --- a/src/backend/access/gin/ginfast.c +++ b/src/backend/access/gin/ginfast.c @@ -1035,7 +1035,6 @@ gin_clean_pending_list(PG_FUNCTION_ARGS) Oid indexoid = PG_GETARG_OID(0); Relation indexRel = index_open(indexoid, RowExclusiveLock); IndexBulkDeleteResult stats; - GinState ginstate; if (RecoveryInProgress()) ereport(ERROR, @@ -1067,8 +1066,26 @@ gin_clean_pending_list(PG_FUNCTION_ARGS) RelationGetRelationName(indexRel)); memset(&stats, 0, sizeof(stats)); - initGinState(&ginstate, indexRel); - ginInsertCleanup(&ginstate, true, true, true, &stats); + + /* + * Can't assume anything about the content of an !indisready index. Make + * those a no-op, not an error, so users can just run this function on all + * indexes of the access method. Since an indisready&&!indisvalid index + * is merely awaiting missed aminsert calls, we're capable of processing + * it. Decline to do so, out of an abundance of caution. + */ + if (indexRel->rd_index->indisvalid) + { + GinState ginstate; + + initGinState(&ginstate, indexRel); + ginInsertCleanup(&ginstate, true, true, true, &stats); + } + else + ereport(DEBUG1, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("index \"%s\" is not valid", + RelationGetRelationName(indexRel)))); index_close(indexRel, RowExclusiveLock); From 3a942e11c87314488e6b4a5ef64949182e61c34b Mon Sep 17 00:00:00 2001 From: David Rowley Date: Tue, 31 Oct 2023 16:43:28 +1300 Subject: [PATCH 81/91] Adjust the order of the prechecks in pgrowlocks() 4b8266415 added a precheck to pgrowlocks() to ensure the given object's pg_class.relam is HEAP_TABLE_AM_OID, however, that check was put before another check which was checking if the given object was a partitioned table. Since the pg_class.relam is always InvalidOid for partitioned tables, if pgrowlocks() was called passing a partitioned table, then the "only heap AM is supported" error would be raised instead of the intended error about the given object being a partitioned table. Here we simply move the pg_class.relam check to after the check that verifies that we are in fact working with a normal (non-partitioned) table. Reported-by: jian he Discussion: https://postgr.es/m/CACJufxFaSp_WguFCf0X98951zFVX+dXFnF1mxAb-G3g1HiHOow@mail.gmail.com Backpatch-through: 12, where 4b8266415 was introduced. --- contrib/pgrowlocks/pgrowlocks.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/pgrowlocks/pgrowlocks.c b/contrib/pgrowlocks/pgrowlocks.c index fd6d21d0bd1..d4d6a8c7299 100644 --- a/contrib/pgrowlocks/pgrowlocks.c +++ b/contrib/pgrowlocks/pgrowlocks.c @@ -107,10 +107,6 @@ pgrowlocks(PG_FUNCTION_ARGS) relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname)); rel = relation_openrv(relrv, AccessShareLock); - if (rel->rd_rel->relam != HEAP_TABLE_AM_OID) - ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("only heap AM is supported"))); - if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), @@ -122,6 +118,10 @@ pgrowlocks(PG_FUNCTION_ARGS) (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is not a table", RelationGetRelationName(rel)))); + else if (rel->rd_rel->relam != HEAP_TABLE_AM_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("only heap AM is supported"))); /* * check permissions: must have SELECT on table or be in From 419e96fafd47793cb990e19956aaaaa93eaf1802 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 31 Oct 2023 09:10:35 -0400 Subject: [PATCH 82/91] doc: 1-byte varlena headers can be used for user PLAIN storage This also updates some C comments. Reported-by: suchithjn22@gmail.com Discussion: https://postgr.es/m/167336599095.2667301.15497893107226841625@wrigleys.postgresql.org Author: Laurenz Albe (doc patch) Backpatch-through: 11 --- doc/src/sgml/storage.sgml | 4 +--- src/backend/access/common/heaptuple.c | 11 ++++++++++- src/backend/utils/adt/rangetypes.c | 3 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index 41891c96b7d..d4155e5f47e 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -461,9 +461,7 @@ for storing TOAST-able columns on disk: PLAIN prevents either compression or - out-of-line storage; furthermore it disables use of single-byte headers - for varlena types. - This is the only possible strategy for + out-of-line storage. This is the only possible strategy for columns of non-TOAST-able data types. diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index c4ad0d6c3d6..8933bdd0397 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -73,7 +73,16 @@ #include "catalog/pg_type.h" #include "cdb/cdbvars.h" /* GpIdentity.segindex */ -/* Does att's datatype allow packing into the 1-byte-header varlena format? */ +/* + * Does att's datatype allow packing into the 1-byte-header varlena format? + * While functions that use TupleDescAttr() and assign attstorage = + * TYPSTORAGE_PLAIN cannot use packed varlena headers, functions that call + * TupleDescInitEntry() use typeForm->typstorage (TYPSTORAGE_EXTENDED) and + * can use packed varlena headers, e.g.: + * CREATE TABLE test(a VARCHAR(10000) STORAGE PLAIN); + * INSERT INTO test VALUES (repeat('A',10)); + * This can be verified with pageinspect. + */ #define ATT_IS_PACKABLE(att) \ ((att)->attlen == -1 && (att)->attstorage != TYPSTORAGE_PLAIN) /* Use this if it's already known varlena */ diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c index 815175a654e..e3ff545fd3f 100644 --- a/src/backend/utils/adt/rangetypes.c +++ b/src/backend/utils/adt/rangetypes.c @@ -2513,7 +2513,8 @@ range_contains_elem_internal(TypeCacheEntry *typcache, const RangeType *r, Datum * values into a range object. They are modeled after heaptuple.c's * heap_compute_data_size() and heap_fill_tuple(), but we need not handle * null values here. TYPE_IS_PACKABLE must test the same conditions as - * heaptuple.c's ATT_IS_PACKABLE macro. + * heaptuple.c's ATT_IS_PACKABLE macro. See the comments thare for more + * details. */ /* Does datatype allow packing into the 1-byte-header varlena format? */ From 738a4baf0d648a20385cdc9e8732e026015c930f Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 31 Oct 2023 09:23:09 -0400 Subject: [PATCH 83/91] doc: add function argument and query parameter limits Also reorder entries and add commas. Reported-by: David G. Johnston Discussion: https://postgr.es/m/CAKFQuwYeNPxeocV3_0+Zx=_Xwvg+sNyEMdzyG5s2E2e0hZLQhg@mail.gmail.com Author: David G. Johnston (partial) Backpatch-through: 12 --- doc/src/sgml/limits.sgml | 42 ++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/limits.sgml b/doc/src/sgml/limits.sgml index d5b2b627ddf..c549447013f 100644 --- a/doc/src/sgml/limits.sgml +++ b/doc/src/sgml/limits.sgml @@ -57,14 +57,14 @@ columns per table - 1600 + 1,600 further limited by tuple size fitting on a single page; see note below columns in a result set - 1664 + 1,664 @@ -74,12 +74,6 @@ - - identifier length - 63 bytes - can be increased by recompiling PostgreSQL - - indexes per table unlimited @@ -92,11 +86,29 @@ can be increased by recompiling PostgreSQL - - partition keys - 32 - can be increased by recompiling PostgreSQL - + + partition keys + 32 + can be increased by recompiling PostgreSQL + + + + identifier length + 63 bytes + can be increased by recompiling PostgreSQL + + + + function arguments + 100 + can be increased by recompiling PostgreSQL + + + + query parameters + 65,535 + +
@@ -104,9 +116,9 @@ The maximum number of columns for a table is further reduced as the tuple being stored must fit in a single 8192-byte heap page. For example, - excluding the tuple header, a tuple made up of 1600 int columns + excluding the tuple header, a tuple made up of 1,600 int columns would consume 6400 bytes and could be stored in a heap page, but a tuple of - 1600 bigint columns would consume 12800 bytes and would + 1,600 bigint columns would consume 12800 bytes and would therefore not fit inside a heap page. Variable-length fields of types such as text, varchar, and char From 553df3d87cb8b96899cc1286429f9fbaa3652d92 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Tue, 31 Oct 2023 10:21:32 -0400 Subject: [PATCH 84/91] doc: improve ALTER SYSTEM description of value list quoting Reported-by: splarv@ya.ru Discussion: https://postgr.es/m/167105927893.1897.13227723035830709578@wrigleys.postgresql.org Backpatch-through: 11 --- doc/src/sgml/ref/alter_system.sgml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/ref/alter_system.sgml b/doc/src/sgml/ref/alter_system.sgml index 5e41f7f6444..ab8d1502a7c 100644 --- a/doc/src/sgml/ref/alter_system.sgml +++ b/doc/src/sgml/ref/alter_system.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation -ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' | DEFAULT } +ALTER SYSTEM SET configuration_parameter { TO | = } { value [, ...] | DEFAULT } ALTER SYSTEM RESET configuration_parameter ALTER SYSTEM RESET ALL @@ -82,9 +82,17 @@ ALTER SYSTEM RESET ALL New value of the parameter. Values can be specified as string constants, identifiers, numbers, or comma-separated lists of these, as appropriate for the particular parameter. + Values that are neither numbers nor valid identifiers must be quoted. DEFAULT can be written to specify removing the parameter and its value from postgresql.auto.conf. + + + For some list-accepting parameters, quoted values will produce + double-quoted output to preserve whitespace and commas; for others, + double-quotes must be used inside single-quoted strings to get + this effect. + From de649d14509facc7507da0fae6ddb1a435995d6a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 2 Nov 2023 07:33:02 +0900 Subject: [PATCH 85/91] doc: Replace reference to ERRCODE_RAISE_EXCEPTION by "raise_exception" This part of the documentation refers to exceptions as handled by PL/pgSQL, and using the internal error code is confusing. Per thinko in 66bde49d96a9. Reported-by: Euler Taveira, Bruce Momjian Discussion: https://postgr.es/m/ZUEUnLevXyW7DlCs@momjian.us Backpatch-through: 11 --- doc/src/sgml/plpgsql.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 8017ba392a4..aecf8ffd846 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3903,7 +3903,7 @@ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id; If no condition name nor SQLSTATE is specified in a RAISE EXCEPTION command, the default is to use - ERRCODE_RAISE_EXCEPTION (P0001). + raise_exception (P0001). If no message text is specified, the default is to use the condition name or SQLSTATE as message text. From dbc5ac3f5b91c28f57967bb29e3e7fe6500515ab Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Fri, 3 Nov 2023 09:51:53 -0400 Subject: [PATCH 86/91] doc: ALTER DEFAULT PRIVILEGES does not affect inherited roles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported-by: Jordi Gutiérrez Hermoso Discussion: https://postgr.es/m/72652d72e1816bfc3c05d40f9e0e0373d07823c8.camel@octave.org Co-authored-by: Laurenz Albe Backpatch-through: 11 --- doc/src/sgml/ref/alter_default_privileges.sgml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/ref/alter_default_privileges.sgml b/doc/src/sgml/ref/alter_default_privileges.sgml index f1d54f5aa35..8a6006188d3 100644 --- a/doc/src/sgml/ref/alter_default_privileges.sgml +++ b/doc/src/sgml/ref/alter_default_privileges.sgml @@ -137,7 +137,11 @@ REVOKE [ GRANT OPTION FOR ] The name of an existing role of which the current role is a member. - If FOR ROLE is omitted, the current role is assumed. + Default access privileges are not inherited, so member roles + must use SET ROLE to access these privileges, + or ALTER DEFAULT PRIVILEGES must be run for + each member role. If FOR ROLE is omitted, + the current role is assumed. From f66b22eb6034a15452f03b03357c1b7754532081 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 3 Nov 2023 11:48:23 -0400 Subject: [PATCH 87/91] Doc: update CREATE RULE ref page's hoary discussion of views. This text left one with the impression that an ON SELECT rule could be attached to a plain table, which has not been true since commit 264c06820 (meaning the text was already misleading when written, evidently by me in 96bd67f61). However, it didn't get really bad until b23cd185f removed the convert-a-table-to-a-view logic, which had made it possible for scripts that thought they were attaching ON SELECTs to tables to still work. Rewrite into a form that makes it clear that an ON SELECT rule is better regarded as an implementation detail of a view. Pre-v16, point out that adding ON SELECT to a table actually converts it to a view. Per bug #18178 from Joshua Uyehara. Back-patch to all supported branches. Discussion: https://postgr.es/m/18178-05534d7064044d2d@postgresql.org --- doc/src/sgml/ref/create_rule.sgml | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/ref/create_rule.sgml b/doc/src/sgml/ref/create_rule.sgml index dbf4c937841..76029abf515 100644 --- a/doc/src/sgml/ref/create_rule.sgml +++ b/doc/src/sgml/ref/create_rule.sgml @@ -59,15 +59,17 @@ CREATE [ OR REPLACE ] RULE name AS - Presently, ON SELECT rules must be unconditional - INSTEAD rules and must have actions that consist - of a single SELECT command. Thus, an - ON SELECT rule effectively turns the table into - a view, whose visible contents are the rows returned by the rule's - SELECT command rather than whatever had been - stored in the table (if anything). It is considered better style - to write a CREATE VIEW command than to create a - real table and define an ON SELECT rule for it. + Presently, ON SELECT rules can only be attached + to views. (Attaching one to a table converts the table into a view.) + Such a rule must be named "_RETURN", + must be an unconditional INSTEAD rule, and must have + an action that consists of a single SELECT command. + This command defines the visible contents of the view. (The view + itself is basically a dummy table with no storage.) It's best to + regard such a rule as an implementation detail. While a view can be + redefined via CREATE OR REPLACE RULE "_RETURN" AS + ..., it's better style to use CREATE OR REPLACE + VIEW. From 464e840a37595faf48281b3ce40423dc78f527b1 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 3 Nov 2023 11:50:25 -0400 Subject: [PATCH 88/91] pg_upgrade: Add missing newline to message This was the backport of 2e3dc8c148, but in older releases the newline must be in the message. --- src/bin/pg_upgrade/check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 80a8f251126..289a0d485e3 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -1583,7 +1583,7 @@ check_for_removed_data_type_usage(ClusterInfo *cluster, const char *version, if (check_for_data_type_usage(cluster, typename, output_path)) { - pg_log(PG_REPORT, "fatal"); + pg_log(PG_REPORT, "fatal\n"); pg_fatal("Your installation contains the \"%s\" data type in user tables.\n" "The \"%s\" type has been removed in PostgreSQL version %s,\n" "so this cluster cannot currently be upgraded. You can drop the\n" From c0555baa3310786e1ad347e2ead4ddee68b14edb Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Fri, 3 Nov 2023 13:39:50 -0400 Subject: [PATCH 89/91] doc: CREATE DATABASE doesn't copy db-level perms. from template Reported-by: david@kapitaltrading.com Discussion: https://postgr.es/m/166007719137.995877.13951579839074751714@wrigleys.postgresql.org Backpatch-through: 11 --- doc/src/sgml/manage-ag.sgml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/src/sgml/manage-ag.sgml b/doc/src/sgml/manage-ag.sgml index 74055a47065..116e5e88506 100644 --- a/doc/src/sgml/manage-ag.sgml +++ b/doc/src/sgml/manage-ag.sgml @@ -207,6 +207,12 @@ createdb -O rolename dbname + + However, CREATE DATABASE does not copy database-level + GRANT permissions attached to the source database. + The new database has default database-level permissions. + + There is a second standard system database named template0.template0 This From a4e64fe597c5ff48ea47c807a94eacb4e0eb6419 Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Fri, 3 Nov 2023 13:57:59 -0400 Subject: [PATCH 90/91] doc: \copy can get data values \. and end-of-input confused Reported-by: Svante Richter Discussion: https://postgr.es/m/fcd57e4-8f23-4c3e-a5db-2571d09208e2@beta.fastmail.com Backpatch-through: 11 --- doc/src/sgml/ref/psql-ref.sgml | 4 ++++ src/bin/psql/copy.c | 1 + 2 files changed, 5 insertions(+) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 74f138b0b0a..fb784fbc4dc 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1100,6 +1100,10 @@ testdb=> destination, because all data must pass through the client/server connection. For large amounts of data the SQL command might be preferable. + Also, because of this pass-through method, \copy + ... from in CSV mode will erroneously + treat a \. data value alone on a line as an + end-of-input marker. diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c index 1e83ad06ee2..9528bf7f583 100644 --- a/src/bin/psql/copy.c +++ b/src/bin/psql/copy.c @@ -712,6 +712,7 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res) * This code erroneously assumes '\.' on a line alone * inside a quoted CSV string terminates the \copy. * https://www.postgresql.org/message-id/E1TdNVQ-0001ju-GO@wrigleys.postgresql.org + * https://www.postgresql.org/message-id/bfcd57e4-8f23-4c3e-a5db-2571d09208e2@beta.fastmail.com */ if (strcmp(buf, "\\.\n") == 0 || strcmp(buf, "\\.\r\n") == 0) From 0c90126afb62ae1675bc56832a12a6bfae61f78f Mon Sep 17 00:00:00 2001 From: reshke Date: Sat, 19 Sep 2026 21:04:45 +0300 Subject: [PATCH 91/91] Remove duplicate check_for_removed_data_type_usage in pg_upgrade/check.c Cherry-picked commit c7f84cf84a2 introduced a second copy of check_for_removed_data_type_usage() while the first one already exists in REL_2_STABLE. Remove the duplicate. --- src/bin/pg_upgrade/check.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 289a0d485e3..ae364b76ef3 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -1561,40 +1561,6 @@ check_for_removed_data_type_usage(ClusterInfo *cluster, const char *version, check_ok(); } -/* - * check_for_removed_data_type_usage - * - * Check for in-core data types that have been removed. Callers know - * the exact list. - */ -static void -check_for_removed_data_type_usage(ClusterInfo *cluster, const char *version, - const char *datatype) -{ - char output_path[MAXPGPATH]; - char typename[NAMEDATALEN]; - - prep_status("Checking for removed \"%s\" data type in user tables", - datatype); - - snprintf(output_path, sizeof(output_path), "tables_using_%s.txt", - datatype); - snprintf(typename, sizeof(typename), "pg_catalog.%s", datatype); - - if (check_for_data_type_usage(cluster, typename, output_path)) - { - pg_log(PG_REPORT, "fatal\n"); - pg_fatal("Your installation contains the \"%s\" data type in user tables.\n" - "The \"%s\" type has been removed in PostgreSQL version %s,\n" - "so this cluster cannot currently be upgraded. You can drop the\n" - "problem columns, or change them to another data type, and restart\n" - "the upgrade. A list of the problem columns is in the file:\n" - " %s\n\n", datatype, datatype, version, output_path); - } - else - check_ok(); -} - /* * check_for_jsonb_9_4_usage()