Adjust the header comment in get_agg_clause_costs so that it matches what
the function currently does.  No recursive searching has been done ever
since 0a2bc5d6.  It also does not determine the aggtranstype like the
comment claimed. That's all done in preprocess_aggref().
preprocess_aggref also now determines the numOrderedAggs, so remove the
mention that get_agg_clause_costs also calculates "counts".

Normally, since this is just an adjustment of a comment it might not be
worth back-patching, but since this code is new to PG14 and that version
is still in beta, then it seems worth having the comments match.

Discussion: https://postgr.es/m/CAApHDvrrGrTJFPELrjx0CnDtz9B7Jy2XYW3Z2BKifAWLSaJYwQ@mail.gmail.com
Backpatch-though: 14

These have been introduced by 7fbe0c8, and could happen for
pg_basebackup and pg_receivewal.

Per report from Coverity for the ones in walmethods.c, I have spotted
the ones in receivelog.c after more review.

Backpatch-through: 10

The point of introducing the hash_mem_multiplier GUC was to let users
reproduce the old behavior of hash aggregation, i.e. that it could use
more than work_mem at need.  However, the implementation failed to get
the job done on Win64, where work_mem is clamped to 2GB to protect
various places that calculate memory sizes using "long int".  As
written, the same clamp was applied to hash_mem.  This resulted in
severe performance regressions for queries requiring a bit more than
2GB for hash aggregation, as they now spill to disk and there's no
way to stop that.

Getting rid of the work_mem restriction seems like a good idea, but
it's a big job and could not conceivably be back-patched.  However,
there's only a fairly small number of places that are concerned with
the hash_mem value, and it turns out to be possible to remove the
restriction there without too much code churn or any ABI breaks.
So, let's do that for now to fix the regression, and leave the
larger task for another day.

This patch does introduce a bit more infrastructure that should help
with the larger task, namely pg_bitutils.h support for working with
size_t values.

Per gripe from Laurent Hasson.  Back-patch to v13 where the
behavior change came in.

Discussion: https://postgr.es/m/997817.1627074924@sss.pgh.pa.us
Discussion: https://postgr.es/m/MN2PR15MB25601E80A9B6D1BA6F592B1985E39@MN2PR15MB2560.namprd15.prod.outlook.com

5a1e1d83 was a minimal bug fix for dc7420c2. To avoid future bugs of
that kind, deduplicate the choice of a relation's horizon into a new helper,
GlobalVisHorizonKindForRel().

As the code in question was only introduced in dc7420c2 it seems worth
backpatching this change as well, otherwise 14 will look different from all
other branches.

A different approach to this was suggested by Matthias van de Meent.

Author: Andres Freund
Discussion: https://postgr.es/m/20210621122919.2qhu3pfugxxp3cji@alap3.anarazel.de
Backpatch: 14, like 5a1e1d83

We have an implementation restriction that PREPARE TRANSACTION can't
handle cases where both session-lifespan and transaction-lifespan locks
are held on the same lockable object.  (That's because we'd otherwise
need to acquire a new PROCLOCK entry during post-prepare cleanup, which
is an operation that might fail.  The situation can only arise with odd
usages of advisory locks, so removing the restriction is probably not
worth the amount of effort it would take.)  AtPrepare_Locks attempted
to enforce this, but its logic was many bricks shy of a load, because
it only detected cases where the session and transaction locks had the
same lockmode.  Locks of different modes on the same object would lead
to the rather unhelpful message "PANIC: we seem to have dropped a bit
somewhere".

To fix, build a transient hashtable with one entry per locktag,
not one per locktag + mode, and use that to detect conflicts.

Per bug #17122 from Alexander Pyhalov.  This bug is ancient,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/17122-04f3c32098a62233@postgresql.org

