Make check_usermap() parameter names consistent.

The function has a bool argument named "case_insensitive", but that was
spelled "case_sensitive" in the declaration.  Make them consistent now
to avoid confusion in the future.

Author: Peter Geoghegan <[email protected]>
Reviewed-By: Michael Paquiër <[email protected]>
Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com
Back: 10-

Include c.h instead of postgres.h in src/port/*p{read,write}*.c

Frontend code shouldn't include postgres.h. Some files in src/port/ need to
include postgres.h/postgres_fe.h, but these files don't.

Discussion: https://postgr.es/m/20220915022626[email protected]
Back: 12-, where 3fd2a7932ef introduced (some) of these files

Remove DLLTOOL, DLLWRAP from configure / Makefile.global.in

We got rid of the need for them in 4f5f485d10c and 846e91e0223.

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

pgstat: Create memory contexts below TopMemoryContext

So far they were created below CacheMemoryContext. However, that's not
guaranteed to exist in all situations, leading to memory contexts created as
top-level contexts. There isn't actually a good reason anymore to create them
below CacheMemoryContext, so just creating them below TopMemoryContext seems
the best approach.

Reported-by: Reid Thompson <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Author: "Drouvot, Bertrand" <[email protected]>
Discussion: https://postgr.es/m/b948b729-42fe-f88c-2f4a-0e65d84c049b@amazon.com
Back: 15-

Fix huge_pages on Windows

Since Windows 10 1703, it is additionally necessary to pass a flag
called FILE_MAP_LARGE_PAGES to MapViewOfFile() to enable large pages at
map time.  This flag is ignored on older versions of Windows, where
large pages should still be able to work properly without setting it.
Note that the flag would be set only for binaries that knew about it at
compile-time, which should be more or less all the Windows environments
these days.

Since 495ed0e, Windows 10 is the minimum version of Windows supported by
Postgres, making this change easy to reason about on HEAD.  Per
discussion, no back is done for the moment.

Reported-by: Okano Naoki
Author: Thomas Munro
Reviewed-by: Tom Lane, Michael Paquier, Julien Rouhaud
Discussion: https://postgr.es/m/17448-0a96583a67edb1f7@postgresql.org

Message style improvements

Fix race condition in stats.sql added in 5264add7847

Very occasionally the stats test failed due to the number of sessions not
being updated yet. Likely this requires that there is contention on the
database's stats entry. Solve this by forcing pending stats to be flushed
before fetching the stats.

I verified that there are no other test failures after making
pgstat_report_stat() only flush stats when force = true.

Per message from Tom Lane and buildfarm member crake.

Discussion: https://postgr.es/m/3428246.1663271992@sss.pgh.pa.us
Back: 15-, where 5264add7847 added the test

Improve plpgsql's ability to handle arguments declared as RECORD.

Treat arguments declared as RECORD as if that were a polymorphic type
(which it is, sort of), in that we substitute the actual argument type
while forming the function cache lookup key.  This allows the specific
composite type to be known in some cases where it was not before,
at the cost of making a separate function cache entry for each named
composite type that's passed to the function during a session.  The
particular symptom discussed in bug #17610 could be solved in other
more-efficient ways, but only at the cost of considerable development
work, and there are other cases where we'd still fail without this.

Per bug #17610 from Martin Jurča.  Back- to v11 where we first
allowed plpgsql functions to be declared as taking type RECORD.

Discussion: https://postgr.es/m/17610-fb1eef75bf6c2364@postgresql.org

aix: xlc: Use -Wl,-b,expfull for old compiler versions

Unfortunately xlc 12.1 and earlier don't support -qvisibility. Therefore
be7c15b194a doesn't suffice to make extension libraries work without the
explicit mkldexport step removed in fe6a64a58ab. While 12.1 is EOL, there is
some desire to leave buildfarm animals using it run a bit longer. But instead
of adding back the complicated mkldexport step, we can use -Wl,-b,expfull to
force all symbols to be exported.

Reviewed-By: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/2490844.1663123546@sss.pgh.pa.us

Clean up minor inconsistencies in pg_attribute_printf() usage.

For some reason we'd never decorated pg_v*printf() with
pg_attribute_printf() annotations.  There is a convention for
how to label va_list-using printf functions (write zero for the
second argument), and we use that liberally elsewhere in the
code, but these core functions lacked it.  It's not clear how
much useful checking the compiler can do for calls of these,
but we might as well add the annotations.

Also, sync win32security.c's log_error() with our normal convention
that pg_attribute_printf must be attached to a function's declaration
not definition.  Apparently this file is only compiled with compilers
that aren't picky about that, but still it'd be better to be
consistent.

No back- since there's little reason to think we would catch
anything.

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

Message wording improvements

postgres_fdw: Remove useless DO block in test

Discussion: https://www.postgresql.org/message-id/flat/b1f9f399-3a1a-b554-283f-4ae7f34608e2@enterprisedb.com

Adjust assorted hint messages that list all valid options.

Instead of listing all valid options, we now try to provide one
that looks similar.  Since this may be useful elsewhere, this
change introduces a new set of functions that can be reused for
similar purposes.

Author: Nathan Bossart <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/b1f9f399-3a1a-b554-283f-4ae7f34608e2@enterprisedb.com

Fix createdb tests for C locale

If the createdb tests run under the C locale, the database cluster
will be initialized with encoding SQL_ASCII.  With the checks added in
c7db01e325a530ec38ec7ba57cd3ed32e123e33c, this will cause several
ICU-related tests to fail because SQL_ASCII is not supported by ICU.
To work around that, use initdb option -E UTF8 for those tests to get
past that check.

Don't allow creation of database with ICU locale with unsupported encoding

Check in CREATE DATABASE and initdb that the selected encoding is
supported by ICU.  Before, they would pass but users would later get
an error from the server when they tried to use the database.

Also document that initdb sets the encoding to UTF8 by default if the
ICU locale provider is chosen.

Author: Marina Polyakova <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Discussion: https://www.postgresql.org/message-id/6dd6db0984d86a51b7255ba79f111971@postgrespro.ru

Detect format-string mistakes in the libpq_pipeline test module.

I happened to notice that libpq_pipeline's private implementation
of pg_fatal lacked any pg_attribute_printf decoration.  Indeed,
adding that turned up a mistake!  We'd likely never have noticed
because the error exits in this code are unlikely to get hit,
but still, it's a bug.

We're so used to having the compiler check this stuff for us that
a printf-like function without pg_attribute_printf is a land mine.
I wonder if there is a way to detect such omissions.

Back- to v14 where this code came in.

Copy-edit docs for logical replication column lists

There was a excessive structure, leading to somewhat disorganized
presentation of the information. Remove a few tags and reorder
paragraphs to make the text flow more easily.  Also, reword some of it
to be more concise.

The bit about column list combination is not modified, other than to
remove an uninteresting (and IMO confusing and wrong) paragraph; I
intend to deal with it differently afterwards.

Back to 15.

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

Revert ill-considered change in pg_resetwal output.

Commit 31dcfae83 changed one pg_resetwal output string, and a
corresponding test in pg_upgrade, without sufficient thought for
the consequences.  We can't change that output without creating
hazards for cross-version upgrades, since pg_upgrade needs to be able
to read the output of several different versions of pg_resetwal.
There may well be external tools with the same requirement.

For the moment, just revert those two changes.  What we really
ought to do here is have a separate, stable, easily machine-readable
output format for pg_resetwal and pg_controldata, as proposed
years ago by Alvaro.  Once that's in place and tools no longer
need to depend on the exact spelling of the human-readable output,
we can put back this change.

Discussion: https://postgr.es/m/fbea8c6f-415a-bad9-c3de-969c40d08a84@dunslane.net

Reset InstallXLogFileSegmentActive after walreceiver self-initiated exit.

After commit cc2c7d65fc27e877c9f407587b0b92d46cd6dd16 added this flag,
failure to reset it caused assertion failures.  In non-assert builds, it
made the system fail to achieve the objectives listed in that commit;
chiefly, we might emit a spurious log message.  Back- to v15, where
that commit first appeared.

Bharath Rupireddy and Kyotaro Horiguchi.  Reviewed by Dilip Kumar,
Nathan Bossart and Michael Paquier.  Reported by Dilip Kumar.

Discussion: https://postgr.es/m/CAFiTN-sE3ry=ycMPVtC+Djw4Fd7gbUGVv_qqw6qfzp=JLvqT3g@mail.gmail.com

Fix grammar in error message

While at it, make ellipses formatting consistent when describing SQL statements.

Ekaterina Kiryanova and Alexander Lakhin

Reviewed by myself and Álvaro Herrera
Discussion: https://www.postgresql.org/message-id/eed5cec0-a542-53da-6a5e-7789c6ed9817%40postgrespro.ru
Back only the grammar fix to v15

Blind attempt to fix LLVM dependency in the backend

Commit ecaf7c5df5 removed gram.h from the backend's generated-headers
target. In LLVM builds, this leads to loss of dependency information
when generating .bc files. To fix, add a rule that mirrors ad-hoc .o
dependencies for .bc files as well.

Per cfbot (no buildfarm failures reported)
Analysis by Tom Lane and Andres Freund
Proposed fix by Andres Freund
Discussion: https://www.postgresql.org/message-id/20220914210427.y26tkagmxo5wwbvp%40awork3.anarazel.de

Use the terminology "WAL file" not "log file" more consistently.

Referring to the WAL as just "log" invites confusion with the
postmaster log, so avoid doing that in docs and error messages.
Also shorten "WAL segment file" to just "WAL file" in various
places.

Bharath Rupireddy, reviewed by Nathan Bossart and Kyotaro Horiguchi

Discussion: https://postgr.es/m/CALj2ACUeXa8tDPaiTLexBDMZ7hgvaN+RTb957-cn5qwv9zf-MQ@mail.gmail.com

Fix outdated convert_saop_to_hashed_saop comment

In 29f45e299, we added support for optimizing the execution of NOT
IN(values) by using a hash table instead of a linear search over the
array.  That commit neglected to update the header comment for
convert_saop_to_hashed_saop() to mention this fact.  Here we fix that.

Author: James Coleman
Discussion: https://postgr.es/m/CAAaqYe99NUpAPcxgchGstgM23fmiGjqQPot8627YgkBgNt=BfA@mail.gmail.com
Back-through: 15, where 29f45e299 was added.

Small wording improvements

Doc: add some doco about using the libpq_pipeline test module.

The README file here was barely a stub.  Try to make it useful.

Jelte Fennema, with some further work by me

Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com

pgcrypto: Remove unused code

The mbuf_tell, mbuf_rewind and pgp_get_cipher_name functions were
introduced in commit e94dd6ab91, but were never used, so remove.

Reviewed-by: Aleksander Alekseev <[email protected]>
Discussion: https://postgr.es/m/FCF3F14E-17D5-41F2-AC58-0A97B341193A@yesql.se

Update .gitignore

Add entry for ab393528fa4b2486237ee7aa51fac67f82fee824.  Remove one
obsolete entry.

Use SIGNAL_ARGS consistently to declare signal handlers.

Various bits of code were declaring signal handlers manually,
using "int signum" or variants of that.  We evidently have no
platforms where that's actually wrong, but let's use our
SIGNAL_ARGS macro everywhere anyway.  If nothing else, it's
good for finding signal handlers easily.

No need for back-, since this is just cosmetic AFAICS.

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

Run xmllint validation only once

Before, each documentation target that built something from
postgres.sgml ran xmllint first to validate the input.  Here, we
change it so that the validation only runs once and produces an output
file, and all the other targets build from that output file.  This
avoids redundant work when building multiple documentation targets
(such as html and man).

Also, when we run xmllint, we can resolve entities (included files).
This helps with tools that don't support vpath builds, such as
dbtoepub.

All this also organizes the make targets a bit better for implementing
equivalent steps in meson.

Discussion: https://www.postgresql.org/message-id/e3ae16de-c9f9-f559-2d11-70b1342ae3d1@enterprisedb.com

Handle SIGTERM in pg_receivewal and pg_recvlogical

In pg_receivewal, compressed output is only flushed on clean exits.  The
reason to support SIGTERM as well as SIGINT (which is currently handled)
is that pg_receivewal might well be running as a daemon, and systemd's
default KillSignal is SIGTERM.

Since pg_recvlogical is also supposed to run as a daemon, teach it about
SIGTERM as well and update the documentation to match.  While in there,
change pg_receivewal's time_to_stop to be sig_atomic_t like it is in
pg_recvlogical.

Author: Christoph Berg <[email protected]>
Reviewed-by: Bharath Rupireddy <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/Yvo/[email protected]

Add subxid-overflow "isolation" test

This test covers a few lines of subxid-overflow-handling code in various
part of the backend, which are otherwise uncovered.

Author: Simon Riggs <[email protected]>
Reviewed-by: Dilip Kumar <[email protected]>
Discussion: https://postgr.es/m/CANbhV-H8ov5+nCMBYQFKhO+UZJjrFgY_ORiMWr3RhS4+x44PzA@mail.gmail.com

Remove duplicate initialization

This appears to be a merge mistake in 96ef3237bf74.  We could put it
back the way it was before JSON_TABLE and it'd be two lines shorter, but
it's likely that JSON_TABLE will be back and will prefer things this
way.  It makes no other difference in practice.

Back to 15.

Reported by Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAr4nOcNQskC4oBEZN4S+4heJ=1ch_ZKOxU+_Ef-FQSf-g@mail.gmail.com

postgres_fdw: Avoid 'variable not found in subplan target list' error.

The tlist of the EvalPlanQual outer plan for a ForeignScan node is
adjusted to produce a tuple whose descriptor matches the scan tuple slot
for the ForeignScan node.  But in the case where the outer plan contains
an extra Sort node, if the new tlist contained columns required only for
evaluating PlaceHolderVars or columns required only for evaluating local
conditions, this would cause setrefs.c to fail with the error.

The cause of this is that when creating the outer plan by injecting the
Sort node into an alternative local join plan that could emit such extra
columns as well, we fail to arrange for the outer plan to propagate them
up through the Sort node, causing setrefs.c to fail to match up them in
the new tlist to what is available from the outer plan.  Repair.

Per report from Alexander Pyhalov.

Richard Guo and Etsuro Fujita, reviewed by Alexander Pyhalov and Tom Lane.
Back to all supported versions.

Discussion: http://postgr.es/m/cfb17bf6dfdf876467bd5ef533852d18%40postgrespro.ru

Fix failure to build gramparse.h standalone in vpath builds

Add include directory in a similar fashion as 829906fb6c.

Per buildfarm animal crake

Fix incorrect value for "strategy" with deflateParams() in walmethods.c

The zlib documentation mentions the values supported for the compression
strategy, but this code has been using a hardcoded value of 0 rather
than Z_DEFAULT_STRATEGY.  This commit adjusts the code to use
Z_DEFAULT_STRATEGY.

Back down to where this code has been added to ease the backport of
any future  touching this area.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us
Back-through: 10

Fix typo in pgbench.c.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20220914.114608.1462991533784489178[email protected]

Bump minimum Perl version to 5.14

The oldest vendor-shipped Perl in the buildfarm is 5.14.2, which is
the last version that Debian Wheezy shipped. That OS is EOL, but we
keep it running because there is no other convenient way to test certain
non-mainstream 32-bit platforms. There is no bugfix in the 5.14.2 release
that is required, and yet it's also not the latest minor release --
that would be 5.14.4. To clarify the situation, we have thus arranged the
buildfarm to test 5.14.0. That allows configure scripts and documentation
to state 5.14 without fine print.

The MSVC build didn't check the version, since our previous minimum 5.8.3
was considered too old to check for on Windows. We will need a check for
Windows sometime during the v16 cycle, but that could be rendered moot
by the impending Meson conversion, so it seems safe to just document
the requirement for now.

Reviewed by Tom Lane
Discussion: https://www.postgresql.org/message-id/20220902181553[email protected]

Move gramparse.h to src/backend/parser

This header is semi-private, being used only in files related to
raw parsing, so move to the backend directory where those files
live. This allows removal of Makefile rules that symlink gram.h to
src/include/parser, since gramparse.h can now include gram.h from
within the same directory. This has the side-effect of no longer
installing gram.h and gramparse.h, but there doesn't seem to be a
good reason to continue doing so.

Per suggestion from Andres Freund and Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/20220904181759.px6uosll6zbxcum5%40awork3.anarazel.de

Simplify handling of compression level with compression specifications

PG_COMPRESSION_OPTION_LEVEL is removed from the compression
specification logic, and instead the compression level is always
assigned with each library's default if nothing is directly given.  This
centralizes the checks on the compression methods supported by a given
build, and always assigns a default compression level when parsing a
compression specification.  This results in complaining at an earlier
stage than previously if a build supports a compression method or not,
aka when parsing a specification in the backend or the frontend, and not
when processing it.  zstd, lz4 and zlib are able to handle in their
respective routines setting up the compression level the case of a
default value, hence the backend or frontend code (pg_receivewal or
pg_basebackup) has now no need to know what the default compression
level should be if nothing is specified: the logic is now done so as the
specification parsing assigns it.  It can also be enforced by passing
down a "level" set to the default value, that the backend will accept
(the replication protocol is for example able to handle a command like
BASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')).

This code simplification fixes an issue with pg_basebackup --gzip
introduced by ffd5365, where the tarball of the streamed WAL segments
would be created as of pg_wal.tar.gz with uncompressed contents, while
the intention is to compress the segments with gzip at a default level.
The origin of the confusion comes from the handling of the default
compression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of
0 was getting assigned, which is what walmethods.c would consider
as equivalent to no compression when  WAL segments with its tar
methods.  Assigning always the compression level removes the confusion
of some code paths considering a value of 0 set in a specification as
either no compression or a default compression level.

Note that 010_pg_basebackup.pl has to be adjusted to skip a few tests
where the shape of the compression detail string for client and
server-side compression was checked using gzip.  This is a result of the
code simplification, as gzip specifications cannot be used if a build
does not support it.

Reported-by: Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us
Back-through: 15

Split up guc.c for better build speed and ease of maintenance.

guc.c has grown to be one of our largest .c files, making it
a bottleneck for compilation.  It's also acquired a bunch of
knowledge that'd be better kept elsewhere, because of our not
very good habit of putting variable-specific check hooks here.
Hence, split it up along these lines:

* guc.c itself retains just the core GUC housekeeping mechanisms.
* New file guc_funcs.c contains the SET/SHOW interfaces and some
  SQL-accessible functions for GUC manipulation.
* New file guc_tables.c contains the data arrays that define the
  built-in GUC variables, along with some already-exported constant
  tables.
* GUC check/assign/show hook functions are moved to the variable's
  home module, whenever that's clearly identifiable.  A few hard-
  to-classify hooks ended up in commands/variable.c, which was
  already a home for miscellaneous GUC hook functions.

To avoid cluttering a lot more header files with #include "guc.h",
I also invented a new header file utils/guc_hooks.h and put all
the GUC hook functions' declarations there, regardless of their
originating module.  That allowed removal of #include "guc.h"
from some existing headers.  The fallout from that (hopefully
all caught here) demonstrates clearly why such inclusions are
best minimized: there are a lot of files that, for example,
were getting array.h at two or more levels of remove, despite
not having any connection at all to GUCs in themselves.

There is some very minor code beautification here, such as
renaming a couple of inconsistently-named hook functions
and improving some comments.  But mostly this just moves
code from point A to point B and deals with the ensuing
needs for #include adjustments and exporting a few functions
that previously weren't exported.

 by me, per a suggestion from Andres Freund; thanks also
to Michael Paquier for the idea to invent guc_funcs.c.

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

Don't reflect unescaped cert data to the logs

Commit 3a0e385048 introduced a new path for unauthenticated bytes from
the client certificate to be printed unescaped to the logs. There are a
handful of these already, but it doesn't make sense to keep making the
problem worse. \x-escape any unprintable bytes.

The test case introduces a revoked UTF-8 certificate. This requires the
addition of the `-utf8` flag to `openssl req`. Since the existing
certificates all use an ASCII subset, this won't modify the existing
certificates' subjects if/when they get regenerated; this was verified
experimentally with

    $ make sslfiles-clean
    $ make sslfiles

Unfortunately the test can't be run in the CI yet due to a test timing
issue; see 55828a6b60.

Author: Jacob Champion <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAAWbhmgsvHrH9wLU2kYc3pOi1KSenHSLAHBbCVmmddW6-mc_=w@mail.gmail.com

pg_clean_ascii(): escape bytes rather than lose them

Rather than replace each unprintable byte with a '?' character, replace
it with a hex escape instead. The API now allocates a copy rather than
modifying the input in place.

Author: Jacob Champion <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAAWbhmgsvHrH9wLU2kYc3pOi1KSenHSLAHBbCVmmddW6-mc_=w@mail.gmail.com

Make locale option behavior more consistent

Locale options can be specified for initdb, createdb, and CREATE
DATABASE.  In initdb, it has always been possible to specify --locale
and then some --lc-* option to override a category.  CREATE DATABASE
and createdb didn't allow that, requiring either the all-categories
option or only per-category options.  In
f2553d43060edb210b36c63187d52a632448e1d2, this was changed in CREATE
DATABASE (perhaps by accident?) to be more like the initdb behavior,
but createdb still had the old behavior.

Now we change createdb to match the behavior of CREATE DATABASE and
initdb, and also update the documentation of CREATE DATABASE to match
the new behavior, which was not done in the above commit.

Author: Marina Polyakova <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Discussion: https://www.postgresql.org/message-id/7c99c132dc9c0ac630e0127f032ac480@postgrespro.ru

Improve wal_decode_buffer_size description some more

Per Thomas Munro

Discussion: https://postgr.es/m/CA+hUKGJ9wP9kpvgoxHvqA=4g1d9-y_w3LhhdhFVU=mFiqjwHww@mail.gmail.com

Remove useless pstrdups in untransformRelOptions

The two strings are already a single palloc'd chunk, not freed; there's
no reason to allocate separate copies that have the same lifetime.

This code is only called in short-lived memory contexts (except in some
cases in TopTransactionContext, which is still short-lived enough not to
really matter), and typically only for short arrays, so the memory or
computation saved is likely negligible.  However, let's fix it to avoid
leaving a bad example of code to copy.  This is the only place I could
find where we're doing this with makeDefElem().

Reported-by: Junwang Zhao <[email protected]>
Discussion: https://postgr.es/m/20220909142050[email protected]

Adjust header exceptions for 0bd9c6297

Per buildfarm animal crake

Treat Unicode codepoints of category "Format" as non-spacing

Commit d8594d123 updated the list of non-spacing codepoints used
for calculating display width, but in doing so inadvertently removed
some, since the script used for that commit only considered combining
characters.

For complete coverage for zero-width characters, include codepoints in
the category Cf (Format). To reflect the wider purpose, also rename files
and update comments that referred specifically to combining characters.

Some of these ranges have been missing since v12, but due to lack of
field complaints it was determined not important enough to justify adding
special-case logic the backbranches.

Kyotaro Horiguchi

Report by Pavel Stehule
Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRBE8yvpQ0FSkPCoe0Ny1jAAsAQ6j3qMgVwWvkqAoaaNmQ%40mail.gmail.com

Rename macro related to pg_backup_stop()

This should have been part of 39969e2 that has renamed pg_stop_backup()
to pg_backup_stop(), and this one is the last reference to
pg_stop/start_backup() I could find in the tree.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACXjvC28ppeDTCrfaSyHga0ggP5nRLJbsjx=7N-74UT4QA@mail.gmail.com

Move any remaining files generated by pg_upgrade into an internal subdir

This change concerns a couple of .txt files (for internal state checks)
that were still written in the path where the binary is executed, and
not in the subdirectory located in the target cluster.  Like the other
.txt files doing already so (like loadable_libraries.txt), these are
saved in the base output directory.  Note that on failure, the logs
report the full path to the .txt file generated, so these are easy to
find.

Oversight in 38bfae3.

Author: Daniel Gustafsson
Reviewed-by: Michael Paquier, Justin Prysby
Discussion: https://postgr.es/m/181A6DA8-3B7F-4B71-82D5-363FF0146820@yesql.se
Back-through: 15

Don't reference out-of-bounds array elements in brin_minmax_multi.c

The primary fix here is to fix has_matching_range() so it does not
reference ranges->values[-1] when nranges == 0.  Similar problems existed
in AssertCheckRanges() too.  It does not look like any of these problems
could lead to a crash as the array in question is at the end of the Ranges
struct, and values[-1] is memory that belongs to other fields in the
struct.  However, let's get rid of these rather unsafe coding practices.

In passing, I (David) adjusted some comments to try to make it more clear
what some of the fields are for in the Ranges struct.  I had to study the
code to find out what nsorted was for as I couldn't tell from the
comments.

Author: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAqJQzPitufX-jR=YUbJafpCDAKUnwgdbX_MzSc93wuvdw@mail.gmail.com
Back-through: 14, where multi-range brin was added.

doc: Fix link to FreeBSD documentation project

The FreeBSD site was changed with a redirect, which in turn seems to
lead to a 404. Replace with the working link.

Author: James Coleman <[email protected]>
Discussion: https://postgr.es/m/CAAaqYe_JZRj+KPn=hACtwsg1iLRYs=jYvxG1NW4AnDeUL1GD-Q@mail.gmail.com

Use normal install program to install server headers.

Commit a7032690f9 replaced $(INSTALL) with plain "cp" for installing the
server header files. It sped up "make install" significantly, because
the old logic called $(INSTALL) separately for every header file,
whereas plain "cp" could copy all the files in one command. However, we
have long since made it a requirement that $(INSTALL) can also install
multiple files in one command, see commit f1c5247563. Switch back to
$(INSTALL).

Discussion: https://www.postgresql.org/message-id/200503252305.j2PN52m23610%40candle.pha.pa.us
Discussion: https://www.postgresql.org/message-id/2415283.1641852217%40sss.pgh.pa.us

Revert "Convert *GetDatum() and DatumGet*() macros to inline functions"

This reverts commit 595836e99bf1ee6d43405b885fb69bb8c6d3ee23.

It has problems when USE_FLOAT8_BYVAL is off.

Convert *GetDatum() and DatumGet*() macros to inline functions

The previous macro implementations just cast the argument to a target
type but did not check whether the input type was appropriate.  The
function implementation can do better type checking of the input type.

Reviewed-by: Aleksander Alekseev <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/8528fb7e-0aa2-6b54-85fb-0c0886dbd6ed%40enterprisedb.com

Fix NaN comparison in circle_same test

Commit c4c340088 changed geometric operators to use float4 and float8
functions, and handle NaN's in a better way. The circle sameness test
had a typo in the code which resulted in all comparisons with the left
circle having a NaN radius considered same.

  postgres=# select '<(0,0),NaN>'::circle ~= '<(0,0),1>'::circle;
  ?column?
  ----------
  t
  (1 row)

This fixes the sameness test to consider the radius of both the left
and right circle.

Back to v12 where this was introduced.

Author: Ranier Vilela <[email protected]>
Discussion: https://postgr.es/m/CAEudQAo8dK=yctg2ZzjJuzV4zgOPBxRU5+Kb+yatFiddtQk6Rw@mail.gmail.com
Back-through: v12

pgstattuple: Use double consistently for percentages

pgstattuple uses data type double for other percentage calculations
and exposes those values to the users via the float8 data type.
However, scanned_percent in struct output_type is of type uint64, even
though it is later returned via Float8GetDatum().  Change it to use
double to be inline with other percentages.

Author: Bharath Rupireddy
Reviewed-by: Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/36ee692b-232f-0484-ce94-dc39d82021ad%40enterprisedb.com

Use float8 datatype for percentiles in pg_walinspect stat functions

pg_walinspect uses datatype double (double precision floating point
number) for WAL stats percentile calculations and expose them via
float4 (single precision floating point number), which an unnecessary
loss of precision and confusing. Even though, it's harmless that way,
let's use float8 (double precision floating-point number) to be in
sync with what pg_walinspect does internally and what it exposes to
the users. This seems to be the pattern used elsewhere in the code.

Reported-by: Peter Eisentraut
Author: Bharath Rupireddy
Reviewed-by: Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/36ee692b-232f-0484-ce94-dc39d82021ad%40enterprisedb.com

Make the tablesync worker's replication origin drop logic robust.

In commit f6c5edb8ab, we started to drop the replication origin slots
before tablesync worker exits to avoid consuming more slots than required.
We were dropping the replication origin in the same transaction where we
were marking the tablesync state as SYNCDONE. Now, if there is any error
after we have dropped the origin but before we commit the containing
transaction, the in-memory state of replication progress won't be rolled
back. Due to this, after the restart, tablesync worker can start 
from the wrong location and can apply the already processed transaction.

To fix this, we need to opportunistically drop the origin after marking
the tablesync state as SYNCDONE. Even, if the tablesync worker fails to
remove the replication origin before exit, the apply worker ensures to
clean it up afterward.

Reported by Tom Lane as per buildfarm.
Diagnosed-by: Masahiko Sawada
Author: Hou Zhijie
Reviewed-By: Masahiko Sawada, Amit Kapila
Discussion: https://postgr.es/m/20220714115155[email protected]
Discussion: https://postgr.es/m/CAD21AoAw0Oofi4kiDpJBOwpYyBBBkJj=sLUOn4Gd2GjUAKG-fw@mail.gmail.com

Assorted examples of expanded type-safer palloc/pg_malloc API

This adds some uses of the new palloc/pg_malloc variants here and
there as a demonstration and test.  This is kept separate from the
actual API , since the latter might be backed at some point.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a907@enterprisedb.com

Expand palloc/pg_malloc API for more type safety

This adds additional variants of palloc, pg_malloc, etc. that
encapsulate common usage patterns and provide more type safety.

Specifically, this adds palloc_object(), palloc_array(), and
repalloc_array(), which take the type name of the object to be
allocated as its first argument and cast the return as a pointer to
that type.  There are also palloc0_object() and palloc0_array()
variants for initializing with zero, and pg_malloc_*() variants of all
of the above.

Inspired by the talloc library.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/bb755632-2a43-d523-36f8-a1e7a389a907@enterprisedb.com

Make eval statement naturally proof against perltidy

This is a bit less verbose than adding an exception. Rewrite
the other eval statement in Catalog.pm as well, just for
the sake of consistency.

Idea from Andrew Dunstan
Discussion: https://www.postgresql.org/message-id/CAD5tBc%2BzRhuWn_S4ayH2sWKe60FQu1guTTokDFS3YMOeSrsozA%40mail.gmail.com

Replace loading of ldap_start_tls_sA() by direct function call

This change impacts the backend-side code in charge of starting a LDAP
TLS session.  It is a bit sad that it is not possible to unify the WIN32
and non-WIN32 code paths, but the different number of arguments for both
discard this possibility.

This is similar to 47bd0b3, where this replaces the last function
loading that seems worth it, any others being either environment or
version-dependent.

Reported-by: Thomas Munro
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/[email protected]

Doc: improve explanation of when custom GUCs appear in pg_settings.

Be more clear about when and how an extension-defined GUC comes to be
visible in pg_settings.  (Move the para to the bottom of the page, too;
whoever thought this point was more important than the para about the
view being updatable had odd priorities IMNSHO.)

Back- to v15 where archive modules were added, since that seems
to have made this more of a sore spot than it was before.

Benoit Lobréau, Nathan Bossart

Discussion: https://postgr.es/m/CAPE8EZ7KHaXMHKwT=HOim23tDVKYA1PruRuTfeYdCrYWwPGhag@mail.gmail.com

Add psql tab compression for ALTER TABLE .. { OF | NOT OF }

ALTER TABLE .. OF is now able to complete with the list of available
composite types that can be used with the query.

Author: Aleksander Alekseev
Reviewed-by: Shinya Kato
Discussion: https://postgr.es/m/47b71e0c523b30357208e79786161281@oss.nttdata.com

Free correctly LDAPMessage returned by ldap_search_s() in auth.c

The LDAP wiki states that the search message should be freed regardless
of the return value of ldap_search_s(), but we failed to do so in one
backend code path when searching LDAP with a filter.  This is not
critical in an authentication code path failing in the backend as this
causes such the process to exit promptly, but let's be clean and free
the search message appropriately, as documented by upstream.

All the other code paths failing a LDAP operation do that already, and
somebody looking at this code in the future may miss what LDAP expects
with the search message.

Author: Zhihong Yu
Discussion: https://postgr.es/m/CALNJ-vTf5Y+8RtzZ4GjOGE9qWVHZ8awfhnFYc_qGm8fMLUNRAg@mail.gmail.com

configure: Expand -fvisibility checks to more compilers, test for -qvisibility

It looks like icc and sunpro both support -fvisibility=hidden and xlc supports
-qvisibility=hidden. I tested this on AIX and solaris with their proprietary
compilers as well as gcc, and with gcc or clang on freebsd, linux, macos,
netbsd and openbsd.

Apparently my prior commit fe6a64a58ab only works in combination with this
. While I tried to test them separately, an unknown caching issue
prevented me from noticing the problem.

Per discussion with Tom Lane and buildfarm member hoverfly.

Discussion: https://postgr.es/m/20220910052741.t7w7csyrggwus2ze%40awork3.anarazel.de
Discussion: https://postgr.es/m/20220820174213[email protected]

aix: No need to use mkldexport when we want to export all symbols

When building a shared library with exports.txt there's no need to build an
intermediary static library, we can just pass -Wl,-bE:... when generating the
.so.

When building a shared library without exports.txt, there's no need to call
mkldexport.sh to export all symbols, because all symbols are exported anyway,
and we don't need the export file on the import side (like we do for
postgres.imp).

This makes building .so's on aix a lot more similar to building on other
platforms. In particular, we don't create and remove a .a of the same name but
different contents anymore.

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

Fix possible omission of variable storage markers in ECPG.

The ECPG preprocessor converted code such as

static varchar str1[10], str2[20], str3[30];

into

static  struct varchar_1  { int len; char arr[ 10 ]; }  str1 ;
        struct varchar_2  { int len; char arr[ 20 ]; }  str2 ;
        struct varchar_3  { int len; char arr[ 30 ]; }  str3 ;

thus losing the storage attribute for the later variables.
Repeat the declaration for each such variable.

(Note that this occurred only for variables declared "varchar"
or "bytea", which may help explain how it escaped detection
for so long.)

Andrey Sokolov

Discussion: https://postgr.es/m/942241662288242@mail.yandex.ru

Doc: improve documentation about where the psqlrc files are.

Remove no-longer-accurate claim that Windows lacks home directories.
Clarify the text by more clearly distinguishing which statements
reflect hard-wired choices versus which ones reflect overridable
defaults.  Update the examples of version-specific file names,
and make them track future version changes by using "&majorversion;"
and "&version;".  (BTW, in devel and beta releases this method
correctly says that you can use strings like "16devel" and "15beta4"
as minor version identifiers.)

Back- to v15, but not further, with the thought that in older
releases the examples with three-part version numbers still had
some historical relevance.  v15 will be the first major release after
the last 9.x branch went out of support.

Robert Treat and Tom Lane, reviewed by Julien Rouhaud

Discussion: https://postgr.es/m/CAJSLCQ07F-WCYYYOY8+dWhHcVeJ1Pb01cWc-c0Hu=M3EjKT2Eg@mail.gmail.com

Reject bogus output from uuid_create(3).

When using the BSD UUID functions, contrib/uuid-ossp expects
uuid_create() to produce a version-1 UUID.  FreeBSD still does so,
but in recent NetBSD releases that function produces a version-4
(random) UUID instead.  That's not acceptable for our purposes:
if the user wanted v4 she would have asked for v4, not v1.
Hence, check the version digit and complain if it's not '1'.

Also drop the documentation's claim that the NetBSD implementation
is usable.  It might be, depending on which OS version you're using,
but we're not going to get into that kind of detail.

(Maybe someday we should ditch all these external libraries
and just write our own UUID code, but today is not that day.)

Nazir Bilal Yavuz, with cosmetic adjustments and docs by me.
Back to all supported versions.

Discussion: https://postgr.es/m/3848059.1661038772@sss.pgh.pa.us
Discussion: https://postgr.es/m/17358-89806e7420797025@postgresql.org

Doc fixes for MERGE statement

In commit 3d895bc846f2 I introduced a bogus semicolon mid-statement by
careless cut-n-paste; move it.  This had already been reported by Justin
Pryzby.

Also, change the styling a bit by avoiding names in CamelCase.  This is
more consistent with the style we use elsewhere.

Back to 15.

Author: Vitaly Burovoy <[email protected]>
Reviewed-by: Vik Fearing <[email protected]>
Discussion: https://postgr.es/m/9afe5766-5a61-7860-598c-136867fad065@gmail.com
Discussion: https://postgr.es/m/20220819133016[email protected]

Fix GetForeignKey*Triggers for self-referential FKs

Because of inadequate filtering, the check triggers were confusing the
search for action triggers in GetForeignKeyActionTriggers and vice-versa
in GetForeignKeyCheckTriggers; this confusion results in seemingly
random assertion failures, and can have real impact in non-asserting
builds depending on catalog order.  Change these functions so that they
correctly ignore triggers that are not relevant to each side.

To reduce the odds of further problems, do not break out of the
searching loop in assertion builds.  This break is likely to hide bugs;
without it, we would have detected this bug immediately.

This problem was introduced by f4566345cf40, so back to 15 where
that commit first appeared.

Author: Amit Langote <[email protected]>
Discussion: https://postgr.es/m/20220908172029[email protected]
Discussion: https://postgr.es/m/4104619.1662663056@sss.pgh.pa.us

Bump minimum version of Flex to 2.5.35

Since the retirement of some older buildfarm members, the oldest Flex
that gets regular testing is 2.5.35.

Reviewed by Andres Freund
Discussion: https://www.postgresql.org/message-id/1097762.1662145681@sss.pgh.pa.us

Bump minimum version of Bison to 2.3

Since the retirement of some older buildfarm members, the oldest Bison
that gets regular testing is 2.3. MacOS ships that version, and will
continue doing so for the forseeable future because of Apple's policy
regarding GPLv3. While Mac users could use a package manager to install
a newer version, there is no compelling reason to force them do so at
this time.

Reviewed by Andres Freund
Discussion: https://www.postgresql.org/message-id/1097762.1662145681@sss.pgh.pa.us

Add jsonpath_gram.h to list of distprep targets

Oversight in dac048f71e

Replace load of functions by direct calls for some WIN32

This commit changes the following code paths to do direct system calls
to some WIN32 functions rather than loading them from an external
library, shaving some code in the process:
- Creation of restricted tokens in pg_ctl.c, introduced by a25cd81.
- QuerySecurityContextToken() in auth.c for SSPI authentication in the
backend, introduced in d602592.
- CreateRestrictedToken() in src/common/.  This change is similar to the
case of pg_ctl.c.

Most of these functions were loaded rather than directly called because,
as mentioned in the code comments, MinGW headers were not declaring
them.  I have double-checked the recent MinGW code, and all the
functions changed here are declared in its headers, so this change
should be safe.  Note that I do not have a MinGW environment at hand so
I have not tested it directly, but that MSVC was fine with the change.
The buildfarm will tell soon enough if this change is appropriate or not
for a much broader set of environments.

A few code paths still use GetProcAddress() to load some functions:
- LDAP authentication for ldap_start_tls_sA(), where I am not confident
that this change would work.
- win32env.c and win32ntdll.c where we have a per-MSVC version
dependency for the name of the library loaded.
- crashdump.c for MiniDumpWriteDump() and EnumDirTree(), where direct
calls were not able to work after testing.

Reported-by: Thomas Munro
Reviewed-by: Justin Prysby
Discussion: https://postgr.es/m/CA+hUKG+BMdcaCe=P-EjMoLTCr3zrrzqbcVE=8h5LyNsSVHKXZA@mail.gmail.com

Add more error context to RestoreBlockImage() and consume it

On failure in restoring a block image, no details were provided, while
it is possible to see failure with an inconsistent record state, a
failure in processing decompression or a failure in decompression
because a build does not support this option.

RestoreBlockImage() is used in two code paths in the backend code,
during recovery and when checking a page consistency after applying
masking, and both places are changed to consume the error message
produced by the internal routine when it returns a false status.  All
the error messages are reported under ERRCODE_INTERNAL_ERROR, that gets
used also when attempting to access a page compressed by a method
not supported by the build attempting the decompression.  This is
something that can happen in core when doing physical replication with
primary and standby using inconsistent build options, for example.

This routine is available since 2c03216d and it has never provided any
context about the error happening when it failed.  This change is
justified even more after 57aa5b2, that introduced compression of FPWs
in WAL.

Reported-by: Justin Prysby
Author: Michael Paquier
Discussion: https://postgr.es/m/20220905002320[email protected]
Back-through: 15

Instrument freezing in autovacuum log reports.

Add a new line to log reports from autovacuum (as well as VACUUM VERBOSE
output) that shows information about freezing.  Emphasis is placed on
the total number of heap pages that had one or more tuples frozen by
VACUUM.  The total number of tuples frozen is also shown.

Author: Peter Geoghegan <[email protected]>
Reviewed-By: Jeff Janes <[email protected]>
Discussion: https://postgr.es/m/CAH2-WznTY6D0zyE8VLrC6Gd4kh_HGAXxnTPtcOQOOsxzLx9zog@mail.gmail.com

Temporarily make MemoryContextContains return false

5265e91fd changed MemoryContextContains to update it so that it works
correctly with the new MemoryChunk code added in c6e0fe1f2.  However,
5265e91fd was done with the assumption that MemoryContextContains would
only ever be given pointers to memory that had been returned by one of our
MemoryContext allocators.  It seems that's not true and many of our 32-bit
buildfarm animals are clearly showing that.

There are some code paths that call MemoryContextContains with a pointer
pointing part way into an allocated chunk.  The example of this found by
the 32-bit buildfarm animals is the int2int4_sum() function.  This
function returns transdata->sum, which is not a pointer to memory that was
allocated directly.  This return value is then subsequently passed to
MemoryContextContains which causes it to crash due to it thinking the
memory directly prior to that pointer is a MemoryChunk.  What's actually
in that memory is the field in the struct that comes prior to the "sum"
field.  This problem didn't occur in 64-bit world because BIGINT is a
byval type and the code which was calling MemoryContextContains with the
bad pointer only does so with non-byval types.

Here, instead of reverting 5265e91fd and making MemoryContextContains
completely broken again, let's just make it always return false for now.
Effectively prior to 5265e91fd it was doing that anyway, this at least
makes that more explicit.  The only repercussions of this with the current
MemoryContextContains calls are that we perform a datumCopy() when we
might not need to.  This should make the 32-bit buildfarm animals happy
again and give us more time to consider a long-term fix.

Discussion: https://postgr.es/m/20220907130552.sfjri7jublfxyyi4%40jrouhaud

Choose FK name correctly during partition attachment

During ALTER TABLE ATTACH PARTITION, if the name of a parent's foreign
key constraint is already used on the partition, the code tries to
choose another one before the FK attributes list has been populated,
so the resulting constraint name was "<relname>__fkey" instead of
"<relname>_<attrs>_fkey".  Repair, and add a test case.

Back to 12.  In 11, the code to attach a partition was not smart
enough to cope with conflicting constraint names, so the problem doesn't
exist there.

Author: Jehan-Guillaume de Rorthais <[email protected]>
Discussion: https://postgr.es/m/20220901184156.738ebee5@karst

Fix recovery_prefetch with low maintenance_io_concurrency.

We should process completed IOs *before* trying to start more, so that
it is always possible to decode one more record when the decoded record
queue is empty, even if maintenance_io_concurrency is set so low that a
single earlier WAL record might have saturated the IO queue.

That bug was hidden because the effect of maintenance_io_concurrency was
arbitrarily clamped to be at least 2.  Fix the ordering, and also remove
that clamp.  We need a special case for 0, which is now treated the same
as recovery_prefetch=off, but otherwise the number is used directly.
This allows for testing with 1, which would have made the problem
obvious in simple test scenarios.

Also add an explicit error message for missing contrecords.  It was a
bit strange that we didn't report an error already, and became a latent
bug with prefetching, since the internal state that tracks aborted
contrecords would not survive retrying, as revealed by
026_overwrite_contrecord.pl with this adjustment.  Reporting an error
prevents that.

Back- to 15.

Reported-by: Justin Pryzby <[email protected]>
Reviewed-by: Kyotaro Horiguchi <[email protected]>
Discussion: https://postgr.es/m/20220831140128.GS31833%40telsasoft.com

Fix perltidy breaking perlcritic

perltidying a "##no critic" line moves the marker to where it becomes
useless.  Put the line back to how it was, and protect it from further
malfeasance.

Per buildfarm member crake.

doc: Fix PL/pgSQL casing to be consistent

Ensure that all mentions of PL/pgSQL is cased equally, a few instances
of PL/PgSQL had snuck in.

Reviewed-by: Pavel Stehule <[email protected]>
Discussion: https://postgr.es/m/DDCF61C3-9E25-48A8-97BE-6113A93D54A5@yesql.se

Add b2e6e7682 to .git-blame-ignore-revs

Run perltidy over Catalog.pm

Commit 69eb643b2 deliberately left indentation unchanged to make the changes
more legible. Rather than waiting until next year's perltidy run, do it now
to avoid confusion

Per suggestion from Álvaro Herrera
Discussion: https://www.postgresql.org/message-id/20220907083558.vfvb5hcauaictgum%40alvherre.pgsql

Parse catalog .dat files as a whole when compiling the backend

Previously Catalog.pm eval'd each individual hash reference
so that comments and whitespace can be preserved when running
reformat-dat-files. This is unnecessary when building, and we can save
~15% off the run time of genbki.pl by simply slurping and eval'-ing
the whole file at once. This saves a bit of time, especially in highly
parallel builds, since most build targets depend on this script's outputs.

Report and review by Andres Freund
Discussion: https://www.postgresql.org/message-id/CAFBsxsGW%3DWRbnxXrc8UqqR479XuxtukSFWV-hnmtgsbuNAUO6w%40mail.gmail.com

Fix the test case introduced by commit 8756930190.

Before dropping a relation, ensure that it has reached a 'ready' state
after initial synchronization.

Author: Vignesh C
Reviewed-By: Amit Kapila
Discussion: https://www.postgresql.org/message-id/CALDaNm0gwjY_4HFxvvty01BOT01q_fJLKQ3pWP9=9orqubhjcQ@mail.gmail.com

Raise a warning if there is a possibility of data from multiple origins.

This commit raises a warning message for a combination of options
('copy_data = true' and 'origin = none') during CREATE/ALTER subscription
operations if the publication tables were also replicated from other
publishers.

During replication, we can skip the data from other origins as we have that
information in WAL but that is not possible during initial sync so we raise
a warning if there is such a possibility.

Author: Vignesh C
Reviewed-By: Peter Smith, Amit Kapila, Jonathan Katz, Shi yu, Wang wei
Discussion: https://www.postgresql.org/message-id/CALDaNm0gwjY_4HFxvvty01BOT01q_fJLKQ3pWP9=9orqubhjcQ@mail.gmail.com

Message style fixes

Make MemoryContextContains work correctly again

c6e0fe1f2 recently changed the way we store headers for allocated chunks
of memory.  Prior to that commit, we stored a pointer to the owning
MemoryContext directly prior to the pointer to the allocated memory.
That's no longer true and c6e0fe1f2 neglected to update
MemoryContextContains() so that it correctly obtains the owning context
with the new method.

A side effect of this change and c6e0fe1f2, in general, is that it's even
less safe than it was previously to pass MemoryContextContains() an
arbitrary pointer which was not allocated by one of our MemoryContexts.
Previously some comments in MemoryContextContains() seemed to indicate
that the worst that could happen by passing an arbitrary pointer would be
a false positive return value.  It seems to me that this was a rather
wishful outlook as we subsequently proceeded to subtract sizeof(void *)
from the given pointer and then dereferenced that memory.  So it seems
quite likely that we could have segfaulted instead of returning a false
positive.  However, it's not impossible that the memory sizeof(void *)
bytes before the pointer could have been owned by the process, but it's
far less likely to work now as obtaining a pointer to the owning
MemoryContext is less direct than before c6e0fe1f2 and will access memory
that's possibly much further away to obtain the owning MemoryContext.
Because of this, I took the liberty of updating the comment to warn
against any future usages of the function and checked the existing core
usages to ensure that we only ever pass in a pointer to memory allocated
by a MemoryContext.

Extension authors updating their code for PG16 who are using
MemoryContextContains should check to ensure that only NULL pointers and
pointers to chunks allocated with a MemoryContext will ever be passed to
MemoryContextContains.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20220905230949[email protected]

Renumber confusing value for GUC_UNIT_BYTE

It had a power-of-two value, which looks right, and causes the other
values which aren't powers-of-two to look wrong.  But this is tested
for equality and not a bitwise test.

See also:
6e7baa322773ff8c79d4d8883c99fdeff5bfa679
https://www.postgresql.org/message-id/CAOG9ApEu8bXVwBxkOO9J7ZpM76TASK_vFMEEiCEjwhMmSLiaqQ%40mail.gmail.com

Author: Justin Pryzby <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/20220720145220[email protected]

Make more effort to put a sentinel at the end of allocated memory

Traditionally, in MEMORY_CONTEXT_CHECKING builds, we only ever marked a
sentinel byte just beyond the requested size if there happened to be
enough space on the chunk to do so.  For Slab and Generation context
types, we only rounded the size of the chunk up to the next maxalign
boundary, so it was often not that likely that those would ever have space
for the sentinel given that the majority of allocation requests are going
to be for sizes which are maxaligned.  For AllocSet, it was a little
different as smaller allocations are rounded up to the next power-of-2
value rather than the next maxalign boundary, so we're a bit more likely
to have space for the sentinel byte, especially when we get away from tiny
sized allocations such as 8 or 16 bytes.

Here we make more of an effort to allow space so that there is enough room
for the sentinel byte in more cases.  This makes it more likely that we'll
detect when buggy code accidentally writes beyond the end of any of its
memory allocations.

Each of the 3 MemoryContext types has been changed as follows:

The Slab allocator will now always set a sentinel byte.  Both the current
usages of this MemoryContext type happen to use chunk sizes which were on
the maxalign boundary, so these never used sentinel bytes previously.

For the Generation allocator, we now always ensure there's enough space in
the allocation for a sentinel byte.

For AllocSet, this commit makes an adjustment for allocation sizes which
are greater than allocChunkLimit.  We now ensure there is always space for
a sentinel byte.  We don't alter the sentinel behavior for request sizes
<= allocChunkLimit.  Making way for the sentinel byte for power-of-2
request sizes would require doubling up to the next power of 2.  Some
analysis done on the request sizes made during installcheck shows that a
fairly large portion of allocation requests are for power-of-2 sizes.  The
amount of additional memory for the sentinel there seems prohibitive, so
we do nothing for those here.

Author: David Rowley
Discussion: https://postgr.es/m/3478405.1661824539@sss.pgh.pa.us

Doc: Explain about Column List feature.

Add a new logical replication section for "Column Lists" (analogous to the
Row Filters page). This explains how the feature can be used and the
caveats in it.

Author: Peter Smith
Reviewed-by: Shi yu, Vignesh C, Erik Rijkers, Amit Kapila
Back-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAHut+PvOuc9=_4TbASc5=VUqh16UWtFO3GzcKQK_5m1hrW3vqg@mail.gmail.com

Fix new pg_publication_tables query.

The addition of published column names forgot to filter on attisdropped,
leading to cases where you could see "........pg.dropped.1........"
or the like as a reportedly-published column.

While we're here, rewrite the new subquery to get a more efficient plan
for it.

Hou Zhijie, per report from Jaime Casanova.  Back- to v15 where
the bug was introduced.  (Sadly, this means we need a post-beta4
catversion bump before beta4 has even hit the streets.  I see no
good alternative though.)

Discussion: https://postgr.es/m/Yxa1SU4nH2HfN3/i@ahch-to

Fix cplusplusscheck in vpath builds

Same solution as 829906fb6.

Add psql tab compression for SET COMPRESSION with ALTER TABLE

Author: Aleksander Alekseev
Reviewed-by: Shinya Kato
Discussion: https://postgr.es/m/CAJ7c6TMuT+=P7uDepjVpdqSEp2xOmXET3Y2K-xWAO=sCz-28gg@mail.gmail.com

Fix headerscheck in vpath builds

Oversight in dac048f71e per buildfarm animal crake.
Fix per suggestion from Andrew Dunstan.

Discussion: https://www.postgresql.org/message-id/e3f4a3d0-dfcc-41cc-1ed2-acc15700ddef%40dunslane.net

Fix failure to maintainer-clean jsonpath_gram.h

Oversight in dac048f71e

Fix typo in 16d69ec29

As noted by Justin Pryzby, just I forgot to commit locally before creating
a  file.

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

Remove buggy and dead code from CreateTriggerFiringOn

Here we remove some dead code from CreateTriggerFiringOn() which was
attempting to find the relevant child partition index corresponding to the
given indexOid.  As it turned out, thanks to -Wshadow=compatible-local,
this code was buggy as the code which was finding the child indexes
assigned those to a shadowed variable that directly went out of scope.
The code which thought it was looking at the List of child indexes was
always referencing an empty List.

On further investigation, this code is dead.  We never call
CreateTriggerFiringOn() passing a valid indexOid in a way that the
function would actually ever execute the code in question.  So, for lack
of a way to test if a fix actually works, let's just remove the dead code
instead.

As a reminder, if there is ever a need to resurrect this code, an Assert()
has been added to remind future feature developers that they might need to
write some code to find the corresponding child index.

Reported-by: Justin Pryzby
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/20220819211824[email protected]