postgresql.git
13 months agoDisable run condition optimization for some WindowFuncs
David Rowley [Wed, 1 May 2024 04:35:05 +0000 (16:35 +1200)]
Disable run condition optimization for some WindowFuncs

94985c210 added code to detect when WindowFuncs were monotonic and
allowed additional quals to be "pushed down" into the subquery to be
used as WindowClause runConditions in order to short-circuit execution
in nodeWindowAgg.c.

The Node representation of runConditions wasn't well selected and
because we do qual pushdown before planning the subquery, the planning
of the subquery could perform subquery pull-up of nested subqueries.
For WindowFuncs with args, the arguments could be changed after pushing
the qual down to the subquery.

This was made more difficult by the fact that the code duplicated the
WindowFunc inside an OpExpr to include in the WindowClauses runCondition
field.  This could result in duplication of subqueries and a pull-up of
such a subquery could result in another initplan parameter being issued
for the 2nd version of the subplan.  This could result in errors such as:

ERROR:  WindowFunc not found in subplan target lists

Here in the backbranches, we don't have the flexibility to improve the
Node representation to resolve this, so instead we just disable the
runCondition optimization for ntile() unless the argument is a Const,
(v16 only) and likewise for count(expr) (both v15 and v16).  count(*) is
unaffected.  All other window functions which support this optimization
all take zero arguments and therefore are unaffected.

Bug: #18170
Reported-by: Zuming Jiang
Discussion: https://postgr.es/m/18170-f1d17bf9a0d58b24@postgresql.org
Backpatch-through 15 (master will be fixed independently)

13 months agoFix parallel vacuum buffer usage reporting.
Masahiko Sawada [Wed, 1 May 2024 03:34:04 +0000 (12:34 +0900)]
Fix parallel vacuum buffer usage reporting.

A parallel worker's buffer usage is accumulated to its pgBufferUsage
and then is accumulated into the leader's one at the end of the
parallel vacuum. However, since the leader process used to use
dedicated VacuumPage{Hit, Miss, Dirty} globals for the buffer usage
reporting, the worker's buffer usage was not included, leading to an
incorrect buffer usage report.

To fix the problem, this commit makes vacuum use pgBufferUsage
instruments for buffer usage reporting instead of VacuumPage{Hit,
Miss, Dirty} globals. These global variables are still used by ANALYZE
command and autoanalyze.

This also fixes the buffer usage report of vacuuming on temporary
tables, since the buffers dirtied by MarkLocalBufferDirty() were not
tracked by the VacuumPageDirty variable.

Parallel vacuum was introduced in 13, but the buffer usage reporting
for VACUUM command with the VERBOSE option was implemented in
15. So backpatch to 15.

Reported-by: Anthonin Bonnefoy
Author: Anthonin Bonnefoy
Reviewed-by: Alena Rybakina, Masahiko Sawada
Discussion: https://postgr.es/m/CAO6_XqrQk+QZQcYs_C6nk0cMfHuUWk85vT9CrcA1NffFbAVE2A@mail.gmail.com
Backpatch-through: 15

13 months agoEnsure we allocate NAMEDATALEN bytes for names in Index Only Scans
David Rowley [Wed, 1 May 2024 01:21:50 +0000 (13:21 +1200)]
Ensure we allocate NAMEDATALEN bytes for names in Index Only Scans

As an optimization, we store "name" columns as cstrings in btree
indexes.

Here we modify it so that Index Only Scans convert these cstrings back
to names with NAMEDATALEN bytes rather than storing the cstring in the
tuple slot, as was happening previously.

Bug: #17855
Reported-by: Alexander Lakhin
Reviewed-by: Alexander Lakhin, Tom Lane
Discussion: https://postgr.es/m/17855-5f523e0f9769a566@postgresql.org
Backpatch-through: 12, all supported versions

13 months agoClose race condition between datfrozen and relfrozen updates.
Noah Misch [Mon, 29 Apr 2024 17:24:56 +0000 (10:24 -0700)]
Close race condition between datfrozen and relfrozen updates.

vac_update_datfrozenxid() did multiple loads of relfrozenxid and
relminmxid from buffer memory, and it assumed each would get the same
value.  Not so if a concurrent vac_update_relstats() did an inplace
update.  Commit 2d2e40e3befd8b9e0d2757554537345b15fa6ea2 fixed the same
kind of bug in vac_truncate_clog().  Today's bug could cause the
rel-level field and XIDs in the rel's rows to precede the db-level
field.  A cluster having such values should VACUUM affected tables.
Back-patch to v12 (all supported versions).

Discussion: https://postgr.es/m/20240423003956[email protected]

13 months agoThrow a more on-point error for functions depending on columns.
Tom Lane [Sun, 28 Apr 2024 18:34:21 +0000 (14:34 -0400)]
Throw a more on-point error for functions depending on columns.

ALTER COLUMN TYPE wasn't expecting to find any pg_proc objects
depending on the column whose type is to be altered.  That indeed
wasn't possible when this code was written, but it is possible
since we introduced new-style SQL function bodies.

It's about as difficult to fix this case as it is to fix dependent
views, and we've been punting on those for years, so I don't feel
too awful about punting for functions too.  (I sure wouldn't risk
back-patching such code.)  So just throw a more user-facing error.
Also, adjust some of the existing comments to reflect that these
are all pretty much the same issue.

(This patch also fixes it so we will tolerate finding such a
dependency during ALTER COLUMN SET EXPRESSION; in that, we need
not do anything to the function, so no error is wanted.  That
problem is new in HEAD.)

Per bug #18449 from Alexander Lakhin.  Back-patch to v14 where
we added new-style SQL functions.

Discussion: https://postgr.es/m/18449-f8248467aaa294d5@postgresql.org

13 months agoDetect more overflows in timestamp[tz]_pl_interval.
Tom Lane [Sun, 28 Apr 2024 17:42:13 +0000 (13:42 -0400)]
Detect more overflows in timestamp[tz]_pl_interval.

In commit 25cd2d640 I (tgl) opined that "The additions of the months
and microseconds fields could also overflow, of course.  However,
I believe we need no additional checks there; the existing range
checks should catch such cases".  This is demonstrably wrong however
for the microseconds field, and given that discovery it seems prudent
to be paranoid about the months addition as well.