We previously took a hard-line attitude that callers should never print
a null string pointer, and doing so is worthy of an assertion failure
or crash.  However, we've long since flushed out any easy-to-find bugs
of that nature.  What remains is a lot of code that perhaps could fail
that way in hard-to-reach corner cases.  For example, in something as
simple as
    ereport(ERROR,
            (errcode(ERRCODE_UNDEFINED_OBJECT),
             errmsg("constraint \"%s\" for table \"%s\" does not exist",
                    conname, get_rel_name(relid))));
one must wonder whether it's completely guaranteed that get_rel_name
cannot return NULL in this context.  If such a situation did occur,
the existing policy converts what might be a pretty minor bug into
a server crash condition.  This is not good for robustness.

Hence, let's follow the lead of glibc and print "(null)" instead
of failing.  We should, of course, still consider it a bug if that
behavior is reachable in ordinary use; but crashing seems less
desirable than not crashing.

This fix works across-the-board in v12 and up, where we always use
src/port/snprintf.c.  Before that, on most platforms we're at the mercy
of the local libc, but it appears that Solaris 10 is the only supported
platform where we'd still get a crash.  Most other platforms such as
*BSD, macOS, and Solaris 11 have adopted glibc's behavior at some
point.  (AIX and HPUX just print "" not "(null)", but that's close
enough.)  I've not checked what Windows' native printf would do, but
it doesn't matter because we've long used snprintf.c on that platform.

In v12 and up, also const-ify related code so that we're not casting
away const on the constant string.  This is just neatnik-ism, since
next to no compilers will warn about that.

Discussion: https://postgr.es/m/17098-b960f3616c861f83@postgresql.org

This testing was useful when it was written, nigh twenty years ago,
but it seems fairly pointless for any platform built in the last
dozen or more years.  (Compare also the comments at 8a212118.)
Also we now have reports that the test program itself fails under
ThreadSanitizer.  Rather than invest effort in fixing it, let's
just drop it, and assume that the few people who still care
already know they need to use --disable-thread-safety.

Back-patch into v14, for consistency with 8a212118.

Discussion: https://postgr.es/m/CADhDkKzPSiNvA3Hyq+wSR_icuPmazG0cFe=YnC3U-CFcYLc8Xw@mail.gmail.com

Bauyrzhan Sakhariyev, via Github

Backpatch to v14

Code inlined by LLVM can crash or fail with "Relocation type not
implemented yet!" if it tries to access thread local variables.  Don't
inline such code.

Back-patch to 11, where LLVM arrived.  Bug #16696.

Author: Dmitry Marakasov <amdmi3@amdmi3.ru>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/16696-29d944a33801fbfe@postgresql.org

Now that we're not having to wedge this into the straitjacket of
the old operator table format, we can add another example to
clarify the point about left-to-right associativity.

Per suggestion from mdione at grulic.org.ar.

https://postgr.es/m/162661954599.693.13700316547731859171@wrigleys.postgresql.org

In postgresql.conf, memory and file size GUCs can be specified with "B"
(bytes) as of b06d8e58. Likewise, time GUCs can be specified with "us"
(microseconds) as of caf626b2. Update postgres.conf.sample to reflect
that fact.

Pavel Luzanov

Backpatch to v12, which is the earliest version that allows both of
these units. A separate commit will document the "B" case for v11.

Discussion: https://www.postgresql.org/message-id/flat/f10d16fc-8fa0-1b3c-7371-cb3a35a13b7a%40postgrespro.ru

Oversight in ea1b99a6

Yukun Wang

Backpatch to v14 where this parameter was introduced
Discussion: https://www.postgresql.org/message-id/flat/OS0PR01MB6003FCEFF0201EF21685FD33B4E39%40OS0PR01MB6003.jpnprd01.prod.outlook.com

Back-patch to v10 where pg_import_system_collations() was added.

Author: Atsushi Torikoshi
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/b7f484692a3e283710032e68b7f40617@oss.nttdata.com

Reported-by: Elena Indrupskaya

Discussion: https://postgr.es/m/38555778-a56b-4aca-2581-e05582fc9bcf@postgrespro.ru

Author: Elena Indrupskaya

