Skip some permissions checks on Cygwin

These are checks that are already skipped on other Windows systems.

Backpatch to all live branches, as appropriate.

Fix infer_arbiter_indexes() to not assume resultRelation is 1.

infer_arbiter_indexes failed to renumber varnos in index expressions
or predicates that it got from the catalogs.  This escaped detection
up to now because the stored varnos in such trees will be 1, and an
INSERT's result relation is usually the first rangetable entry,
so that that was fine.  However, in cases such as inserting through
an updatable view, it's not fine, leading to failure to match the
expressions to the query with ensuing "there is no unique or exclusion
constraint matching the ON CONFLICT specification" errors.

Fix by copy-and-paste from get_relation_info().

Per bug #18502 from Michael Wang.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/18502-545b53f5b81e54e0@postgresql.org

Fix creation of partition descriptor during concurrent detach

When a partition is being detached in concurrent mode, it is possible
for find_inheritance_children_extended() to return that partition in the
list, and immediately after that receive an invalidation message that
sets its relpartbound to NULL just before we read it.  (This can happen
because table_open() reads invalidation messages.)  Currently we raise
an error
  ERROR:  missing relpartbound for relation %u
about the situation, but that's bogus because the table is no longer a
partition, so we shouldn't be complaining about it.  A better reaction
is to retry the find_inheritance_children_extended call to get a new
list, which will no longer have the partition being detached.

Noticed while investigating bug #18377.

Backpatch to 14, where DETACH CONCURRENTLY appeared.

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

Doc: Fix ambuiguity in column lists.

The behavior for columns added later to the table for publications with no
specified column lists was not clear.

Reported-by: Koen De Groote
Author: Peter Smith
Reviewed-by: Vignesh C, Laurenz Albe
Backpatch-through: 15
Discussion: https://postgr.es/m/171621878740.686.11325940592820985181@wrigleys.postgresql.org

Tighten test_predtest's input checks, and improve error messages.

test_predtest() neglected to consider the possibility that
SPI_plan_get_cached_plan would return NULL.  This led to a core
dump if the input (incorrectly) contains more than one SQL
command.

While here, let's expend more than zero effort on the error
message for this case and nearby ones.

