Author: Julien Rouhaud
Backpatch-through: 11
Discussion: https://postgr.es/m/20210420121659.odjueyd4rpilorn5@nol

Document VACUUM's soft assumption that any LP_DEAD items encountered
during pruning will become LP_UNUSED items before VACUUM finishes up.
This is integral to the accounting used by VACUUM to generate its final
report on the table to the stats collector.  It also affects how VACUUM
determines which heap pages are truncatable.  In both cases VACUUM is
concerned with the likely contents of the page in the near future, not
the current contents of the page.

This state of affairs created the false impression that VACUUM's dead
tuple accounting had significant difference with similar accounting used
during ANALYZE.  There were and are no substantive differences, at least
when the soft assumption completely works out.  This is far clearer now.

Also document cases where things don't quite work out for VACUUM's dead
tuple accounting.  It's possible that a significant number of LP_DEAD
items will be left behind by VACUUM, and won't be recorded as remaining
dead tuples in VACUUM's statistics collector report.  This behavior
dates back to commit a96c41fe, which taught VACUUM to run without index
and heap vacuuming at the user's request.  The failsafe mechanism added
to VACUUM more recently by commit 1e55e7d1 takes the same approach to
dead tuple accounting.
Reported-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=Jmtu18PrsYq3EvvZJGOmZqSO2u3bvKpx9xJa5uhNp=Q@mail.gmail.com

Should be signed, not unsigned.

In the new test after resetting the stats, we were not waiting for the
stats message to be delivered. Also, we need to decode the results for
the new test, otherwise, it will show the old stats.

In passing,
a. Change docs added by commit f5fc2f5b as per suggestion by
Justin Pryzby.
b. Bump the PGSTAT_FILE_FORMAT_ID as commit f5fc2f5b changes the file
format of stats.

Reported-by: Tom Lane based on buildfarm reports
Author: Vignesh C, Justin Pryzby
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de

Author: Justin Pryzby
Discussion: https://postgr.es/m/20210416070310.GG3315@telsasoft.com

A couple of tests have been using 0 as magic constant while SEEK_SET can
be used instead.  This makes the code easier to understand, and more
consistent with the changes done in 3c5b0685.

Per discussion with Andrew Dunstan.

Discussion: https://postgr.es/m/YHrc24AgJQ6tQ1q0@paquier.xyz

Add a few words of comment to explain why SIGURG doesn't follow the
dummy_handler pattern used for SIGUSR2, since that might otherwise
appear to be a bug.

Discussion: https://postgr.es/m/4006115.1618577212%40sss.pgh.pa.us

Should be %u rather than %d.

As it stands, find_expr_references_walker() pays attention to leaf-node
collation fields while ignoring the input collations of actual function
and operator nodes.  That seems exactly backwards from a semantic
standpoint, and it leads to reporting dependencies on collations that
really have nothing to do with the expression's behavior.

Hence, rewrite to look at function input collations instead.  This
isn't completely perfect either; it fails to account for the behavior
of record_eq and its siblings.  (The previous coding at least gave an
approximation of that, though I think it could be fooled pretty easily
into considering the columns of irrelevant composite types.)  We may
be able to improve on this later, but for now this should satisfy the
buildfarm members that didn't like ef387bed.

In passing fix some oversights in GetTypeCollations(), and get
rid of its duplicative de-duplications.  (I'm worried that it's
still potentially O(N^2) or worse, but this makes it a little
better.)

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

Ooops, forgot to s/system_views.sql/system_functions.sql/g
in this part of 767982e3.

No need for an additional catversion bump, I think, since
these strings are gone by the time initdb finishes.

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

Adopt the new pre-parsed representation for all built-in and
information_schema SQL-language functions, except for a small
number that can't presently be converted because they have
polymorphic arguments.

This eliminates residual hazards around search-path safety of
these functions, and might provide some small performance benefits
by reducing parsing costs.  It seems useful also to provide more
test coverage for the SQL-standard-body feature.

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

Invent system_functions.sql to carry the function definitions that
were formerly in system_views.sql.  The function definitions were
already a quarter of the file and are about to be more, so it seems
appropriate to give them their own home.

In passing, fix an oversight in dfb75e47: it neglected to call
check_input() for system_constraints.sql.

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

In order to avoid getting old logfile contents certain functions in
PostgresNode were doing one of two things. On Windows it rotated the
logfile and restarted the server, while elsewhere it truncated the log
file. Both of these are unnecessary. We borrow from the buildfarm which
does this instead: note the size of the logfile before we start, and
then when fetching the logfile skip to that position before accumulating
contents. This is spelled differently on Windows but the effect is the
same. This is largely centralized in TestLib's slurp_file function,
which has a new optional parameter, the offset to skip to before
starting to reading the file. Code in the client becomes much neater.