Backpatch-through: 14 only

If an error was raised during our initial attempt to check whether
a successfully-compiled expression is "simple", subsequent calls of
exec_stmt_execsql would suppose that stmt->mod_stmt was already computed
when it had not been.  This could lead to assertion failures in debug
builds; in production builds the effect would typically be to act as
if INTO STRICT had been specified even when it had not been.  Of course
that only matters if the subsequent attempt to execute the expression
succeeds, so that the problem can only be reached by fixing a failure
in some referenced, inline-able SQL function and then retrying the
calling plpgsql function in the same session.

(There might be even-more-obscure ways to change the expression's
behavior without changing the plpgsql function, but that one seems
like the only one people would be likely to hit in practice.)

The most foolproof way to fix this would be to arrange for
exec_prepare_plan to not set expr->plan until we've finished the
subsidiary simple-expression check.  But it seems hard to do that
without creating reference-count leak issues.  So settle for documenting
the hazard in a comment and fixing exec_stmt_execsql to test separately
for whether it's computed stmt->mod_stmt.  (That adds a test-and-branch
per execution, but hopefully that's negligible in context.)  In v11 and
up, also fix exec_stmt_call which had a variant of the same issue.

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

Discussion: https://postgr.es/m/17113-077605ce00e0e7ec@postgresql.org

The logic handling the opening of new WAL segments was fuzzy when using
--compress if a partial, non-compressed, segment with the same base name
existed in the repository storing those files.  In this case, using
--compress would cause the code to first check for the existence and the
size of a non-compressed segment, followed by the opening of a new
compressed, partial, segment.  The code was accidentally working
correctly on most platforms as the buildfarm has proved, except
bowerbird where gzflush() could fail in this code path.  It is wrong
anyway to take the code path used pre-padding when creating a new
partial, non-compressed, segment, so let's fix it.

Note that this issue exists when users mix successive runs of
pg_receivewal with or without compression, as discovered with the tests
introduced by ffc9dda.

While on it, this refactors the code so as code paths that need to know
about the ".gz" suffix are down from four to one in walmethods.c, easing
a bit the introduction of new compression methods.  This addresses a
second issue where log messages generated for an unexpected failure
would not show the compressed segment name involved, which was
confusing, printing instead the name of the non-compressed equivalent.

Reported-by: Georgios Kokolatos
Discussion: https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyz
Backpatch-through: 10

Oversight in commit 1e55e7d1, which added a wraparound failsafe
mechanism to VACUUM.

Backpatch: 14-, where VACUUM failsafe was introduced.

Commit 3499df0d added a comment that incorrectly suggested that
--force-index-cleanup did not appear in the same major version as the
similar --no-index-cleanup option.  In fact, both options are new to
PostgreSQL 14.

Backpatch: 14-, where both options were introduced.

Further fix the test code in ead9e51e8236, this time by waiting until
the checkpoint has completed before moving on; this ensures that the
WAL segment removal has already happened when we create the next slot.

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20210719.111318.2042379313472032754.horikyota.ntt@gmail.com

We don't allow to create replication slot_name as an empty string ('') via
SQL API pg_create_logical_replication_slot() but it is allowed to be set
via Alter Subscription command. This will lead to apply worker repeatedly
keep trying to stream data via slot_name '' and the user is not allowed to
create the slot with that name.

Author: Japin Li
Reviewed-By: Ranier Vilela, Amit Kapila
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/MEYP282MB1669CBD98E721C77CA696499B61A9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

This grammar has no effect as there are no dependencies on statistics,
but it is supported by the parser.  This is more consistent with the
other DROP commands.

Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm1LA=yNmzcSfy+0oe6CEAgsxXRf_-UutE3ZncFi8QkFNQ@mail.gmail.com
Backpatch-through: 10

It has been spotted that multiranges lack of ability to decompose them into
individual ranges.  Subscription and proper expanded object representation
require substantial work, and it's too late for v14.  This commit
provides the implementation of unnest(multirange), which is quite trivial.
unnest(multirange) is defined as a polymorphic procedure.