Per (half of) bug #18483 from Alexander Kozhemyakin.
Back-patch to all supported branches, not because this is
very significant (it's merely test scaffolding) but to make
our world a bit safer for fuzz testing.

Discussion: https://postgr.es/m/18483-30bfff42de238000@postgresql.org

Reject modifying a temp table of another session with ALTER TABLE.

Normally this case isn't even reachable by non-superusers, since
permissions checks prevent naming such a table.  However, it is
possible to make it happen by altering a parent table whose child
is another session's temp table.

We definitely can't support any such ALTER that requires modifying
the contents of such a table, since we lack access to the other
session's temporary-buffer pool.  But there seems no good reason
to allow it even if it'd only require changing catalog contents.
One reason not to allow it is that we'd rather not expose the
implementation-dependent behavior of whether a specific ALTER
requires touching the table contents.  Another is that there may
be (in future, even if not today) optimizations that assume that
a session's own temp tables won't be modified by other sessions.

Hence, add a RELATION_IS_OTHER_TEMP() check to all the places
where ALTER TABLE currently does CheckTableNotInUse().  (I looked
through all other callers of CheckTableNotInUse(), and they seem
OK already.)

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

Discussion: https://postgr.es/m/18492-c7a2634bf4968763@postgresql.org

Fix behavior of stable functions called from a CALL's argument list.

If the CALL is within an atomic context (e.g. there's an outer
transaction block), _SPI_execute_plan should acquire a fresh snapshot
to execute any such functions with.  We failed to do that and instead
passed them the Portal snapshot, which had been acquired at the start
of the current SQL command.  This'd lead to seeing stale values of
rows modified since the start of the command.

This is arguably a bug in 84f5c2908: I failed to see that "are we in
non-atomic mode" needs to be defined the same way as it is further
down in _SPI_execute_plan, i.e. check !_SPI_current->atomic not just
options->allow_nonatomic.  Alternatively the blame could be laid on
plpgsql, which is unconditionally passing allow_nonatomic = true
for CALL/DO even when it knows it's in an atomic context.  However,
fixing it in spi.c seems like a better idea since that will also fix
the problem for any extensions that may have copied plpgsql's coding
pattern.

While here, update an obsolete comment about _SPI_execute_plan's
snapshot management.

Per report from Victor Yegorov.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/CAGnEboiRe+fG2QxuBO2390F7P8e2MQ6UyBjZSL_w1Cej+E4=Vw@mail.gmail.com

Add more debugging information when dropping twice pgstats entry

Floris Van Nee has reported a bug in the pgstats facility where a stats
entry already dropped would get again dropped.  This case should not
happen, still the error generated did not offer any details about the
stats entry getting dropped.

This commit improves the error message generated to inform about the
stats entry kind, database OID, object OID and refcount, which should
help to debug more the problem reported.  Bertrand Drouvot has been
independently able to reach this error path while writing a new feature,
and more details about the failure would have been helpful for
debugging.

Author: Andres Freund, Bertrand Drouvot
Discussion: https://postgr.es/m/20240505160915[email protected]
Discussion: https://postgr.es/m/ZkM30paAD8Cr/[email protected]
Backpatch-through: 15

postgres_fdw: Refuse to send FETCH FIRST WITH TIES to remote servers.

Previously, when considering LIMIT pushdown, postgres_fdw failed to
check whether the query has this clause, which led to pushing false
LIMIT clauses, causing incorrect results.

This clause has been supported since v13, so we need to do a
remote-version check before deciding that it will be safe to push such a
clause, but we do not currently have a way to do the check (without
accessing the remote server); disable pushing such a clause for now.

Oversight in commit 357889eb1.  Back-patch to v13, where that commit
added the support.

Per bug #18467 from Onder Kalaci.

Patch by Japin Li, per a suggestion from Tom Lane, with some changes to
the comments by me.  Review by Onder Kalaci, Alvaro Herrera, and me.

Discussion: https://postgr.es/m/18467-7bb89084ff03a08d%40postgresql.org

doc: Fix copy-and-paste mistake

The wording from the "columns" view was copied to the "attributes"
view without the required adjustments.

Fix failure with SQL-procedure polymorphic output arguments in v12.

Before the v13-era commit 913bbd88d, check_sql_fn_retval fails to
resolve polymorphic output types and then just throws up its hands and
assumes the check will be made at runtime.  I think that's true for
ordinary functions returning RECORD, but it doesn't happen in CALL,
potentially resulting in crashes if the actual output of the SQL
procedure's SELECT doesn't match the type inferred from polymorphism.
With a little bit of rearrangement, we can use get_call_result_type
instead of get_func_result_type and thereby infer the correct types.

I'm still unwilling to back-patch all of 913bbd88d, so if the types
don't match you'll get an error rather than perhaps silently inserting
a cast as v13 and later can.  That's consistent with prior behavior
though, so it seems fine.

Prior to 70ffb27b2, you'd typically get other errors due to other
shortcomings of CALL's management of polymorphism.  Nonetheless,
this is an independent bug.

Although there is no bug in v13 and up, it seems prudent to add
the test case for this to the newer branches too.  It's clearly
an under-tested area.

Per report from Andrew Bille.

Discussion: https://postgr.es/m/CAJnzarw9EeWHAQRm76dXd=7j+rgw6ERqC=nCay8jeFqTwKwhqQ@mail.gmail.com

Prevent inconsistent use of stats entry for replication slots

Concurrent activity around replication slot creation and drop could
cause a replication slot to use a stats entry it should not have used
when created, triggering an assertion failure when retrieving this
inconsistent entry from the dshash table used by the stats facility.

The issue is that pgstat_drop_replslot() calls pgstat_drop_entry()
without checking the result.  If pgstat_drop_entry() cannot free the
entry related to the object dropped, pgstat_request_entry_refs_gc()
should be called.  AtEOXact_PgStat_DroppedStats() and surrounding
routines dropping stats entries already do that.

This is documented in pgstat_internal.h, but let's add a comment at the
top of pgstat_drop_entry() as that can be easy to miss.

Reported-by: Alexander Lakhin
Author: Floris Van Nee
Analyzed-by: Andres Freund
Discussion: https://postgr.es/m/17947-b9554521ad963c9c@postgresql.org
Backpatch-through: 15

Fix documentation for POSIX semaphores.

The documentation for POSIX semaphores is missing a reference to
max_wal_senders.  This commit fixes that in the same way that
commit 4ebe51a5fb fixed the same issue in the documentation for
System V semaphores.

Discussion: https://postgr.es/m/20240517164452.GA1914161%40nathanxps13
Backpatch-through: 12

Fix pl/tcl's handling of errors from Tcl_ListObjGetElements().

In a procedure or function returning tuple, we use that function to
parse the Tcl script's result, which is supposed to be a Tcl list.
If it isn't, you get an error.  Commit 26abb50c4 incautiously
supposed that we could use throw_tcl_error() to report such an error.
That doesn't actually work, because low-level functions like
Tcl_ListObjGetElements() don't fill Tcl's errorInfo variable.
The result is either a null-pointer-dereference crash or emission
of misleading context information describing the previous Tcl error.

Back off to just reporting the interpreter's result string, and
improve throw_tcl_error()'s comment to explain when to use it.

Also, although the similar code in pltcl_trigger_handler() avoided
this mistake, it was using a fairly confusing wording of the
error message.  Improve that while we're here.

Per report from A. Kozhemyakin.  Back-patch to all supported
branches.

Erik Wienhold and Tom Lane

Discussion: https://postgr.es/m/6a2a1c40-2b2c-4a33-8b72-243c0766fcda@postgrespro.ru

ci: windows: Use the same image for VS and MinGW tasks

The VS and MinGW Windows images have been merged, to reduce the space needed
for images. Before 98811323c8e the split helped boot performance, but now that
we are using VMs that doesn't appear to be the case anymore.

Author: Nazir Bilal Yavuz 
Discussion: https://postgr.es/m/CAN55FZ2kWYjPd7uUC5QswrB3tfVJDiURqC%2BMGM6a3oeev%3DVgOA%40mail.gmail.com
Backpatch: 15-, where CI was added

Fix documentation for System V semaphores.

The formulas for SEMMNI and SEMMNS do not include the archiver
process, which was converted to an auxiliary process in v14, and
the WAL summarizer process, which was introduced in v17.  This
commit corrects these formulas and adds a missing reference to
max_wal_senders nearby.  Since this section of the documentation
tends to be incorrect quite often, we should likely give up on
documenting the exact formulas in favor of something less fragile,
but that is left as a future exercise.

Reported-by: Sami Imseih
Reviewed-by: Sami Imseih
Discussion: https://postgr.es/m/20240517164452.GA1914161%40nathanxps13
Backpatch-through: 12

Improve stability of subscription/029_on_error.pl

This test was failing when using wal_debug=on and -DWAL_DEBUG because of
additional log entries that made the test grab an LSN not mapping with
the error expected in the test.

Previously the test would look for the first matching line to get the
LSN to skip up to.  This is improved by having the test scan the logs
with a regexp that checks for the expected ERROR string, ensuring that
the wanted LSN comes from the correct context.

Backpatch down to 15 where this test has been introduced.

Author: Ian Ilyasov
Discussion: https://postgr.es/m/GV1P251MB100415F17E6B2FDD7188777ECDE32@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM
Backpatch-through: 15

Remove race conditions between ECPGdebug() and ecpg_log().

Coverity complains that ECPGdebug is accessing debugstream without
holding debug_mutex, which is a fair complaint: we should take
debug_mutex while changing the settings ecpg_log looks at.

In some branches it also complains about unlocked use of simple_debug.
I think it's intentional and safe to have a quick unlocked check of
simple_debug at the start of ecpg_log, since that early exit will
always be taken in non-debug cases.  But we should recheck
simple_debug after acquiring the mutex.  In the worst case, calling
ECPGdebug concurrently with ecpg_log in another thread could result
in a null-pointer dereference due to debugstream transiently being
NULL while simple_debug isn't 0.

This is largely hypothetical, since it's unlikely anybody uses
ECPGdebug() at all in the field, and our own regression tests
don't seem to be hitting the theoretical race conditions either.
Still, if we're going to the trouble of having mutexes here, we ought
to be using them in a way that's actually safe not just almost safe.
Hence, back-patch to all supported branches.

doc: Fix column_name parameter in ALTER MATERIALIZED VIEW

Parameter column_name must be an existing column because ALTER
MATERIALIZED VIEW cannot add new columns.  The old description was
likely copied from ALTER TABLE.

Author: Erik Wienhold
Discussion: https://postgr.es/m/6880ca53-7961-4eeb-86d5-6bd05fc2027e@ewie.name
Backpatch-through: 12

Fix handling of extended expression statistics in CREATE TABLE LIKE.

transformTableLikeClause believed that it could process extended
statistics immediately because "the representation of CreateStatsStmt
doesn't depend on column numbers".  That was true when extended stats
were first introduced, but it was falsified by the addition of
extended stats on expressions: the parsed expression tree is fed
forward by the LIKE option, and that will contain Vars.  So if the
new table doesn't have attnums identical to the old one's (typically
because there are some dropped columns in the old one), that doesn't
work.  The CREATE goes through, but it emits invalid statistics
objects that will cause problems later.

Fortunately, we already have logic that can adapt expression trees
to the possibly-new column numbering.  To use it, we have to delay
processing of CREATE_TABLE_LIKE_STATISTICS into expandTableLikeClause,
just as for other LIKE options that involve expressions.

Per bug #18468 from Alexander Lakhin.  Back-patch to v14 where
extended statistics on expressions were added.

Discussion: https://postgr.es/m/18468-f5add190e3fa5902@postgresql.org

Account for optimized MinMax aggregates during SS_finalize_plan.

We are capable of optimizing MIN() and MAX() aggregates on indexed
columns into subqueries that exploit the index, rather than the normal
thing of scanning the whole table.  When we do this, we replace the
Aggref node(s) with Params referencing subquery outputs.  Such Params
really ought to be included in the per-plan-node extParam/allParam
sets computed by SS_finalize_plan.  However, we've never done so
up to now because of an ancient implementation choice to perform
that substitution during set_plan_references, which runs after
SS_finalize_plan, so that SS_finalize_plan never sees these Params.

The cleanest fix would be to perform a separate tree walk to do
these substitutions before SS_finalize_plan runs.  That seems
unattractive, first because a whole-tree mutation pass is expensive,
and second because we lack infrastructure for visiting expression
subtrees in a Plan tree, so that we'd need a new function knowing
as much as SS_finalize_plan knows about that.  I also considered
swapping the order of SS_finalize_plan and set_plan_references,
but that fell foul of various assumptions that seem tricky to fix.
So the approach adopted here is to teach SS_finalize_plan itself
to check for such Aggrefs.  I refactored things a bit in setrefs.c
to avoid having three copies of the code that does that.

Back-patch of v17 commits d0d44049d and 779ac2c74.  When d0d44049d
went in, there was no evidence that it was fixing a reachable bug,
so I refrained from back-patching.  Now we have such evidence.

Per bug #18465 from Hal Takahara.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18465-2fae927718976b22@postgresql.org
Discussion: https://postgr.es/m/2391880.1689025003@sss.pgh.pa.us

Fix documentation about DROP DATABASE FORCE process termination rights.

Specifically, it terminates a background worker even if the caller
couldn't terminate the background worker with pg_terminate_backend().
Commit 3a9b18b3095366cd0c4305441d426d04572d88c1 neglected to update
this.  Back-patch to v13, which introduced DROP DATABASE FORCE.

Reviewed by Amit Kapila.  Reported by Kirill Reshke.

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

Fix query result leak during binary upgrade

9a974cbcba00 moved the query in binary_upgrade_set_pg_class_oids to the
outer level, but left the PQclear and query buffer destruction in the
is_index conditional.  353708e1fb2d fixed the leak of the query buffer
but left the PGresult leak. This moves clearing the result to the outer
level ensuring that it will be called.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/374550C1-F4ED-4D9D-9498-0FD029CCF674@yesql.se
Backpatch-through: v15

doc: Remove claims that initdb and pg_ctl use libpq environment variables

Erroneously introduced by 571df93cff8.

Reviewed-by: Daniel Gustafsson
Discussion: https://www.postgresql.org/message-id/flat/8458c9c5-18f1-46d7-94c4-1c30e4f44908%40eisentraut.org

Fix handling of polymorphic output arguments for procedures.

Most of the infrastructure for procedure arguments was already
okay with polymorphic output arguments, but it turns out that
CallStmtResultDesc() was a few bricks shy of a load here.  It thought
all it needed to do was call build_function_result_tupdesc_t, but
that function specifically disclaims responsibility for resolving
polymorphic arguments.  Failing to handle that doesn't seem to be
a problem for CALL in plpgsql, but CALL from plain SQL would get
errors like "cannot display a value of type anyelement", or even
crash outright.

In v14 and later we can simply examine the exposed types of the
CallStmt.outargs nodes to get the right type OIDs.  But it's a lot
more complicated to fix in v12/v13, because those versions don't
have CallStmt.outargs, nor do they do expand_function_arguments
until ExecuteCallStmt runs.  We have to duplicatively run
expand_function_arguments, and then re-determine which elements
of the args list are output arguments.

Per bug #18463 from Drew Kimball.  Back-patch to all supported
versions, since it's busted in all of them.

Discussion: https://postgr.es/m/18463-f8cd77e12564d8a2@postgresql.org

Fix pg_sequence_last_value() for unlogged sequences on standbys.

Presently, when this function is called for an unlogged sequence on
a standby server, it will error out with a message like

ERROR:  could not open file "base/5/16388": No such file or directory

Since the pg_sequences system view uses pg_sequence_last_value(),
it can error similarly.  To fix, modify the function to return NULL
for unlogged sequences on standby servers.  Since this bug is
present on all versions since v15, this approach is preferable to
making the ERROR nicer because we need to repair the pg_sequences
view without modifying its definition on released versions.  For
consistency, this commit also modifies the function to return NULL
for other sessions' temporary sequences.  The pg_sequences view
already appropriately filters out such sequences, so there's no bug
there, but we might as well offer some defense in case someone
invokes this function directly.

Unlogged sequences were first introduced in v15, but temporary
sequences are much older, so while the fix for unlogged sequences
is only back-patched to v15, the temporary sequence portion is
back-patched to all supported versions.

We could also remove the privilege check in the pg_sequences view
definition in v18 if we modify this function to return NULL for
sequences for which the current user lacks privileges, but that is
left as a future exercise for when v18 development begins.

Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Backpatch-through: 12

Fix recursive RECORD-returning plpython functions.

If we recursed to a new call of the same function, with a different
coldeflist (AS clause), it would fail because the inner call would
overwrite the outer call's idea of what to return.  This is vaguely
like 1d2fe56e4 and c5bec5426, but it's not due to any API decisions:
it's just that we computed the actual output rowtype at the start of
the call, and saved it in the per-procedure data structure.  We can
fix it at basically zero cost by doing the computation at the end
of each call instead of the start.

It's not clear that there's any real-world use-case for such a
function, but given that it doesn't cost anything to fix,
it'd be silly not to.

Per report from Andreas Karlsson.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/1651a46d-3c15-4028-a8c1-d74937b54e19@proxel.se

Fix overread in JSON parsing errors for incomplete byte sequences

json_lex_string() relies on pg_encoding_mblen_bounded() to point to the
end of a JSON string when generating an error message, and the input it
uses is not guaranteed to be null-terminated.

It was possible to walk off the end of the input buffer by a few bytes
when the last bytes consist of an incomplete multi-byte sequence, as
token_terminator would point to a location defined by
pg_encoding_mblen_bounded() rather than the end of the input.  This
commit switches token_terminator so as the error uses data up to the
end of the JSON input.

More work should be done so as this code could rely on an equivalent of
report_invalid_encoding() so as incorrect byte sequences can show in
error messages in a readable form.  This requires work for at least two
cases in the JSON parsing API: an incomplete token and an invalid escape
sequence.  A more complete solution may be too invasive for a backpatch,
so this is left as a future improvement, taking care of the overread
first.

A test is added on HEAD as test_json_parser makes this issue
straight-forward to check.

Note that pg_encoding_mblen_bounded() no longer has any callers.  This
will be removed on HEAD with a separate commit, as this is proving to
encourage unsafe coding.

Author: Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com
Backpatch-through: 13

Ensure that "pg_restore -l" reports dependent TOC entries correctly.

If -l was specified together with selective-restore options such as -n
or -N, dependent TOC entries such as comments would be omitted from
the listing, even when an actual restore would have selected them.
This happened because PrintTOCSummary neglected to update the te->reqs
marking of the entry they depended on.

Per report from Justin Pryzby.  This has been wrong since 0d4e6ed30
taught _tocEntryRequired to sometimes look at the "reqs" marking of
other TOC entries, so back-patch to all supported branches.

Discussion: https://postgr.es/m/ZjoeirG7yxODdC4P@pryzbyj2023

Don't corrupt plpython's "TD" dictionary in a recursive trigger call.

If a plpython-language trigger caused another one to be invoked,
the "TD" dictionary created for the inner one would overwrite the
outer one's "TD" dictionary.  This is more or less the same problem
that 1d2fe56e4 fixed for ordinary functions in plpython, so fix it
the same way, by saving and restoring "TD" during a recursive
invocation.

This fix makes an ABI-incompatible change in struct PLySavedArgs.
I'm not too worried about that because it seems highly unlikely that
any extension is messing with those structs.  We could imagine doing
something weird to preserve nominal ABI compatibility in the back
branches, like keeping the saved TD object in an extra element of
namedargs[].  However, that would only be very nominal compatibility:
if anything *is* touching PLySavedArgs, it would likely do the wrong
thing due to not knowing about the additional value.  So I judge it
not worth the ugliness to do something different there.

(I also changed struct PLyProcedure, but its added field fits
into formerly-padding space, so that should be safe.)

Per bug #18456 from Jacques Combrink.  This bug is very ancient,
so back-patch to all supported branches.

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

Stamp 15.7.

Last-minute updates for release notes.

Security: CVE-2024-4317

Fix privilege checks in pg_stats_ext and pg_stats_ext_exprs.

The catalog view pg_stats_ext fails to consider privileges for
expression statistics.  The catalog view pg_stats_ext_exprs fails
to consider privileges and row-level security policies.  To fix,
restrict the data in these views to table owners or roles that
inherit privileges of the table owner.  It may be possible to apply
less restrictive privilege checks in some cases, but that is left
as a future exercise.  Furthermore, for pg_stats_ext_exprs, do not
return data for tables with row-level security enabled, as is
already done for pg_stats_ext.

On the back-branches, a fix-CVE-2024-4317.sql script is provided
that will install into the "share" directory.  This file can be
used to apply the fix to existing clusters.

Bumps catversion on 'master' branch only.

Reported-by: Lukas Fittl
Reviewed-by: Noah Misch, Tomas Vondra, Tom Lane
Security: CVE-2024-4317
Backpatch-through: 14

Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 141c2cc465bc7bd1e2d43243cf81215b0b14abd4

Release notes for 16.3, 15.7, 14.12, 13.15, 12.19.

Throw a more on-point error for publications depending on columns.

Same as 42b041243, except that the trouble case is a publication
WHERE clause that depends on a column.

Again reported by Alexander Lakhin.  Back-patch to v15 where
we added publication WHERE clauses.

Discussion: https://postgr.es/m/548a47bc-87ae-b3df-c6a2-60b9966f808b@gmail.com

doc: Fix description of deterministic flag of CREATE COLLATION

The documentation said that you need to pick a suitable LC_COLLATE
setting in addition to setting the DETERMINISTIC flag.  This would
have been correct if the libc provider supported nondeterministic
collations, but since it doesn't, you actually need to set the LOCALE
option.

Reviewed-by: Kashif Zeeshan
Discussion: https://www.postgresql.org/message-id/flat/a71023c2-0ae0-45ad-9688-cf3b93d0d65b%40eisentraut.org

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)

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

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