Report and patch by Joseph Koshakow.  As before, back-patch to all
supported branches.  (However, the test case doesn't work before
v15 because we didn't allow wider-than-int32 numbers in interval
literals.  A variant test could probably be built that fits within
that restriction, but it didn't seem worth the trouble.)

Discussion: https://postgr.es/m/CAAvxfHf77sRHKoEzUw9_cMYSpbpNS2C+J_+8Dq4+0oi8iKopeA@mail.gmail.com

13 months agoFix make headerscheck
John Naylor [Sat, 27 Apr 2024 04:38:41 +0000 (11:38 +0700)]
Fix make headerscheck

In the wake of commits dac048f71 and ecaf7c5df, `make headerscheck`
no longer generated all headers that are included by other headers,
causing headerscheck/cpluspluscheck to fail. To fix, backpatch enough
makefile rules from 721856ff2 to generate all required headers.

Reported by Marina Polyakova
Backpatch to version 16 only, as the issue is not present on master

Discussion: https://postgr.es/m/231ea1127719b2b3d6d1c05f75808981%40postgrespro.ru

13 months agoAvoid unnecessary "touch meson.build" in vpath builds
Andres Freund [Thu, 25 Apr 2024 14:51:33 +0000 (07:51 -0700)]
Avoid unnecessary "touch meson.build" in vpath builds

In e6927270cd1 I added a 'touch meson.build' to configure.ac, to ensure
conflicts between in-tree configure based builds and meson builds are
automatically detected. Unfortunately I omitted spaces around the condition
restricting this to in-tree builds, leading to touch meson.build to also be
executed in vpath builds. While the only consequence of this buglet is an
unnecessary empty file in build directories, it seems worth backpatching.

Reported-by: Christoph Berg
Discussion: https://postgr.es/m/20240417230002[email protected]
Backpatch: 16-, where the meson based build was added

13 months agoFix the missing table sync due to improper invalidation handling.
Amit Kapila [Thu, 25 Apr 2024 05:22:34 +0000 (10:52 +0530)]
Fix the missing table sync due to improper invalidation handling.

We missed performing table sync if the invalidation happened while the
non-ready tables list was being prepared. This occurs because the sync
state was set to valid at the end of non-ready table list preparation
irrespective of the invalidations processed while the list is being
prepared.

Fix it by changing the boolean variable to a tri-state enum and by setting
table state to valid only if no invalidations have occurred while the list
is being prepared.

Reprted-by: Alexander Lakhin
Diagnosed-by: Alexander Lakhin
Author: Vignesh C
Reviewed-by: Hou Zhijie, Alexander Lakhin, Ajin Cherian, Amit Kapila
Backpatch-through: 15
Discussion: https://postgr.es/m/711a6afe-edb7-1211-cc27-1bef8239eec7@gmail.com

13 months agoDoc: fix minor oversight in ALTER DEFAULT PRIVILEGES ref page.
Tom Lane [Wed, 24 Apr 2024 14:18:16 +0000 (10:18 -0400)]
Doc: fix minor oversight in ALTER DEFAULT PRIVILEGES ref page.

Since schemas have more than one kind of privilege, we should
use the synopsis form that shows the privilege being possibly
repeated.

Yugo Nagata

Discussion: https://postgr.es/m/20240424155052.7ac0d0773e4ae27ab723faea@sraoss.co.jp

13 months agodoc: Correct jsonpath string literal escapes description
Peter Eisentraut [Wed, 24 Apr 2024 09:31:47 +0000 (11:31 +0200)]
doc: Correct jsonpath string literal escapes description

The paragraph describing the JavaScript string literals allowed in
jsonpath expressions unnecessarily mentions JSON by erroneously
listing \v as allowed by JSON and mentioning the \xNN and \u{N...}
backslash escapes as deviations from JSON when in fact both are
accepted by ECMAScript/JavaScript.  Fix this by only referring to
JavaScript.

Author: Erik Wienhold 
Discussion: https://www.postgresql.org/message-id/flat/1EB17DF9-2636-484B-9DD0-3CAB19C4F5C4@justatheory.com

13 months agocreatedb: compare strategy case-insensitive
Tomas Vondra [Sun, 21 Apr 2024 19:21:55 +0000 (21:21 +0200)]
createdb: compare strategy case-insensitive

When specifying the createdb strategy, the documentation suggests valid
options are FILE_COPY and WAL_LOG, but the code does case-sensitive
comparison and accepts only "file_copy" and "wal_log" as valid.

Fixed by doing a case-insensitive comparison using pg_strcasecmp(), same
as for other string parameters nearby.

While at it, apply fmtId() to a nearby "locale_provider". This already
did the comparison in case-insensitive way, but the value would not be
double-quoted, confusing the parser and the error message.

Backpatch to 15, where the strategy was introduced.

Backpatch-through: 15
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/90c6913a-1dd2-42b4-8365-ce3b09c39b17@enterprisedb.com

13 months agoMake postgres_fdw request remote time zone 'GMT' not 'UTC'.
Tom Lane [Sun, 21 Apr 2024 17:46:20 +0000 (13:46 -0400)]
Make postgres_fdw request remote time zone 'GMT' not 'UTC'.

This should have the same results for all practical purposes.
The advantage of selecting 'GMT' is that it's guaranteed to work
even when the remote system's timezone database is missing
entries, because pg_tzset() hard-wires handling of that,
at least in 9.2 and later.

(It seems like it would be a good idea to similarly hard-wire
correct handling of 'UTC', but that'll be a little more invasive
than I want to consider back-patching.  Leave that for another
day when we're not in feature freeze.)

Per trouble report from Adnan Dautovic.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/465248.1712211585@sss.pgh.pa.us

13 months agocreatedb: Correct parameter name in SGML docs
Tomas Vondra [Sat, 20 Apr 2024 16:54:09 +0000 (18:54 +0200)]
createdb: Correct parameter name in SGML docs

Commit 9c08aea6a309 introduced -S/--strategy option, but forgot to
rename the parameter when copying the -T/--template bit.

13 months agoDoc: document cases where queryid is stable
David Rowley [Sat, 20 Apr 2024 01:54:24 +0000 (13:54 +1200)]
Doc: document cases where queryid is stable

The documents were clear that queryid should not be assumed to be stable
between major versions but said nothing about minor versions and left
the reader to guess if that was implied by the mention of the
instability of queryid between major versions.

Here we give minor versions an explicit mention to indicate queryid can
generally be assumed stable between minor versions.

Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAApHDvpYGE6h0cD9UO-eHySPynPj1L3J%3DHxT%2BA7Ud8_Yo6AuzA%40mail.gmail.com
Backpatch-through: 12

13 months agoDoc: Remove mention of @ and ~ GiST operators
Daniel Gustafsson [Fri, 19 Apr 2024 12:50:10 +0000 (14:50 +0200)]
Doc: Remove mention of @ and ~ GiST operators

These operators were removed by 2f70fdb0644c in the v14 cycle but they were
accidentally left in the table of build-in operator classes. Backpatch down
to v14 where the operators where removed.

Author: Aleksander Alekseev 
Reported-by: Colin Caine
Discussion: https://postgr.es/m/CADwQTQbbr2UQ_fpbyc+8ay=RwEYgYk=TZxH3+RHDqAQfoG+EWA@mail.gmail.com
Backpatch-through: v14

13 months agoFix MSVC recipe for ecpg regression tests, redux.
Tom Lane [Fri, 19 Apr 2024 05:07:16 +0000 (01:07 -0400)]
Fix MSVC recipe for ecpg regression tests, redux.

Forgot to inject -DCMDLINESYM=123 ...

Per buildfarm.

Discussion: https://postgr.es/m/4cc4dc47-ca2b-4129-8784-db69b5f82777@dunslane.net

13 months agoFix MSVC recipe for ecpg regression tests.
Tom Lane [Fri, 19 Apr 2024 00:47:37 +0000 (20:47 -0400)]
Fix MSVC recipe for ecpg regression tests.

While back-patching commit 6f0cef935, I forgot that the MSVC
build scripts would also need adjustment in the back branches.
This is a blind attempt at a fix, but it's basically copying
nearby code so I think it will work.

Per buildfarm (via Andrew Dunstan)

Discussion: https://postgr.es/m/4cc4dc47-ca2b-4129-8784-db69b5f82777@dunslane.net

14 months agoFix assorted bugs in ecpg's macro mechanism.
Tom Lane [Tue, 16 Apr 2024 16:31:32 +0000 (12:31 -0400)]
Fix assorted bugs in ecpg's macro mechanism.

The code associated with EXEC SQL DEFINE was unreadable and full of
bugs, notably:

* It'd attempt to free a non-malloced string if the ecpg program
tries to redefine a macro that was defined on the command line.

* Possible memory stomp if user writes "-D=foo".

* Undef'ing or redefining a macro defined on the command line would
change the state visible to the next file, when multiple files are
specified on the command line.  (While possibly that could have been
an intentional choice, the code clearly intends to revert to the
original macro state; it's just failing to consider this interaction.)

* Missing "break" in defining a new macro meant that redefinition
of an existing name would cause an extra entry to be added to the
definition list.  While not immediately harmful, a subsequent undef
would result in the prior entry becoming visible again.

* The interactions with input buffering are subtle and were entirely
undocumented.

It's not that surprising that we hadn't noticed these bugs,
because there was no test coverage at all of either the -D
command line switch or multiple input files.  This patch adds
such coverage (in a rather hacky way I guess).

In addition to the code bugs, the user documentation was confused
about whether the -D switch defines a C macro or an ecpg one, and
it failed to mention that you can write "-Dsymbol=value".

These problems are old, so back-patch to all supported branches.

Discussion: https://postgr.es/m/998011.1713217712@sss.pgh.pa.us

14 months agoEnsure generated join clauses for child rels have correct relids.
Tom Lane [Tue, 16 Apr 2024 15:03:43 +0000 (11:03 -0400)]
Ensure generated join clauses for child rels have correct relids.

When building a join clause derived from an EquivalenceClass, if the
clause is to be used with an appendrel child relation then make sure
its clause_relids include the relids of that child relation.
Normally this would be true already because the EquivalenceMember
would be a Var of that relation.  However, if the appendrel represents
a flattened UNION ALL construct then some child EquivalenceMembers
could be constants with no relids.  The resulting under-marked clause
is problematic because it could mislead join_clause_is_movable_into
about where the clause should be evaluated.  We do not have an example
showing incorrect plan generation, but there are existing cases in
the regression tests that will fail the Asserts this patch adds to
get_baserel_parampathinfo.  A similarly wrong conclusion about a
clause being considered by get_joinrel_parampathinfo would lead to
wrong placement of the clause.  (This also squares with the way
that clause_relids is calculated for non-equijoin clauses in
adjust_appendrel_attrs.)

The other reason for wanting these new Asserts is that the previous
blithe assumption that the results of generate_join_implied_equalities
"necessarily satisfy join_clause_is_movable_into" turns out to be
wrong pre-v16.  If it's still wrong it'd be good to find out.

Per bug #18429 from Benoît Ryder.  The bug as filed was fixed by
commit 2489d76c4, but these changes correlate with the fix we
will need to apply in pre-v16 branches.

Discussion: https://postgr.es/m/18429-8982d4a348cc86c6@postgresql.org

14 months agoxml2: Replace deprecated routines with recommended ones
Michael Paquier [Tue, 16 Apr 2024 03:25:48 +0000 (12:25 +0900)]
xml2: Replace deprecated routines with recommended ones

Some functions are used in the tree and are currently marked as
deprecated by upstream.  This commit refreshes the code to use the
recommended functions, leading to the following changes:
- xmlSubstituteEntitiesDefault() is gone, and needs to be replaced with
XML_PARSE_NOENT for the paths doing the parsing.
- xmlParseMemory() -> xmlReadMemory().

These functions, as well as more functions setting global states, have
been officially marked as deprecated by upstream in August 2022.  Their
replacements exist since the 2001-ish area, as far as I have checked,
so that should be safe.

This has been originally applied as 65c5864d7fac without a backpatch,
and this has come up as well when working on 400928b83.  Per request
from Tom Lane, for new buildfarm member indri that is able to see
deprecation warnings with xmlSubstituteEntitiesDefault() in 16 and older
stable branches.

Author: Dmitry Koval
Discussion: https://postgr.es/m/18274-98d16bc03520665f@postgresql.org
Discussion: https://postgr.es/m/1012981.1713222862@sss.pgh.pa.us
Bakpatch-through: 12

14 months agoFix type-checking of RECORD-returning functions in FROM, redux.
Tom Lane [Mon, 15 Apr 2024 16:56:56 +0000 (12:56 -0400)]
Fix type-checking of RECORD-returning functions in FROM, redux.

Commit 2ed8f9a01 intended to institute a policy that if a
RangeTblFunction has a coldeflist, then the function return type is
certainly RECORD, and we should use the coldeflist as the source of
truth about what the columns of the record type are.  When the
original function has been folded to a constant, inspection of the
constant might give a different answer.  This situation will lead to
a tuple-type-mismatch error at execution, but up until that point we
need to consistently believe the coldeflist, or we'll have problems
from different bits of code reaching different conclusions.

expandRTE didn't get that memo though, and would try to produce a
tupdesc based on the constant in this situation, leading to an
assertion failure.  (Desultory testing suggests that non-assert
builds often manage to give the expected error, although I also
saw a "cache lookup failed for type 0" error, and it seems at
least possible that a crash could happen.)

Some other callers of get_expr_result_type and get_expr_result_tupdesc
were also being incautious about this.  While none of them seem to
have actual bugs, they're working harder than necessary in this case,
besides which it seems safest to have an explicit policy of not using
those functions on an RTE with a coldeflist.  Adjust the code
accordingly, and add commentary to funcapi.c about this policy.

Also fix an obsolete comment that claimed "get_expr_result_type()
doesn't know how to extract type info from a RECORD constant".
That hasn't been true since commit d57534740.

Per bug #18422 from Alexander Lakhin.
As with the previous commit, back-patch to all supported branches.

Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org

14 months agoUse the correct PG_DETOAST_DATUM macro in BRIN
Tomas Vondra [Sun, 14 Apr 2024 16:19:52 +0000 (18:19 +0200)]
Use the correct PG_DETOAST_DATUM macro in BRIN

Commit 6bcda4a721 replaced PG_DETOAST_DATUM with PG_DETOAST_DATUM_PACKED
in two BRIN output functions, for minmax-multi and bloom opclasses. But
this is incorrect - the code is accessing the data through structs that
already include a 4B header, so the detoast needs to match that. But the
PACKED macro may keep the 1B header, which means the struct fields will
point to incorrect data.

Backpatch-through: 16
Discussion: https://postgr.es/m/1df00a66-db5a-4e66-809a-99b386a06d86%40enterprisedb.com

14 months agoUpdate nbits_set in brin_bloom_union
Tomas Vondra [Sun, 14 Apr 2024 15:58:59 +0000 (17:58 +0200)]
Update nbits_set in brin_bloom_union

Properly update the number of bits set in the bitmap after merging the
filters in brin_bloom_union.

This is mostly harmless, as the counter is used only in the output
function, which means pageinspect may show incorrect information about
the BRIN summary. The counter does not affect correctness.

Discovered while adding a regression test comparing indexes built with
and without parallelism. The parallel index builds exercise the union
procedure when merging results from workers, which is otherwise very
hard to do in a test. Which is why this went unnoticed until now.

Backpatch through 14, where the BRIN bloom opclasses were introduced.

Backpatch-through: 14
Discussion: https://postgr.es/m/1df00a66-db5a-4e66-809a-99b386a06d86%40enterprisedb.com

14 months agofreespace: Don't return blocks past the end of the main fork.
Noah Misch [Sat, 13 Apr 2024 15:34:20 +0000 (08:34 -0700)]
freespace: Don't return blocks past the end of the main fork.

GetPageWithFreeSpace() callers assume the returned block exists in the
main fork, failing with "could not read block" errors if that doesn't
hold.  Make that assumption reliable now.  It hadn't been guaranteed,
due to the weak WAL and data ordering of participating components.  Most
operations on the fsm fork are not WAL-logged.  Relation extension is
not WAL-logged.  Hence, an fsm-fork block on disk can reference a
main-fork block that no WAL record has initialized.  That could happen
after an OS crash, a replica promote, or a PITR restore.  wal_log_hints
makes the trouble easier to hit; a replica promote or PITR ending just
after a relevant fsm-fork FPI_FOR_HINT may yield this broken state.  The
v16 RelationAddBlocks() mechanism also makes the trouble easier to hit,
since it bulk-extends even without extension lock waiters.  Commit
917dc7d2393ce680dea7a59418be9ff341df3c14 stopped trouble around
truncation, but vectors involving PageIsNew() pages remained.

This implementation adds a RelationGetNumberOfBlocks() call when the
cached relation size doesn't confirm a block exists.  We've been unable
to identify a benchmark that slows materially, but this may show up as
additional time in lseek().  An alternative without that overhead would
be a new ReadBufferMode such that ReadBufferExtended() returns NULL
after a 0-byte read, with all other errors handled normally.  However,
each GetFreeIndexPage() caller would then need code for the return-NULL
case.  Back-patch to v14, due to earlier versions not caching relation
size and the absence of a pre-v16 problem report.

Ronan Dunklau.  Reported by Ronan Dunklau.

Discussion: https://postgr.es/m/1878547.tdWV9SEqCh%40aivenlaptop

14 months agoCorrect "improve role option documentation".
Noah Misch [Sat, 13 Apr 2024 14:56:14 +0000 (07:56 -0700)]
Correct "improve role option documentation".

This corrects doc commit 21912e3c0262e2cfe64856e028799d6927862563.
Back-patch to v16, like that one.

Reviewed by David G. Johnston.

Discussion: https://postgr.es/m/20240331061642[email protected]

14 months agoDoc: fix bogus to_date() examples.
Tom Lane [Thu, 11 Apr 2024 15:09:00 +0000 (11:09 -0400)]
Doc: fix bogus to_date() examples.

November doesn't have 31 days.  Remarkably, this thinko
has escaped detection since commit 3f1998727.

Noted by Y. Saburov.

Discussion: https://postgr.es/m/171276122213.681.531905738590773705@wrigleys.postgresql.org

14 months agomeson: Remove obsolete function test
Peter Eisentraut [Thu, 11 Apr 2024 10:44:54 +0000 (12:44 +0200)]
meson: Remove obsolete function test

The test for pstat was removed from configure by 9db300ce6e3 but not
from meson.build.  Do that now.

14 months agoFix WaitEventSet resource leak in WaitLatchOrSocket().
Etsuro Fujita [Thu, 11 Apr 2024 10:05:00 +0000 (19:05 +0900)]
Fix WaitEventSet resource leak in WaitLatchOrSocket().

This function would have the same issue we solved in commit 501cfd07d:
If an error is thrown after calling CreateWaitEventSet(), the file
descriptor (on epoll- or kqueue-based systems) or handles (on Windows)
that the WaitEventSet contains are leaked.

Like that commit, use PG_TRY-PG_FINALLY (PG_TRY-PG_CATCH in v12) to make
sure the WaitEventSet is freed properly.

Back-patch to all supported versions, but as we do not have this issue
in HEAD (cf. commit 50c67c201), no need to apply this patch to it.

Discussion: https://postgr.es/m/CAPmGK16MqdDoD8oatp8SQWaEa4vS3nfQqDN_Sj9YRuu5J3Lj9g%40mail.gmail.com

14 months agoUse correct datatype for xmin variables in slot.c
Michael Paquier [Thu, 11 Apr 2024 08:19:35 +0000 (17:19 +0900)]
Use correct datatype for xmin variables in slot.c

Two variables storing a slot's effective_xmin and effective_catalog_xmin
were saved as XLogRecPtr, which is incorrect as these should be
TransactionIds.

Oversight in 818fefd8fd44.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVPSB74mrDTFezz-LV3Oi6F3SN71QA0oUHvndzi5dwTNg@mail.gmail.com
Backpatch-through: 16

14 months agoFix plpgsql's handling of -- comments following expressions.
Tom Lane [Wed, 10 Apr 2024 19:45:58 +0000 (15:45 -0400)]
Fix plpgsql's handling of -- comments following expressions.

Up to now, read_sql_construct() has collected all the source text from
the statement or expression's initial token up to the character just
before the "until" token.  It normally tries to strip trailing
whitespace from that, largely for neatness.  If there was a "-- text"
comment after the expression, this resulted in removing the newline
that terminates the comment, which creates a hazard if we try to paste
the collected text into a larger SQL construct without inserting a
newline after it.  In particular this caused our handling of CASE
constructs to fail if there's a comment after a WHEN expression.

Commit 4adead1d2 noticed a similar problem with cursor arguments,
and worked around it through the rather crude hack of suppressing
the whitespace-trimming behavior for those.  Rather than do that
and leave the hazard open for future hackers to trip over, let's
fix it properly.  pl_scanner.c already has enough infrastructure
to report the end location of the expression's last token, so
we can copy up to that location and never collect any trailing
whitespace or comment to begin with.

Erik Wienhold and Tom Lane, per report from Michal Bartak.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAAVzF_FjRoi8fOVuLCZhQJx6HATQ7MKm=aFOHWZODFnLmjX-xA@mail.gmail.com

14 months agoDoc: Update ulinks to RFC documents to avoid redirect
Daniel Gustafsson [Wed, 10 Apr 2024 11:53:25 +0000 (13:53 +0200)]
Doc: Update ulinks to RFC documents to avoid redirect

The tools.ietf.org site has been decommissioned and replaced by a
number of sites serving various purposes.  Links to RFCs and BCPs
are now 301 redirected to their new respective IETF sites.  Since
this serves no purpose and only adds network overhead, update our
links to the new locations.

Backpatch to all supported versions.

Discussion: https://postgr.es/m/3C1CEA99-FCED-447D-9858-5A579B4C6687@yesql.se
Backpatch-through: v12

14 months agoFix illegal attribute propagation in LLVM JIT.
Thomas Munro [Tue, 9 Apr 2024 22:46:15 +0000 (10:46 +1200)]
Fix illegal attribute propagation in LLVM JIT.

Commit 72559438 started copying more attributes from AttributeTemplate
to the functions we generate on the fly.  In the case of deform
functions, which return void, this meant that "noundef", from
AttributeTemplate's return value (a Datum) was copied to a void type.
Older LLVM releases were OK with that, but LLVM 18 crashes.

Update our llvm_copy_attributes() function to skip copying the attribute
for the return value, if the target function returns void.

Thanks to Dmitry Dolgov for help chasing this down.

Back-patch to all supported releases, like 72559438.

Reported-by: Pavel Stehule
Reviewed-by: Dmitry Dolgov <[email protected]>
Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com

14 months agodoc: Remove stray comma from list of psql options
Daniel Gustafsson [Tue, 9 Apr 2024 21:39:38 +0000 (23:39 +0200)]
doc: Remove stray comma from list of psql options

Back in 7.2 the list of options had short options and long options
on the same line separated by comma, but since 7.3 they are listed
separate lines. The comma on -X was left behind so fix by removing
and backpatching all the way.

Reported-by: [email protected]
Discussion: https://postgr.es/m/171267154345.684.7212826057932148541@wrigleys.postgresql.org
Backpatch-through: v12

14 months agoIn psql, avoid leaking a PGresult after a query is cancelled.
Tom Lane [Mon, 8 Apr 2024 21:00:07 +0000 (17:00 -0400)]
In psql, avoid leaking a PGresult after a query is cancelled.

After a query cancel, the tail end of ExecQueryAndProcessResults
took care to clear any not-yet-read PGresults; but it forgot about
the one it has already read.  There would only be such a result
when handling a multi-command string made with "\;", so that you'd
have to cancel an earlier command in such a string to reach the
bug at all.  Even then, there would only be leakage of a single
PGresult per cancel, so it's not surprising nobody noticed this.
But a leak is a leak.

Noted while re-reviewing 90f517821, but this is independent of that:
it dates to 7844c9918.  Back-patch to v15 where that came in.

14 months agosimplehash: Free collisions array in SH_STAT
Andres Freund [Mon, 8 Apr 2024 02:00:11 +0000 (19:00 -0700)]
simplehash: Free collisions array in SH_STAT

While SH_STAT() is only used for debugging, the allocated array can be large,
and therefore should be freed.

It's unclear why coverity started warning now.

Reported-by: Tom Lane
Reported-by: Coverity
Discussion: https://postgr.es/m/3005248.1712538233@sss.pgh.pa.us
Backpatch: 12-

14 months agoDoc: update documentation about EXCLUDE constraint elements.
Tom Lane [Sun, 7 Apr 2024 19:36:08 +0000 (15:36 -0400)]
Doc: update documentation about EXCLUDE constraint elements.

What the documentation calls an exclude_element is an index_elem
according to gram.y, and it allows all the same options that
a CREATE INDEX column specification does.  The COLLATE patch
neglected to update the CREATE/ALTER TABLE docs about that,
and later the opclass-parameters patch made the same oversight.
Add those options to the syntax synopses, and polish the
associated text a bit.

Back-patch to v13 where opclass parameters came in.  We could
update v12 with just the COLLATE omission, but it doesn't quite
seem worth the trouble at this point.

Shihao Zhong, reviewed by Daniel Vérité, Shubham Khanna and myself

Discussion: https://postgr.es/m/CAGRkXqShbVyB8E3gapfdtuwiWTiK=Q67Qb9qwxu=+-w0w46EBA@mail.gmail.com

14 months agoDon't clobber test exit code at cleanup in LDAP/Kerberors tests
Heikki Linnakangas [Sun, 7 Apr 2024 17:21:27 +0000 (20:21 +0300)]
Don't clobber test exit code at cleanup in LDAP/Kerberors tests

If the test script die()d before running the first test, the whole test
was interpreted as SKIPped rather than failed. The PostgreSQL::Cluster
module got this right.

Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/fb898a70-3a88-4629-88e9-f2375020061d@iki.fi

14 months agoImprove check in LDAP test to find the OpenLDAP installation
Heikki Linnakangas [Sun, 7 Apr 2024 17:21:21 +0000 (20:21 +0300)]
Improve check in LDAP test to find the OpenLDAP installation

If the OpenLDAP installation directory is not found, set $setup to 0
so that the LDAP tests are skipped. The macOS checks were already
doing that, but the checks on other OS's were not. While we're at it,
improve the error message when the tests are skipped, to specify
whether the OS is supported at all, or if we just didn't find the
installation directory.

This was accidentally "working" without this, i.e. we were skipping
the tests if the OpenLDAP installation was not found, because of a bug
in the LdapServer test module: the END block clobbered the exit code
so if the script die()s before running the first subtest, the whole
test script was marked as SKIPped. The next commit will fix that bug,
but we need to fix the setup code first.

These checks should probably go into configure/meson, but this is
better than nothing and allows fixing the bug in the END block.

Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/fb898a70-3a88-4629-88e9-f2375020061d@iki.fi

14 months agoFix ecpg's mechanism for detecting unsupported cases in the grammar.
Tom Lane [Thu, 4 Apr 2024 19:31:53 +0000 (15:31 -0400)]
Fix ecpg's mechanism for detecting unsupported cases in the grammar.

ecpg wants to emit a warning if it parses a SQL construct that the
backend can parse but will immediately throw a FEATURE_NOT_SUPPORTED
error for.  The way it was testing for this was to see if the string
ERRCODE_FEATURE_NOT_SUPPORTED appeared anywhere in the gram.y code.
This is, of course, not nearly good enough, as there are plenty of
rules in gram.y that throw that error only conditionally.  There was
a hack dating to 2008 to suppress the warning in one rule that
doesn't even exist anymore, but nothing for other cases we've created
since then.  End result was that you could get "unsupported feature
will be passed to server" warnings while compiling perfectly good SQL
code in ecpg.  Somehow we'd not heard complaints about this, but
it was exposed by the recent addition of an ecpg test for a SQL/JSON
construct.

To fix, suppress the warning if the rule contains any "if" statement.
Manual comparison of gram.y with the generated preproc.y file shows
that the warning is now emitted only in rules where it's sensible.

This problem has existed for a long time, so back-patch to all
supported branches.

Discussion: https://postgr.es/m/603615.1712245382@sss.pgh.pa.us

14 months agoFix bogus coding in ExecAppendAsyncEventWait().
Etsuro Fujita [Thu, 4 Apr 2024 08:25:00 +0000 (17:25 +0900)]
Fix bogus coding in ExecAppendAsyncEventWait().

No configured-by-FDW events would result in "return" directly out of a
PG_TRY block, making the exception stack dangling.  Repair.

Oversight in commit 501cfd07d; back-patch to v14, like that commit, but
as we do not have this issue in HEAD (cf. commit 50c67c201), no need to
apply this patch to it.

In passing, improve a comment about the handling of in-process requests
in a postgres_fdw.c function called from this function.

Alexander Pyhalov, with comment adjustment/improvement by me.

Discussion: https://postgr.es/m/425fa29a429b21b0332737c42a4fdc70%40postgrespro.ru

14 months agoFix the parameters order for TableAmRoutine.relation_copy_for_cluster()
Alexander Korotkov [Wed, 3 Apr 2024 18:29:18 +0000 (21:29 +0300)]
Fix the parameters order for TableAmRoutine.relation_copy_for_cluster()

Specify OldTable first, NewTable second as used by
table_relation_copy_for_cluster() and as implemented in
heapam_relation_copy_for_cluster().

Backpatch to PostgreSQL 12, where TableAmRoutine was introduced.

Discussion: https://postgr.es/m/ME3P282MB3166860D4911AE82F92DF7C5B63F2%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
Author: Japin Li
Reviewed-by: Pavel Borisov
Backpatch-through: 12

14 months agoAvoid deadlock during orphan temp table removal.
Tom Lane [Tue, 2 Apr 2024 18:59:04 +0000 (14:59 -0400)]
Avoid deadlock during orphan temp table removal.

If temp tables have dependencies (such as sequences) then it's
possible for autovacuum's cleanup of orphan temp tables to deadlock
against an incoming backend that's trying to clean out the temp
namespace for its own use.  That can happen because RemoveTempRelations'
performDeletion call can visit objects within the namespace in
an order different from the order in which a per-table deletion
will visit them.

To fix, observe that performDeletion will begin by taking an exclusive
lock on the temp namespace (even though it won't actually delete it).
So, if we can get a shared lock on the namespace, we can be sure we're
not running concurrently with RemoveTempRelations, while also not
conflicting with ordinary use of the namespace.  This requires
introducing a conditional version of LockDatabaseObject, but that's no
big deal.  (It's surprising we've got along without that this long.)

Report and patch by Mikhail Zhilin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/c43ce028-2bc2-4865-9b89-3f706246eed5@postgrespro.ru

14 months agoAvoid "unused variable" warning on non-USE_SSL_ENGINE platforms.
Tom Lane [Mon, 1 Apr 2024 23:01:18 +0000 (19:01 -0400)]
Avoid "unused variable" warning on non-USE_SSL_ENGINE platforms.

If we are building with openssl but USE_SSL_ENGINE didn't get set,
initialize_SSL's variable "pkey" is declared but used nowhere.
Apparently this combination hasn't been exercised in the buildfarm
before now, because I've not seen this warning before, even though
the code has been like this a long time.  Move the declaration
to silence the warning (and remove its useless initialization).

Per buildfarm member sawshark.  Back-patch to all supported branches.

14 months agoAvoid possible longjmp-induced logic error in PLy_trigger_build_args.
Tom Lane [Mon, 1 Apr 2024 19:15:03 +0000 (15:15 -0400)]
Avoid possible longjmp-induced logic error in PLy_trigger_build_args.

The "pltargs" variable wasn't marked volatile, which makes it unsafe
to change its value within the PG_TRY block.  It looks like the worst
outcome would be to fail to release a refcount on Py_None during an
(improbable) error exit, which would likely go unnoticed in the field.
Still, it's a bug.  A one-liner fix could be to mark pltargs volatile,
but on the whole it seems cleaner to arrange things so that we don't
change its value within PG_TRY.

Per report from Xing Guo.  This has been there for quite awhile,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/CACpMh+DLrk=fDv07MNpBT4J413fDAm+gmMXgi8cjPONE+jvzuw@mail.gmail.com

14 months agodoc: fix CREATE ROLE typo
Bruce Momjian [Wed, 27 Mar 2024 21:58:10 +0000 (17:58 -0400)]
doc:  fix CREATE ROLE typo

This wording typo was added in PG 16.

Reported-by: [email protected]
Discussion: https://postgr.es/m/171150077554.7105.801523271545956671@wrigleys.postgresql.org

Backpatch-through: 16

14 months agoFix unnecessary use of moving-aggregate mode with non-moving frame.
Tom Lane [Wed, 27 Mar 2024 17:39:03 +0000 (13:39 -0400)]
Fix unnecessary use of moving-aggregate mode with non-moving frame.

When a plain aggregate is used as a window function, and the window
frame start is specified as UNBOUNDED PRECEDING, the frame's head
cannot move so we do not need to use moving-aggregate mode.  The check
for that was put into initialize_peragg(), failing to notice that
ExecInitWindowAgg() calls that function before it's filled in
winstate->frameOptions.  Since makeNode() would have zeroed the field,
this didn't provoke uninitialized-value complaints, nor would the
erroneous decision have resulted in more than a little inefficiency.
Still, it's wrong, so move the initialization of
winstate->frameOptions earlier to make it work properly.

While here, also fix a thinko in a comment.  Both errors crept in in
commit a9d9acbf2 which introduced the moving-aggregate mode.

Spotted by Vallimaharajan G.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/18e7f2a5167.fe36253866818.977923893562469143@zohocorp.com

14 months agoFix unstable aggregate regression test
David Rowley [Wed, 27 Mar 2024 11:14:17 +0000 (00:14 +1300)]
Fix unstable aggregate regression test

Buildfarm member avocet has shown a plan change by switching the
finalize aggregate stage to use a GroupAggregate rather than a
HashAggregate.  This is consistent with autovacuum having triggered on
the table, per analysis by Alexander Lakhin.

Fix this by disabling autovacuum on the table.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/d4493a28-589a-5328-fed5-250f2d7d3e2a@gmail.com
Backpatch-through: 16, where this test was added.

14 months agoFix failure of ALTER FOREIGN TABLE SET SCHEMA to move sequences.
Tom Lane [Tue, 26 Mar 2024 19:28:16 +0000 (15:28 -0400)]
Fix failure of ALTER FOREIGN TABLE SET SCHEMA to move sequences.

Ordinary ALTER TABLE SET SCHEMA will also move any owned sequences
into the new schema.  We failed to do likewise for foreign tables,
because AlterTableNamespaceInternal believed that only certain
relkinds could have indexes, owned sequences, or constraints.
We could simply add foreign tables to that relkind list, but it
seems likely that the same oversight could be made again in
future.  Instead let's remove the relkind filter altogether.
These functions shouldn't cost much when there are no objects
that they need to process, and surely this isn't an especially
performance-critical case anyway.

Per bug #18407 from Vidushi Gupta.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18407-4fd07373d252c6a0@postgresql.org

14 months agoAllow "make check"-style testing to work with musl C library.
Tom Lane [Tue, 26 Mar 2024 15:44:49 +0000 (11:44 -0400)]
Allow "make check"-style testing to work with musl C library.

The musl dynamic linker saves a pointer to the process' environment
value of LD_LIBRARY_PATH very early in startup.  When we move/clobber
the environment to make more room for ps status strings, we clobber
that value and thereby prevent libraries from being found via
LD_LIBRARY_PATH, which breaks the use of a temporary installation
for testing purposes.  To fix, stop collecting usable space for
ps status if we notice that the variable we are about to clobber
is LD_LIBRARY_PATH.  This will result in some reduction in how long
the ps status can be, but it's only likely to occur in temporary
test contexts, so it doesn't seem like a big problem.  In any case,
we don't have to do it if we see we are on glibc, which surely is
where the majority of our Linux testing is done.

Thomas Munro, Bruce Momjian, and Tom Lane, per report from Wolfgang
Walther.  Back-patch to all supported branches, with the hope that
we'll set up a buildfarm animal to test on this platform.

Discussion: https://postgr.es/m/fddd1cd6-dc16-40a2-9eb5-d7fef2101488@technowledgy.de

14 months agoAvoid edge case in pg_visibility test with small shared_buffers
Andres Freund [Tue, 26 Mar 2024 02:58:10 +0000 (19:58 -0700)]
Avoid edge case in pg_visibility test with small shared_buffers

Since 82a4edabd27 we can bulk extend relations. The bulk relation extension
logic has a heuristic component. Normally the heurstic does not trigger in the
occasionally-failing test case, as the relation is only extended once. But
with very small shared_buffers the limits for the number of buffers pinned at
once prevent the extension from happening at once. With the second "bulk"
extension, the heuristic kicks in, and the relation ends up one block bigger.
That's ok from a correctness perspective, but changes the results of the test
query due to one additional block.

We discussed a few more expansive fixes, but for now have decided to avoid
this by making the table a bit smaller.

Author: Heikki Linnakangas 
Reported-by:
Discussion: https://postgr.es/m/29c74104-210b-ef39-2522-27a6aa7a704f@iki.fi
Discussion: https://postgr.es/m/20230916000011[email protected]
Backpatch: 16-, where the new relation extension logic was added

14 months agoci: macos: Choose python version
Andres Freund [Mon, 25 Mar 2024 20:06:58 +0000 (13:06 -0700)]
ci: macos: Choose python version

The CI base image used to have a python3 with headers etc installed in PATH,
but doesn't anymore. Instead of relying on a specific version in the base
image, explicitly install one ourselves.

On 16 and HEAD this lead to a build without python support, but on 15 CI
failed, due to explicitly enabled python3 support.

14 months agoClarify comment for LogicalTapeSetBlocks().
Jeff Davis [Mon, 25 Mar 2024 18:51:44 +0000 (11:51 -0700)]
Clarify comment for LogicalTapeSetBlocks().

Discussion: https://postgr.es/m/1229327.1711160246@sss.pgh.pa.us
Backpatch-through: 13

14 months agodoc: Clarify requirements for SET ROLE.
Nathan Bossart [Sun, 24 Mar 2024 20:24:09 +0000 (15:24 -0500)]
doc: Clarify requirements for SET ROLE.

Since commit 3d14e171e9, SET ROLE has required the current session
user to have membership with the SET option in the target role, but
the SET ROLE documentation only mentions the membership
requirement.  This commit adds this important detail to the SET
ROLE page.

Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/CA%2BRLCQysHtME0znk2KUMJN343ksboSRQSU-hCnOjesX6VK300Q%40mail.gmail.com
Backpatch-through: 16

14 months agoamcheck: Normalize index tuples containing uncompressed varlena
Alexander Korotkov [Sat, 23 Mar 2024 21:00:06 +0000 (23:00 +0200)]
amcheck: Normalize index tuples containing uncompressed varlena

It might happen that the varlena value wasn't compressed by index_form_tuple()
due to current storage parameters.  If compression is currently enabled, we
need to compress such values to match index tuple coming from the heap.

Backpatch to all supported versions.

Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru
Author: Andrey Borodin
Reviewed-by: Alexander Lakhin, Michael Zhilin, Jian He, Alexander Korotkov
Backpatch-through: 12

14 months agoamcheck: Support for different header sizes of short varlena datum
Alexander Korotkov [Sat, 23 Mar 2024 20:59:56 +0000 (22:59 +0200)]
amcheck: Support for different header sizes of short varlena datum

In the heap, tuples may contain short varlena datum with both 1B header and 4B
headers.  But the corresponding index tuple should always have such varlena's
with 1B headers.  So, for fingerprinting, we need to convert.

Backpatch to all supported versions.

Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru
Author: Michael Zhilin
Reviewed-by: Alexander Lakhin, Andrey Borodin, Jian He, Alexander Korotkov
Backpatch-through: 12

14 months agoUse a hash table for catcache.c's CatCList objects.
Tom Lane [Fri, 22 Mar 2024 21:13:53 +0000 (17:13 -0400)]
Use a hash table for catcache.c's CatCList objects.

Up to now, all of the "catcache list" objects within a catalog cache
were just chained together on a single dlist, requiring O(N) time to
search.  Remarkably, we've not had serious performance problems with
that so far; but we got a complaint of a bad performance regression
from v15 in a case with a large number of roles in the system, which
traced down to O(N^2) total time when we probed N catcache lists.

Replace that data structure with a hashtable having an enlargeable
number of dlists, in an exactly parallel way to the data structure
we've used for years for the plain CatCTup cache members.  The extra
cost of maintaining a hash table seems negligible, since we were
already computing a hash value for list searches.

Normally this'd be HEAD-only material, but in view of the performance
regression it seems advisable to back-patch into v16.  In the v16
version of the patch, leave the dead cc_lists field where it is and
add the new fields at the end of struct catcache, to avoid possible
ABI breakage in case any external code is looking at these structs.
(We assume no external code is actually allocating new catcache
structs.)

Per report from alex work.

Discussion: https://postgr.es/m/CAGvXd3OSMbJQwOSc-Tq-Ro1CAz=vggErdSG7pv2s6vmmTOLJSg@mail.gmail.com

14 months agoFix dumping role comments when using --no-role-passwords
Daniel Gustafsson [Thu, 21 Mar 2024 22:31:57 +0000 (23:31 +0100)]
Fix dumping role comments when using --no-role-passwords

Commit 9a83d56b38c added support for allowing pg_dumpall to dump
roles without including passwords, which accidentally made dumps
omit COMMENTs on roles.  This fixes it by using pg_authid to get
the comment.

Backpatch to all supported versions. Patch simultaneously written
independently by Álvaro and myself.

Author: Álvaro Herrera 
Author: Daniel Gustafsson 
Reported-by: Bartosz Chroł
Discussion: https://postgr.es/m/AS8P194MB1271CDA0ADCA7B75FCD8E767F7332@AS8P194MB1271.EURP194.PROD.OUTLOOK.COM
Discussion: https://postgr.es/m/CAEP4nAz9V4H41_4ESJd1Gf0v%3DdevkqO1%3Dpo91jUw-GJSx8Hxqg%40mail.gmail.com
Backpatch-through: v12

14 months agoReview wording on tablespaces w.r.t. partitioned tables
Alvaro Herrera [Wed, 20 Mar 2024 14:28:14 +0000 (15:28 +0100)]
Review wording on tablespaces w.r.t. partitioned tables

Remove a redundant comment, and document pg_class.reltablespace properly
in catalogs.sgml.

After commits a36c84c3e4a987259588d0ab and others.

Backpatch to 12.

Discussion: https://postgr.es/m/202403191013[email protected]

14 months agoFix EXPLAIN Bitmap heap scan to count pages with no visible tuples
Heikki Linnakangas [Mon, 18 Mar 2024 12:04:13 +0000 (14:04 +0200)]
Fix EXPLAIN Bitmap heap scan to count pages with no visible tuples

Previously, bitmap heap scans only counted lossy and exact pages for
explain when there was at least one visible tuple on the page.

heapam_scan_bitmap_next_block() returned true only if there was a
"valid" page with tuples to be processed. However, the lossy and exact
page counters in EXPLAIN should count the number of pages represented
in a lossy or non-lossy way in the constructed bitmap, regardless of
whether or not the pages ultimately contained visible tuples.

Backpatch to all supported versions.

Author: Melanie Plageman
Discussion: https://www.postgresql.org/message-id/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA@mail.gmail.com
Discussion: https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%[email protected]

15 months agoFix EXPLAIN output for subplans in MERGE.
Dean Rasheed [Sun, 17 Mar 2024 10:19:31 +0000 (10:19 +0000)]
Fix EXPLAIN output for subplans in MERGE.

Given a subplan in a MERGE query, EXPLAIN would sometimes fail to
properly display expressions involving Params referencing variables in
other parts of the plan tree.

This would affect subplans outside the topmost join plan node, for
which expansion of Params would go via the top-level ModifyTable plan
node.  The problem was that "inner_tlist" for the ModifyTable node's
deparse_namespace was set to the join node's targetlist, but
"inner_plan" was set to the ModifyTable node itself, rather than the
join node, leading to incorrect results when descending to the
referenced variable.

Fix and backpatch to v15, where MERGE was introduced.

Discussion: https://postgr.es/m/CAEZATCWAv-sZuH%2BwG5xJ-%2BGt7qGNGX8wUQd3XYydMFDKgRB9nw%40mail.gmail.com

15 months agoFix handling of expecteddir in pg_regress
Daniel Gustafsson [Fri, 15 Mar 2024 16:02:07 +0000 (17:02 +0100)]
Fix handling of expecteddir in pg_regress

Commit c855872074b introduced a new parameter to pg_regress to set
the directory where to look for expected files, but accidentally
only implemented it for when compiling pg_regress for ECPG tests.
Fix by adding support for the parameter to the main regression test
compilation of pg_regress as well.

Backpatch to v16 where --expecteddir was introduced.

Author: Anthonin Bonnefoy 
Discussion: https://postgr.es/m/CAO6_Xqq5yKJHcJsq__LPcKwSY0XHRqVERNWGxx5ttNXXj7+W=A@mail.gmail.com
Backpatch-through: 16

15 months agoTrim ORDER BY/DISTINCT aggregate pathkeys in gather_grouping_paths
David Rowley [Thu, 14 Mar 2024 22:55:50 +0000 (11:55 +1300)]
Trim ORDER BY/DISTINCT aggregate pathkeys in gather_grouping_paths

Similar to d8a295389, trim off any PathKeys which are for ORDER BY /
DISTINCT aggregate functions from the PathKey List for the Gather Merge
paths created by gather_grouping_paths().  These additional PathKeys are
not valid to use after grouping has taken place as these PathKeys belong
to columns which are inputs to an aggregate function and, therefore are
unavailable after aggregation.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/cf63174c-8c89-3953-cb49-48f41f74941a@gmail.com
Backpatch-through: 16, where 1349d2790 was added

15 months agoMake INSERT-from-multiple-VALUES-rows handle domain target columns.
Tom Lane [Thu, 14 Mar 2024 18:57:16 +0000 (14:57 -0400)]
Make INSERT-from-multiple-VALUES-rows handle domain target columns.

Commit a3c7a993d fixed some cases involving target columns that are
arrays or composites by applying transformAssignedExpr to the VALUES
entries, and then stripping off any assignment ArrayRefs or
FieldStores that the transformation added.  But I forgot about domains
over arrays or composites :-(.  Such cases would either fail with
surprising complaints about mismatched datatypes, or insert unexpected
coercions that could lead to odd results.  To fix, extend the
stripping logic to get rid of CoerceToDomain if it's atop an ArrayRef
or FieldStore.

While poking at this, I realized that there's a poorly documented and
not-at-all-tested behavior nearby: we coerce each VALUES column to
the domain type separately, and rely on the rewriter to merge those
operations so that the domain constraints are checked only once.
If that merging did not happen, it's entirely possible that we'd get
unexpected domain constraint failures due to checking a
partially-updated container value.  There's no bug there, but while
we're here let's improve the commentary about it and add some test
cases that explicitly exercise that behavior.

Per bug #18393 from Pablo Kharo.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18393-65fedb1a0de9260d@postgresql.org

15 months agodoc: Improve a couple of places in the MERGE docs.
Dean Rasheed [Wed, 13 Mar 2024 13:15:07 +0000 (13:15 +0000)]
doc: Improve a couple of places in the MERGE docs.

In the synopsis, make the syntax for merge_update consistent with the
syntax for a plain UPDATE command. It was missing the optional "ROW"
keyword that can be used in a multi-column assignment, and the option
to assign from a multi-column subquery, both of which have been
supported by MERGE since it was introduced.

In the parameters section for the with_query parameter, mention that
WITH RECURSIVE isn't supported, since this is different from plain
INSERT, UPDATE, and DELETE commands. While at it, move that entry to
the top of the list, for consistency with the other pages.

Back-patch to v15, where MERGE was introduced.

Discussion: https://postgr.es/m/CAEZATCWoQyWkMFfu7JXXQr8dA6%3DgxjhYzgpuBP2oz0QoJTxGWw%40mail.gmail.com

15 months agomeson: macos: Avoid warnings on Sonoma
Andres Freund [Wed, 13 Mar 2024 08:40:48 +0000 (01:40 -0700)]
meson: macos: Avoid warnings on Sonoma

Starting with the Sonoma toolchain macos' linker emits warnings when the same
library is linked to twice. That's ill considered, as the same library can be
used by multiple subsidiary libraries. Luckily there's a flag to suppress that
warning.

On Ventura meson's default of -Wl,-undefined,dynamic_lookup caused warnings,
which we suppressed with -Wl,-undefined,error. Unfortunately that causes a
warning on Sonoma, which is absurd, as it's documented linker default. To
avoid that warning, only add -Wl,-undefined,error if it does not trigger
warnings. Luckily dynamic_lookup doesn't trigger a warning on Sonoma anymore.

Discussion: https://postgr.es/m/20231201040515[email protected]
Backpatch: 16-, where the meson build was added

15 months agoFix confusion about the return rowtype of SQL-language procedures.
Tom Lane [Tue, 12 Mar 2024 22:16:10 +0000 (18:16 -0400)]
Fix confusion about the return rowtype of SQL-language procedures.

There is a very ancient hack in check_sql_fn_retval that allows a
single SELECT targetlist entry of composite type to be taken as
supplying all the output columns of a function returning composite.
(This is grotty and fundamentally ambiguous, but it's really hard
to do nested composite-returning functions without it.)

As far as I know, that doesn't cause any problems in ordinary
functions.  It's disastrous for procedures however.  All procedures
that have any output parameters are labeled with prorettype RECORD,
and the CALL code expects it will get back a record with one column
per output parameter, regardless of whether any of those parameters
is composite.  Doing something else leads to an assertion failure
or core dump.

This is simple enough to fix: we just need to not apply that rule
when considering procedures.  However, that requires adding another
argument to check_sql_fn_retval, which at least in principle might be
getting called by external callers.  Therefore, in the back branches
convert check_sql_fn_retval into an ABI-preserving wrapper around a
new function check_sql_fn_retval_ext.

Per report from Yahor Yuzefovich.  This has been broken since we
implemented procedures, so back-patch to all supported branches.

Discussion: https://postgr.es/m/CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com

15 months agoDisconnect if socket cannot be put into non-blocking mode
Heikki Linnakangas [Tue, 12 Mar 2024 08:18:32 +0000 (10:18 +0200)]
Disconnect if socket cannot be put into non-blocking mode

Commit 387da18874 moved the code to put socket into non-blocking mode
from socket_set_nonblocking() into the one-time initialization
function, pq_init(). In socket_set_nonblocking(), there indeed was a
risk of recursion on failure like the comment said, but in pq_init(),
ERROR or FATAL is fine. There's even another elog(FATAL) just after
this, if setting FD_CLOEXEC fails.

Note that COMMERROR merely logged the error, it did not close the
connection, so if putting the socket to non-blocking mode failed we
would use the connection anyway. You might not immediately notice,
because most socket operations in a regular backend wait for the
socket to become readable/writable anyway. But e.g. replication will
be quite broken.

Backpatch to all supported versions.

Discussion: https://www.postgresql.org/message-id/d40a5cd0-2722-40c5-8755-12e9e811fa3c@iki.fi

15 months agodoc: add missing word "the"
Bruce Momjian [Mon, 11 Mar 2024 17:31:13 +0000 (13:31 -0400)]
doc:  add missing word "the"

Reported-by: [email protected]
Discussion: https://postgr.es/m/170993253510.640.5664117187431542912@wrigleys.postgresql.org

Backpatch-through: 12

15 months agoSet all_visible_according_to_vm correctly with DISABLE_PAGE_SKIPPING
Heikki Linnakangas [Mon, 11 Mar 2024 07:28:09 +0000 (09:28 +0200)]
Set all_visible_according_to_vm correctly with DISABLE_PAGE_SKIPPING

It's important for 'all_visible_according_to_vm' to correctly reflect
whether the VM bit is set or not, even when we are not trusting the VM
to skip pages, because contrary to what the comment said,
lazy_scan_prune() relies on it.

If it's incorrectly set to 'false', when the VM bit is in fact set,
lazy_scan_prune() will try to set the VM bit again and dirty the page
unnecessarily. As a result, if you used DISABLE_PAGE_SKIPPING, all
heap pages were dirtied, even if there were no changes. We would also
fail to clear any VM bits that were set incorrectly.

This was broken in commit 980ae17310, so backpatch to v16.

Backpatch-through: 16
Reviewed-by: Melanie Plageman, Peter Geoghegan
Discussion: https://www.postgresql.org/message-id/3df2b582-dc1c-46b6-99b6-38eddd1b2784@iki.fi

15 months agoFix incorrect accessing of pfree'd memory in Memoize
David Rowley [Mon, 11 Mar 2024 05:20:39 +0000 (18:20 +1300)]
Fix incorrect accessing of pfree'd memory in Memoize

For pass-by-reference types, the code added in 0b053e78b, which aimed to
resolve a memory leak, was overly aggressive in resetting the per-tuple
memory context which could result in pfree'd memory being accessed
resulting in failing to find previously cached results in the hash
table.

What was happening was prepare_probe_slot() was switching to the
per-tuple memory context and calling ExecEvalExpr().  ExecEvalExpr() may
have required a memory allocation.  Both MemoizeHash_hash() and
MemoizeHash_equal() were aggressively resetting the per-tuple context
and after determining the hash value, the context would have gotten reset
before MemoizeHash_equal() was called.  This could have resulted in
MemoizeHash_equal() looking at pfree'd memory.

This is less likely to have caused issues on a production build as some
other allocation would have had to have reused the pfree'd memory to
overwrite it.  Otherwise, the original contents would have been intact.
However, this clearly caused issues on MEMORY_CONTEXT_CHECKING builds.

Author: Tender Wang, Andrei Lepikhov
Reported-by: Tender Wang (using SQLancer)
Reviewed-by: Andrei Lepikhov, Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com
Backpatch-through: 14, where Memoize was added

15 months agoBackpatch missing check_stack_depth() to some recursive functions
Alexander Korotkov [Fri, 16 Feb 2024 14:02:00 +0000 (16:02 +0200)]
Backpatch missing check_stack_depth() to some recursive functions

Backpatch changes from d57b7cc33375bcba6cbd to all supported branches per
proposal of Egor Chindyaskin.

Discussion: https://postgr.es/m/DE5FD776-A8CD-4378-BCFA-3BF30F1F6D60%40mail.ru

15 months agoFix deparsing of Consts in postgres_fdw ORDER BY
David Rowley [Sun, 10 Mar 2024 23:27:46 +0000 (12:27 +1300)]
Fix deparsing of Consts in postgres_fdw ORDER BY

For UNION ALL queries where a union child query contained a foreign
table, if the targetlist of that query contained a constant, and the
top-level query performed an ORDER BY which contained the column for the
constant value, then postgres_fdw would find the EquivalenceMember with
the Const and then try to produce an ORDER BY containing that Const.

This caused problems with INT typed Consts as these could appear to be
requests to order by an ordinal column position rather than the constant
value.  This could lead to either an error such as:

ERROR:  ORDER BY position  is not in select list

or worse, if the constant value is a valid column, then we could just
sort by the wrong column altogether.

Here we fix this issue by just not including these Consts in the ORDER
BY clause.

In passing, add a new section for testing ORDER BY in the postgres_fdw
tests and move two existing tests which were misplaced in the WHERE
clause testing section into it.

Reported-by: Michał Kłeczek
Reviewed-by: Ashutosh Bapat, Richard Guo
Bug: #18381
Discussion: https://postgr.es/m/0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E%40kleczek.org
Discussion: https://postgr.es/m/18381-137456acd168bf93%40postgresql.org
Backpatch-through: 12, oldest supported version

15 months agoCope with a deficiency in OpenSSL 3.x's error reporting.
Tom Lane [Fri, 8 Mar 2024 00:37:51 +0000 (19:37 -0500)]
Cope with a deficiency in OpenSSL 3.x's error reporting.

In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses
to provide a string for error codes representing system errno values
(e.g., "No such file or directory").  There is a poorly-documented way
to extract the errno from the SSL error code in this case, so do that
and apply strerror, rather than falling back to reporting the error
code's numeric value as we were previously doing.

Problem reported by David Zhang, although this is not his proposed
patch; it's instead based on a suggestion from Heikki Linnakangas.
Back-patch to all supported branches, since any of them are likely
to be used with recent OpenSSL.

Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca

15 months agoFix handling of self-modified tuples in MERGE.
Dean Rasheed [Thu, 7 Mar 2024 09:55:39 +0000 (09:55 +0000)]
Fix handling of self-modified tuples in MERGE.

When an UPDATE or DELETE action in MERGE returns TM_SelfModified,
there are 2 possible causes:

1). The target tuple was already updated or deleted by the current
    command. This can happen if the target row joins to more than one
    source row, and the SQL standard explicitly says that this must be
    an error.

2). The target tuple was already updated or deleted by a later command
    in the current transaction. This can happen if the tuple is
    modified by a BEFORE trigger or a volatile function used in the
    query, and should be an error for the same reason that it is in a
    plain UPDATE or DELETE command.

In MERGE's primary error handling block, it failed to check for (2),
causing it to return a misleading error message in such cases.

In the secondary error handling block, following a concurrent update
from another session, it failed to check for (1), causing it to
silently ignore target rows joined to more than one source row,
instead of reporting an error.

Fix this, and add tests for both of these cases.

Per report from Wenjiang Zhang. Back-patch to v15, where MERGE was
introduced.

Discussion: https://postgr.es/m/tencent_41DE0FF443FE14B94A5898D373792109E408%40qq.com

15 months agoRevert "Fix parallel-safety check of expressions and predicate for index builds"
Michael Paquier [Wed, 6 Mar 2024 23:31:00 +0000 (08:31 +0900)]
Revert "Fix parallel-safety check of expressions and predicate for index builds"

This reverts commit eae7be600be7, following a discussion with Tom Lane,
due to concerns that this impacts the decisions made by the planner for
the number of workers spawned based on the inlining and const-folding of
index expressions and predicate for cases that would have worked until
this commit.

Discussion: https://postgr.es/m/162802.1709746091@sss.pgh.pa.us
Backpatch-through: 12

15 months agoFix type-checking of RECORD-returning functions in FROM.
Tom Lane [Wed, 6 Mar 2024 19:41:13 +0000 (14:41 -0500)]
Fix type-checking of RECORD-returning functions in FROM.

In the corner case where a function returning RECORD has been
simplified to a RECORD constant or an inlined ROW() expression,
ExecInitFunctionScan failed to cross-check the function's result
rowtype against the coldeflist provided by the calling query.
That happened because get_expr_result_type is able to extract a
tupdesc from such expressions, which led ExecInitFunctionScan to
ignore the coldeflist.  (Instead, it used the extracted tupdesc
to check the function's output, which of course always succeeds.)

I have not been able to demonstrate any really serious consequences
from this, because if some column of the result is of the wrong
type and is directly referenced by a Var of the calling query,
CheckVarSlotCompatibility will catch it.  However, we definitely do
fail to report the case where the function returns more columns than
the coldeflist expects, and in the converse case where it returns
fewer columns, we get an assert failure (but, seemingly, no worse
results in non-assert builds).

To fix, always build the expected tupdesc from the coldeflist if there
is one, and consult get_expr_result_type only when there isn't one.

Also remove the failing Assert, even though it is no longer reached
after this fix.  It doesn't seem to be adding anything useful, since
later checking will deal with cases with the wrong number of columns.

The only other place I could find that is doing something similar
is inline_set_returning_function.  There's no live bug there because
we cannot be looking at a Const or RowExpr, but for consistency
change that code to agree with ExecInitFunctionScan.

Per report from PetSerAl.  After some debate I've concluded that
this should be back-patched.  There is a small risk that somebody
has been relying on such a case not throwing an error, but I judge
this outweighed by the risk that I've missed some way in which the
failure to cross-check has worse consequences than sketched above.

Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com

15 months agoFix parallel-safety check of expressions and predicate for index builds
Michael Paquier [Wed, 6 Mar 2024 08:24:05 +0000 (17:24 +0900)]
Fix parallel-safety check of expressions and predicate for index builds

As coded, the planner logic that calculates the number of parallel
workers to use for a parallel index build uses expressions and
predicates from the relcache, which are flattened for the planner by
eval_const_expressions().

As reported in the bug, an immutable parallel-unsafe function flattened
in the relcache would become a Const, which would be considered as
parallel-safe, even if the predicate or the expressions including the
function are not safe in parallel workers.  Depending on the expressions
or predicate used, this could cause the parallel build to fail.

Tests are included that check parallel index builds with parallel-unsafe
predicate and expressions.  Two routines are added to lsyscache.h to be
able to retrieve expressions and predicate of an index from its pg_index
data.

Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Jian He, Michael Paquier
Discussion: https://postgr.es/m/CAHewXN=UaAaNn9ruHDH3Os8kxLVmtWqbssnf=dZN_s9=evHUFA@mail.gmail.com
Backpatch-through: 12

15 months agodocs: Update HOT update docs for 19d8e2308b
Jeff Davis [Tue, 5 Mar 2024 17:25:04 +0000 (09:25 -0800)]
docs: Update HOT update docs for 19d8e2308b

Commit 19d8e2308b changed when the HOT update optimization is possible
but neglected to update the Heap-Only Tuples (HOT) documentation.  This
patch updates that documentation accordingly.

Author: Elizabeth Christensen
Backpatch-through: 16
Reviewed-By: Stephen Frost, Alvaro Herrera
Discussion: https://postgr.es/m/CABoUFXRjisr58Ct_3VsFEdQx+fJeQTWTdJnM7XAp=8MUbtoa9A@mail.gmail.com

15 months agoFix incorrectly reported stats kind in "can't happen" ERROR
David Rowley [Tue, 5 Mar 2024 03:17:53 +0000 (16:17 +1300)]
Fix incorrectly reported stats kind in "can't happen" ERROR

The error message(s) were reporting the stats kind of 'f', which is not
correct as that's for the "dependencies" statistics kind.

Reported-by: Horst Reiterer
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/18375-ba99383eb9062d6a@postgresql.org
Backpatch-through: 12, where MCV extended stats were added.

15 months agoFix initdb's -c option to treat the GUC name case-insensitively.
Tom Lane [Mon, 4 Mar 2024 17:00:39 +0000 (12:00 -0500)]
Fix initdb's -c option to treat the GUC name case-insensitively.

The backend treats GUC names case-insensitively, so this code should
too.  This avoids ending up with a confusing set of redundant entries
in the generated postgresql.conf file.

Per report from Kyotaro Horiguchi.  Back-patch to v16 where this
feature was added (in commit 3e51b278d).

Discussion: https://postgr.es/m/20230928.164904.2153358973162534034[email protected]

15 months agodoc: Fix datatype for postgres_fdw option
Daniel Gustafsson [Mon, 4 Mar 2024 09:52:19 +0000 (10:52 +0100)]
doc: Fix datatype for postgres_fdw option

The datatype for analyze_sampling had accidentally been set to text
and not string.  Backpatch to v16 where analyze_sampling first was
introduced.

Author: Shinya Kato 
Reviewed-by: Laurenz Albe
Discussion: https://postgr.es/m/7fd9166b9fda267411793f39986d7f24@oss.nttdata.com
Backpatch-through: v16

15 months agoFix integer underflow in shared memory debugging
Daniel Gustafsson [Thu, 29 Feb 2024 11:19:52 +0000 (12:19 +0100)]
Fix integer underflow in shared memory debugging

dsa_dump would print a large negative number instead of zero for
segment bin 0.  Fix by explicitly checking for underflow and add
special case for bin 0. Backpatch to all supported versions.

Author: Ian Ilyasov 
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/GV1P251MB1004E0D09D117D3CECF9256ECD502@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM
Backpatch-through: v12

15 months agoFix mis-rounding and overflow hazards in date_bin().
Tom Lane [Wed, 28 Feb 2024 19:00:30 +0000 (14:00 -0500)]
Fix mis-rounding and overflow hazards in date_bin().

In the case where the target timestamp is before the origin timestamp
and their difference is already an exact multiple of the stride, the
code incorrectly subtracted the stride anyway.

Also detect several integer-overflow cases that previously produced
bogus results.  (The submitted patch tried to avoid overflow, but
I'm not convinced it's right, and problematic cases are so far out of
the plausibly-useful range that they don't seem worth sweating over.
Let's just use overflow-detecting arithmetic and throw errors.)

timestamp_bin() and timestamptz_bin() are basically identical and
so had identical bugs.  Fix both.

Report and patch by Moaaz Assali, adjusted some by me.  Back-patch
to v14 where date_bin() was introduced.

Discussion: https://postgr.es/m/CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@mail.gmail.com

15 months agoRevise MERGE documentation
Alvaro Herrera [Mon, 26 Feb 2024 17:19:03 +0000 (18:19 +0100)]
Revise MERGE documentation

Add a note about the additional privileges required after the fix in
4989ce72644b (wording per Tom Lane); also change marked-up mentions of
"target_table_name" to be simply "the target table" or the like.  Also,
note that "join_condition" is scouted for requisite privileges.

Backpatch to 15.

Discussion: https://postgr.es/m/202402211653[email protected]

15 months agoBack-patch test modifications that were done as part of b6df0798a5.
Amit Kapila [Mon, 26 Feb 2024 04:03:57 +0000 (09:33 +0530)]
Back-patch test modifications that were done as part of b6df0798a5.

This commit fixes the intermittent buildfarm failures in 031_column_list.
I missed to back-patch while committing b6df0798a5 in the HEAD.

Diagnosed-by: Amit Kapila
Author: Vignesh C
Backpatch-through: 15
Discussion: https://postgr.es/m/3307255.1706911634@sss.pgh.pa.us

15 months agoPromote assertion about !ReindexIsProcessingIndex to runtime error.
Tom Lane [Sun, 25 Feb 2024 21:15:07 +0000 (16:15 -0500)]
Promote assertion about !ReindexIsProcessingIndex to runtime error.

When this assertion was installed (in commit d2f60a3ab), I thought
it was only for catching server logic errors that caused accesses to
catalogs that were undergoing index rebuilds.  However, it will also
fire in case of a user-defined index expression that attempts to
access its own table.  We occasionally see reports of people trying
to do that, and typically getting unintelligible low-level errors
as a result.  We can provide a more on-point message by making this
a regular runtime check.

While at it, adjust the similar error check in
systable_beginscan_ordered to use the same message text.  That one
is (probably) not reachable without a coding bug, but we might as
well use a translatable message if we have one.

Per bug #18363 from Alexander Lakhin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18363-e3598a5a572d0699@postgresql.org

15 months agoDoc: fix minor typos in two ECPG function descriptions.
Tom Lane [Sun, 25 Feb 2024 20:29:09 +0000 (15:29 -0500)]
Doc: fix minor typos in two ECPG function descriptions.

Noted by Aidar Imamov.

Discussion: https://postgr.es/m/170869935022.643.3709087848818148291@wrigleys.postgresql.org

15 months agoAvoid dangling-pointer problem with partitionwise joins under GEQO.
Tom Lane [Fri, 23 Feb 2024 20:21:53 +0000 (15:21 -0500)]
Avoid dangling-pointer problem with partitionwise joins under GEQO.

build_child_join_sjinfo creates a derived SpecialJoinInfo in
the short-lived GEQO context, but afterwards the semi_rhs_exprs
from that may be used in a UniquePath for a child base relation.
This breaks the expectation that all base-relation-level structures
are in the planning-lifespan context, leading to use of a dangling
pointer with probable ensuing crash later on in create_unique_plan.
To fix, copy the expression trees when making a UniquePath.

Per bug #18360 from Alexander Lakhin.  This has been broken since
partitionwise joins were added, so back-patch to all supported
branches.

Discussion: https://postgr.es/m/18360-a23caf3157f34e62@postgresql.org

15 months agoFix mistake in SQL features list
Peter Eisentraut [Fri, 23 Feb 2024 13:40:25 +0000 (14:40 +0100)]
Fix mistake in SQL features list

Fix for c9f57541d97: Feature F302-02 was renamed to F305, but that
commit failed to delete the old line.

Reported-by: Satoru Koizumi (小泉 悟)
Discussion: https://www.postgresql.org/message-id/flat/170866661469.645.14101429540172934386%40wrigleys.postgresql.org

15 months agoMERGE ... DO NOTHING: require SELECT privileges
Alvaro Herrera [Wed, 21 Feb 2024 16:18:52 +0000 (17:18 +0100)]
MERGE ... DO NOTHING: require SELECT privileges

Verify that a user running MERGE with a DO NOTHING clause has
privileges to read the table, even if no columns are referenced.  Such
privileges were already required if the ON clause or any of the WHEN
conditions referenced any column at all, so there's no functional change
in practice.

This change fixes an assertion failure in the case where no column is
referenced by the command and the WHEN clauses are all DO NOTHING.

Backpatch to 15, where MERGE was introduced.

Reported-by: Alena Rybakina
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/4d65a385-7efa-4436-a825-0869f89d9d92@postgrespro.ru

15 months agodoc: Fix link to pg_ident_file_mappings view
Daniel Gustafsson [Wed, 21 Feb 2024 08:22:18 +0000 (09:22 +0100)]
doc: Fix link to pg_ident_file_mappings view

Commit a2c84990bea7 accidentally used the link for pg_hba_file_rules
when linking to pg_ident_file_mappings.  Backpatch to v16 where this
was introduced.

Author: Erik Wienhold 
Discussion: https://postgr.es/m/qt5hvgvfi4qzlgml2dfssaut2t2x5nwf7b5l63fklr7fpxwm6g@hle3mtglpm4y
Backpatch-through: v16

15 months agoDoc: improve explanation of type interval, especially extract().
Tom Lane [Tue, 20 Feb 2024 19:35:12 +0000 (14:35 -0500)]
Doc: improve explanation of type interval, especially extract().

The explanation of interval's behavior in datatype.sgml wasn't wrong
exactly, but it was unclear, partly because it buried the lede about
there being three internal fields.  Rearrange and wordsmith for more
clarity.

The discussion of extract() claimed that input of type date was
handled by casting, but actually there's been a separate SQL function
taking date for a very long time.  Also, it was mostly silent about
how interval inputs are handled, but there are several field types
for which it seems useful to be specific.

Improve discussion of justify_days()/justify_hours() too.

In passing, remove vertical space in some groups of examples,
as there was little consistency about whether to have such space
or not.  (I only did this within the datetime functions section;
there are some related inconsistencies elsewhere.)

Per discussion of bug #18348 from Michael Bondarenko.  There
may be some code changes coming out of that discussion too,
but we likely won't back-patch them.  This docs-only patch
seems useful to back-patch, though I only carried it back to
v13 because it didn't apply easily in v12.

Discussion: https://postgr.es/m/18348-b097a3587dfde8a4@postgresql.org

15 months agoDoc: correct minor error in back-branch release notes.
Tom Lane [Tue, 20 Feb 2024 16:58:28 +0000 (11:58 -0500)]
Doc: correct minor error in back-branch release notes.

Commits 1b2c6b756 et al affected the core BRIN "bloom" opclasses,
not contrib/bloom.  This only corrected a bad assertion so it's not
too significant to end users, but since we documented it we should
do so accurately.

Spotted by Takatsuka Haruka.

Discussion: https://postgr.es/m/18353-926aa99cfe58aa78@postgresql.org

15 months agoFix race leading to incorrect conflict cause in InvalidatePossiblyObsoleteSlot()
Michael Paquier [Tue, 20 Feb 2024 04:43:56 +0000 (13:43 +0900)]
Fix race leading to incorrect conflict cause in InvalidatePossiblyObsoleteSlot()

The invalidation of an active slot is done in two steps:
- Termination of the backend holding it, if any.
- Report that the slot is obsolete, with a conflict cause depending on
the slot's data.

This can be racy because between these two steps the slot mutex would be
released while doing system calls, which means that the effective_xmin
and effective_catalog_xmin could advance during that time, detecting a
conflict cause different than the one originally wanted before the
process owning a slot is terminated.

Holding the mutex longer is not an option, so this commit changes the
code to record the LSNs stored in the slot during the termination of the
process owning the slot.

Bonus thanks to Alexander Lakhin for the various tests and the analysis.

Author: Bertrand Drouvot
Reviewed-by: Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 16

15 months agodoc: Use system-username instead of system-user
Michael Paquier [Tue, 20 Feb 2024 02:59:10 +0000 (11:59 +0900)]
doc: Use system-username instead of system-user

This inconsistency has been introduced in efb6f4a4f9b6.

Reported-by: Julien Rouhaud
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 16

15 months agoFix incorrect pruning of NULL partition for boolean IS NOT clauses
David Rowley [Mon, 19 Feb 2024 23:50:09 +0000 (12:50 +1300)]
Fix incorrect pruning of NULL partition for boolean IS NOT clauses

Partition pruning wrongly assumed that, for a table partitioned on a
boolean column, a clause in the form "boolcol IS NOT false" and "boolcol
IS NOT true" could be inverted to correspondingly become "boolcol IS true"
and "boolcol IS false".  These are not equivalent as the NOT version
matches the opposite boolean value *and* NULLs.  This incorrect assumption
meant that partition pruning pruned away partitions that could contain
NULL values.

Here we fix this by correctly not pruning partitions which could store
NULLs.

To be affected by this, the table must be partitioned by a NULLable boolean
column and queries would have to contain "boolcol IS NOT false" or "boolcol
IS NOT true".  This could result in queries filtering out NULL values
with a LIST partitioned table and "ERROR:  invalid strategy number 0"
for RANGE and HASH partitioned tables.

Reported-by: Alexander Lakhin
Bug: #18344
Discussion: https://postgr.es/m/18344-8d3f00bada6d09c6@postgresql.org
Backpatch-through: 12

15 months agoFix test race between primary XLOG_RUNNING_XACTS and standby logical slot.
Noah Misch [Mon, 19 Feb 2024 20:52:28 +0000 (12:52 -0800)]
Fix test race between primary XLOG_RUNNING_XACTS and standby logical slot.

Before the previous commit, the test could hang until
LOG_SNAPSHOT_INTERVAL_MS (15s), until checkpoint_timeout (300s), or
indefinitely.  An indefinite hang was awfully improbable.  It entailed
the test reaching checkpoint_timeout before the
DecodingContextFindStartpoint() of a CREATE SUBSCRIPTION, yet after the
preceding WAL record.  Back-patch to v16, which introduced the test.

Bertrand Drouvot, reported by Noah Misch.

Discussion: https://postgr.es/m/20240211010227[email protected]

15 months agoBound waits in 035_standby_logical_decoding.pl.
Noah Misch [Mon, 19 Feb 2024 20:52:07 +0000 (12:52 -0800)]
Bound waits in 035_standby_logical_decoding.pl.

One IPC::Run::start() used an IPC::Run::timer() without checking for
expiration.  The other used no timeout or timer.  Back-patch to v16,
which introduced the test.

Reviewed by Bertrand Drouvot.

Discussion: https://postgr.es/m/20240211010227[email protected]

15 months agoDoc: fix typo in SECURITY LABEL synopsis.
Tom Lane [Mon, 19 Feb 2024 19:17:11 +0000 (14:17 -0500)]
Doc: fix typo in SECURITY LABEL synopsis.

One case missed its trailing "|".

Reported by Tim Needham.

Discussion: https://postgr.es/m/170833547220.3279712.700702770281879175@wrigleys.postgresql.org