Catversion is bumped.

Reported-by: Jonathan S. Katz
Discussion: https://postgr.es/m/flat/60258efe-bd7e-4886-82e1-196e0cac5433%40postgresql.org
Author: Alexander Korotkov
Reviewed-by: Justin Pryzby, Jonathan S. Katz, Zhihong Yu, Tom Lane
Reviewed-by: Alvaro Herrera

The new test code added in ead9e51e8236 is racy -- it hinges on
shared-memory state, which changes before the WARNING message is logged.
Put it the other way around.

Backpatch to 13.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/202107161809.zclasccpfcg3@alvherre.pgsql

We had documentation of default_transaction_isolation et al,
but for some reason not of transaction_isolation et al.
AFAICS this is just an ancient oversight, so repair.

Per bug #17077 from Yanliang Lei.

Discussion: https://postgr.es/m/17077-ade8e166a01e1374@postgresql.org

pg_dump failed to preserve the 'enabled' flag (which can be not only
disabled, but also REPLICA or ALWAYS) for partitions which had it
changed from their respective parents.  Attempt to handle that by
including a definition for such triggers in the dump, but replace the
standard CREATE TRIGGER line with an ALTER TRIGGER line.

Backpatch to 11, where these triggers can exist.  In branches 11 and 12,
pick up a few test lines from commit b9b408c4 to verify that
pg_upgrade is okay with these arrangements.
Co-authored-by: Justin Pryzby <pryzby@telsasoft.com>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com

When triggers are cloned from partitioned tables to their partitions,
the 'tgenabled' flag (origin/replica/always/disable) was not propagated.
Make it so that the flag on the trigger on partition is initially set to
the same value as on the partitioned table.

Add a test case to verify the behavior.

Backpatch to 11, where this appeared in commit 86f57594.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com

When some slots are invalidated due to the max_slot_wal_keep_size limit,
the old segment horizon should move forward to stay within the limit.
However, in commit c6550776 we forgot to call KeepLogSeg again to
recompute the horizon after invalidating replication slots.  In cases
where other slots remained, the limits would be recomputed eventually
for other reasons, but if all slots were invalidated, the limits would
not move at all afterwards.  Repair.

Backpatch to 13 where the feature was introduced.

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported-by: Marcin Krupowicz <mk@071.ovh>
Discussion: https://postgr.es/m/17103-004130e8f27782c9@postgresql.org

Ensure to properly mark up function parameters in text with <parameter>,
avoid using <acronym> for terms which aren't acronyms and properly place
the ", and" in a value list. The acronym removal is a follow-up to commit
fb72a7b8 which removed it for minmax-multi.  In passing, also fix an
incorrectly cased word.

Author: Ekaterina Kiryanova <e.kiryanova@postgrespro.ru>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://postgr.es/m/c050ecbc-80b2-b360-3c1d-9fe6a6a11bb5@postgrespro.ru
Backpatch-through: v14

Autoconf's AC_CHECK_DECLS() always defines HAVE_DECL_whatever
as 1 or 0, but some of the entries in msvc/Solution.pm showed
such symbols as "undef" instead of 0.  Fix that for consistency.
There's no live bug in current usages AFAICS, but it's not hard
to imagine one creeping in if more-complex #if tests get added.

Back-patch to v13, which is as far back as Solution.pm contains
this data.  The inconsistency still exists in the manually-filled
pg_config_ext.h.win32 files of older branches; but as long as the
problem is only latent, it doesn't seem worth the trouble to
clean things up there.

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

This commit fixes the description of a couple of multirange operators and
oprjoin for another multirange operator.  The change of oprjoin is more
cosmetic since both old and new functions return the same constant.

These cosmetic changes don't worth catalog incompatibility between 14beta2
and 14beta3.  So, catversion isn't bumped.