Disallow converting a table to a view within an outer SQL command.

We have long disallowed all forms of ALTER TABLE if the table is
already opened by some outer SQL command in the same session.
This has the same purpose as obtaining AccessExclusiveLock, but
since a session's own locks don't conflict the lock only blocks use
of the table by other sessions, not our own.  Without this check,
the ALTER might confuse the outer SQL command since any previous
inspection of the table would potentially become invalid.

However, the RelisBecomingView code path in DefineQueryRewrite never
got that memo, and assumed that AccessExclusiveLock is sufficient
for performing something morally equivalent to a rather invasive
ALTER TABLE.  Unsurprisingly, this can confuse an outer command
that is trying to do something with the table.

This was submitted as a security issue, but the security team
has been unable to identify any consequence worse than a null
pointer dereference (from trying to access rd_tableam methods
that the relation no longer has).  Therefore, in accordance
with our usual policy, it's not security material and should
just be fixed as a routine bug.

Fix by disallowing the operation if the table is open locally,
exactly as ALTER TABLE does it.

Per an anonymous security researcher, via Bundesamt für Sicherheit
in der Informationstechnik.

Patch v12-v15 only.  In v16 and later, we removed this code
altogether (cf. commit b23cd185f), so that there's no issue.

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]

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

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

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

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

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

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

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

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.

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

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

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

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

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