Backpatch to all live branches.

Michael Paquier, slightly modified by me.

Discussion: https://postgr.es/m/YHajnhcMAI3++pJL@paquier.xyz

recordMultipleDependencies had the wrong scope for its "version"
variable, allowing a version label to leak from the collation entry it
was meant for to subsequent non-collation entries.  This is relatively
hard to trigger because of the OID-descending order that the inputs
will normally arrive in: subsequent non-collation items will tend to
be pinned.  But it can be exhibited easily with a custom collation.

Also, don't special-case the default collation, but instead ignore
pinned-ness of a collation when we've found a version for it.  This
avoids creating useless pg_depend entries, and removes a not-very-
future-proof assumption that C, POSIX, and DEFAULT are the only
pinned collations.

A small problem is that, because the default collation may or may
not have a version, the regression tests can't assume anything about
whether dependency entries will be made for it.  This seems OK though
since it's now handled just the same as other collations, and we have
test cases for both versioned and unversioned collations.

Fixes oversights in commit 257836a7.  Thanks to Julien Rouhaud
for review.

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

This is only a latent bug, since these calls are only reached for
non-text output formats, and currently none of those will print
the units.  Still, we should get it right in case that ever changes.

Justin Pryzby

Discussion: https://postgr.es/m/20210415163846.GA3315@telsasoft.com

Only track BEGIN...END blocks if they are in a CREATE [OR REPLACE]
{FUNCTION|PROCEDURE} statement.  Ignore if in parentheses.
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://www.postgresql.org/message-id/cee01d26fe55bc086b3bcf10bfe4e8d450e2f608.camel@cybertec.at

Author: Erik Rijkers
Discussion: https://postgr.es/m/1219476687.20432.1617452918468@webmailclassic.xs4all.nl
Backpatch-through: 12

This adds the statistics about total transactions count and total
transaction data logically sent to the decoding output plugin from
ReorderBuffer. Users can query the pg_stat_replication_slots view to check
these stats.

Suggested-by: Andres Freund
Author: Vignesh C and Amit Kapila
Reviewed-by: Sawada Masahiko, Amit Kapila
Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de

Warn users that locales with traditional Windows NLS names like
"English_United States.1252" won't provide version information, and that
something like initdb --lc-collate=en-US would be needed to fix that
problem for the initial template databases.

Discussion: https://postgr.es/m/CA%2BhUKGJ_hk3rU%3D%3Dg2FpAMChb_4i%2BTJacpjjqFsinY-tRM3FBmA%40mail.gmail.com

Without this, we lose error cursor positions, as shown in the
modified regression test result.

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

This reverts commit b3ee4c50.
We don't need it in the wake of the preceding commit, which
added an upstream check that the querystring isn't null.

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