Discussion: https://postgr.es/m/CAPpHfdv9OZEuZDqOQoUKpXhq%3Dmc-qa4gKCPmcgG5Vvesu7%3Ds1w%40mail.gmail.com
Backpatch-throgh: 14

A code path asserted that the archiver was dead, but a check made that
impossible to happen.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACW=CYE1ars+2XyPTEPq0wQvru4c0dPZ=Nrn3EqNBkksvQ@mail.gmail.com
Backpatch-throgh: 14

Reported-By: Peter Eisentraut
Backpatch-through: 14
Discussion: https://postgr.es/m/8f5e63b8-e8ed-0f80-d8c4-68222624c200@enterprisedb.com

Some commands of ALTER TABLE could fail with the following error:
ERROR:  "tab" is of the wrong type

This error is unexpected, as all the code paths leading to
ATWrongRelkindError() should use a supported set of relkinds to generate
correct error messages.  This commit closes the gap with such mistakes,
by adding all the missing relkind combinations.  Tests are added to
check all the problems found.  Note that some combinations are not used,
but these are left around as it could have an impact on applications
relying on this code.

2ed532e has done a much larger refactoring on HEAD to make such error
messages easier to manage in the long-term, so nothing is needed there.

Author: Kyotaro Horiguchi
Reviewed-by: Peter Eisentraut, Ahsan Hadi, Michael Paquier
Discussion: https://postgr.es/m/20210216.181415.368926598204753659.horikyota.ntt@gmail.com
Backpatch-through: 11

"Result Cache" was never a great name for this node, but nobody managed
to come up with another name that anyone liked enough.  That was until
David Johnston mentioned "Node Memoization", which Tom Lane revised to
just "Memoize".  People seem to like "Memoize", so let's do the rename.

Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/20210708165145.GG1176@momjian.us
Backpatch-through: 14, where Result Cache was introduced

The name introduced by commit 4656e3d6 was agreed to be unreasonably
long.  To match this change, rename initdb's recently-added
--clobber-cache option to --discard-caches.

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

41469253e went to the trouble of removing a theoretical bug from
free_sort_tuple by checking if the tuple was NULL before freeing it. Let's
make this a little more robust by also setting the tuple to NULL so that
should we be called again we won't end up doing a pfree on the already
pfree'd tuple. Per advice from Tom Lane.

Discussion: https://postgr.es/m/3188192.1626136953@sss.pgh.pa.us
Backpatch-through: 9.6, same as 41469253e

This fixes a theoretical bug in tuplesort.c which, if a bounded sort was
used in combination with a byval Datum sort (tuplesort_begin_datum), when
switching the sort to a bounded heap in make_bounded_heap(), we'd call
free_sort_tuple().  The problem was that when sorting Datums of a byval
type, the tuple is NULL and free_sort_tuple() would free the memory for it
regardless of that.  This would result in a crash.

Here we fix that simply by adding a check to see if the tuple is NULL
before trying to disassociate and free any memory belonging to it.

The reason this bug is only theoretical is that nowhere in the current
code base do we do tuplesort_set_bound() when performing a Datum sort.
However, let's backpatch a fix for this as if any extension uses the code
in this way then it's likely to cause problems.

Author: Ronan Dunklau
Discussion: https://postgr.es/m/CAApHDvpdoqNC5FjDb3KUTSMs5dg6f+XxH4Bg_dVcLi8UYAG3EQ@mail.gmail.com
Backpatch-through: 9.6, oldest supported version

Apple's mechanism for dealing with functions that are available
in only some OS versions confuses AC_CHECK_FUNCS, and therefore
AC_REPLACE_FUNCS.  We can use AC_CHECK_DECLS instead, so long as
we enable -Werror=unguarded-availability-new.  This allows people
compiling for macOS to control whether or not preadv/pwritev are
used by setting MACOSX_DEPLOYMENT_TARGET, rather than supplying
a back-rev SDK.  (Of course, the latter still works, too.)

James Hilliard

Discussion: https://postgr.es/m/20210122193230.25295-1-james.hilliard1@gmail.com