Fix generation of EC join conditions at the wrong plan level.

get_baserel_parampathinfo previously assumed without checking that
the results of generate_join_implied_equalities "necessarily satisfy
join_clause_is_movable_into".  This turns out to be wrong in the
presence of outer joins, because the generated clauses could include
Vars that mustn't be evaluated below a relevant outer join.  That
led to applying clauses at the wrong plan level and possibly getting
incorrect query results.  We must check each clause's nullable_relids,
and really the right thing to do is test join_clause_is_movable_into.

However, trying to fix it that way exposes an oversight in
equivclass.c: it wasn't careful about marking join clauses for
appendrel children with the correct clause_relids.  That caused the
modified get_baserel_parampathinfo code to reject some clauses it
still needs to accept.  (See parallel commit for HEAD/v16 for more
commentary about that.)

Per bug #18429 from Benoît Ryder.  This misbehavior existed for
a long time before commit 2489d76c4, so patch v12-v15 this way.

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

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

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

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

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

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

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

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

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

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

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

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.

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-

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

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

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

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

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

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

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

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.

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

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

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

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

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.

Clarify comment for LogicalTapeSetBlocks().

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

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

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

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

Review wording on tablespaces w.r.t. partitioned tables

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

After commits a36c84c3e4a9, 87259588d0ab and others.

Backpatch to 12.

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

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]

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

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

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

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

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

doc:  add missing word "the"

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

Backpatch-through: 12

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

Backpatch missing check_stack_depth() to some recursive functions

Backpatch changes from d57b7cc333, 75bcba6cbd to all supported branches per
proposal of Egor Chindyaskin.

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

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

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

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

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

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