Commit e717a9a1 changed the longstanding rule that prosrc is NOT NULL
because when a SQL-language function is written in SQL-standard style,
we don't currently have anything useful to put there.  This seems a poor
decision though, as it could easily have negative impacts on external
PLs (opening them to crashes they didn't use to have, for instance).
SQL-function-related code can just as easily test "is prosqlbody not
null" as "is prosrc null", so there's no real gain there either.
Hence, revert the NOT NULL marking removal and adjust related logic.

For now, we just put an empty string into prosrc for SQL-standard
functions.  Maybe we'll have a better idea later, although the
history of things like pg_attrdef.adsrc suggests that it's not
easy to maintain a string equivalent of a node tree.

This also adds an assertion that queryDesc->sourceText != NULL
to standard_ExecutorStart.  We'd been silently relying on that
for awhile, so let's make it less silent.

Also fix some overlooked documentation and test cases.

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

These queries could show unexpected entries if the core system,
or concurrently-running test scripts, created any functions that
would appear in the information_schema views.  Restrict them
to showing functions belonging to this test's schema, as the
far-older nearby test case does.

Per experimentation with conversion of some built-in functions
to SQL-function-body style.

This reverts commit 3a513067.

Per discussion, this patch had too many issues to resolve at this
point of the development cycle.  We'll try again in the future.

Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.21.1904132231510.8961@lancre

Commit bbe0a81d introduced "INCLUDING COMPRESSION" option
in CREATE TABLE command, but forgot to mention it in the
CREATE TABLE syntax synopsis.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/54d30e66-dbd6-5485-aaf6-a291ed55919d@oss.nttdata.com

e4c76191 has added a space to the example used for HISTFILE in the docs
of psql before the variable DBNAME, as a workaround because variables
were not parsed the same way back then.  This behavior has changed in
9.2, causing the example in the psql docs to result in the same history
file created with or without a space added before the DBNAME variable.

Let's just remove this space in the example, to reduce any confusion, as
the point of it is to prove that a per-database history file is easy to
set up, and that's easier to read this way.

Per discussion with Tom Lane.

Reported-by: Ludovic Kuty
Discussion: https://postgr.es/m/161830067409.691.16198363670687811485@wrigleys.postgresql.org

Several of these were already fixed in passing in
9acaf1a6, but one was remaining
inconsistent.

6568cef2, that introduced the option, had an inconsistent behavior when
it comes to configuration tables set up by pg_extension_config_dump, as
the data of all configuration tables would included in a dump even for
extensions not listed by a set of --extension switches.

The contents dumped changed depending on the schema where an extension
was installed when an extension was not listed.  For example, an
extension installed under the public schema would have its configuration
data not dumped even when not listed with --extension, which was
inconsistent with the case of an extension installed on a non-public
schema, where the configuration would be dumped.

Per discussion with Noah, we have settled down to the simple rule of
dumping configuration data of an extension if it is listed in
--extension (default is unchanged and backward-compatible, to dump
everything on sight if there are no extensions directly listed).  This
avoids some weird cases where the dumps depended on a --schema for one.

More tests are added to cover the gap, where we cross-check more
behaviors depending on --schema when an extension is not listed.

Reported-by: Noah Misch
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/20210404220802.GA728316@rfd.leadboat.com

That field went away in commit edca44b1, but it seems that
commit 45be99f8 re-introduced some comments mentioning it.
Noted by James Coleman, though this isn't exactly his
proposed new wording.  Also thanks to Justin Pryzby for
software archaeology.

Discussion: https://postgr.es/m/CAAaqYe8fxZjq3na+XkNx4C78gDqykH-7dbnzygm9Qa9nuDTePg@mail.gmail.com

We don't need to mention the attribute number in these messages, because
there's a dedicated column for that, but we should mention the toast
value ID, because that's really useful for any follow-up troubleshooting
the user wants to do. This also rewords some of the messages to hopefully
read a little better.

Also, use VARATT_EXTERNAL_GET_POINTER in case we're accessing a TOAST
pointer that isn't aligned on a platform that's fussy about alignment,
so that we don't crash while corruption-checking the user's data.

Mark Dilger, reviewed by me.

Discussion: http://postgr.es/m/7D3B9BF6-50D0-4C30-8506-1C1851C7F96F@enterprisedb.com

This GUC has always been classified as a planner option since its
introduction in 7c944bd9, and was listed in postgresql.conf.sample.  As
this parameter exists for testing purposes, move it to the section
dedicated to developer parameters and hence remove it from
postgresql.conf.sample.  This will avoid any temptation to play with it
on production servers for users that should never really have to touch
this parameter.

The general description used for developer options is reworded a bit, to
take into account the inclusion of force_parallel_mode, per a suggestion
from Tom Lane.

Per discussion between Tom Lane, Bruce Momjian, Justin Pryzby, Bharath
Rupireddy and me.

Author: Justin Pryzby, Tom Lane
Discussion: https://postgr.es/m/20210403152402.GA8049@momjian.us

The tests introduced in 32a9c0bd for connections broken and
re-established rely on pg_terminate_backend() for their logic.  When
these were introduced, this function simply sent a signal to a backend
without waiting for the operation to complete, and the tests repeatedly
looked at pg_stat_activity to check if the operation was completed or
not.  Since aaf04325, it is possible to define a timeout to make
pg_terminate_backend() wait for a certain duration, so make use of it,
with a timeout reasonably large enough (3min) to give enough room for
the tests to pass even on slow machines.

Some measurements show that the tests of postgres_fdw are much faster
with this change.  For example, on my laptop, they now take 4s instead
of 6s.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACXGY_EfGrMTjKjHy2zi-u1u9rdeioU_fro0T6Jo8t56KQ@mail.gmail.com

This will make it consistent with the other usage of slotname in the code.
In the passing, change pgstat_report_replslot signature to use a structure
rather than multiple parameters.

Reported-by: Andres Freund
Author: Vignesh C
Reviewed-by: Sawada Masahiko, Amit Kapila
Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de

The function is building a fake heap tuple, but left some of the header
fields (tid and table OID) uninitialized. Per Coverity report.

Reported-by: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQApj6h8tZ0-eP5Af5PKc5NG1YUc7=SdN_99YoHS51fKa0Q@mail.gmail.com

It seems like a good idea to bypass heap truncation when the wraparound
failsafe mechanism (which was added in commit 1e55e7d1) is in effect.

Deliberately don't bypass heap truncation in the INDEX_CLEANUP=off case,
even though it is similar to the failsafe case.  There is already a
separate reloption (and related VACUUM parameter) for that.
Reported-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDWRh6oTN5T8wa+cpZUVpHXET8BJ8Da7WHVHpwkPP6KLg@mail.gmail.com