Avoid corner-case memory leak in SSL parameter processing.

After reading the root cert list from the ssl_ca_file, immediately
install it as client CA list of the new SSL context.  That gives the
SSL context ownership of the list, so that SSL_CTX_free will free it.
This avoids a permanent memory leak if we fail further down in
be_tls_init(), which could happen if bogus CRL data is offered.

The leak could only amount to something if the CRL parameters get
broken after server start (else we'd just quit) and then the server
is SIGHUP'd many times without fixing the CRL data.  That's rather
unlikely perhaps, but it seems worth fixing, if only because the
code is clearer this way.

While we're here, add some comments about the memory management
aspects of this logic.

Noted by Jelte Fennema and independently by Andres Freund.
Back-patch to v10; before commit de41869b6 it doesn't matter,
since we'd not re-execute this code during SIGHUP.

Discussion: https://postgr.es/m/16160-18367e56e9a28264@postgresql.org

Fix race condition in psql \e's detection of file modification.

psql's editing commands decide whether the user has edited the file
by checking for change of modification timestamp.  This is probably
fine for a pre-existing file, but with a temporary file that is
created within the command, it's possible for a fast typist to
save-and-exit in less than the one-second granularity of stat(2)
timestamps.  On Windows FAT filesystems the granularity is even
worse, 2 seconds, making the race a bit easier to hit.

To fix, try to set the temp file's mod time to be two seconds ago.
It's unlikely this would fail, but then again the race condition
itself is unlikely, so just ignore any error.

Also, we might as well check the file size as well as its mod time.

While this is a difficult bug to hit, it still seems worth
back-patching, to ensure that users' edits aren't lost.

Laurenz Albe, per gripe from Jacob Champion; based on fix suggestions
from Jacob and myself

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

Forbid marking an identity column as nullable.

GENERATED ALWAYS AS IDENTITY implies NOT NULL, but the code failed
to complain if you overrode that with "GENERATED ALWAYS AS IDENTITY
NULL".  One might think the old behavior was a feature, but it was
inconsistent because the outcome varied depending on the order of
the clauses, so it seems to have been just an oversight.

Per bug #16913 from Pavel Boev.  Back-patch to v10 where identity
columns were introduced.

Vik Fearing (minor tweaks by me)

Discussion: https://postgr.es/m/16913-3b5198410f67d8c6@postgresql.org

Restore vacuum_cleanup_index_scale_factor coverage.

Revert two recent commits that had btree_index.sql drop regression test
indexes rather than leave them behind for pg_dump testing.

This is intended to restore pg_upgrade coverage of indexes with the
vacuum_cleanup_index_scale_factor storage parameter set on buildfarm
member crake.

Backpatch: 11-12 only

Re-simplify management of inStart in pqParseInput3's subroutines.

Commit 92785dac2 copied some logic related to advancement of inStart
from pqParseInput3 into getRowDescriptions and getAnotherTuple,
because it wanted to allow user-defined row processor callbacks to
potentially longjmp out of the library, and inStart would have to be
updated before that happened to avoid an infinite loop.  We later
decided that that API was impossibly fragile and reverted it, but
we didn't undo all of the related code changes, and this bit of
messiness survived.  Undo it now so that there's just one place in
pqParseInput3's processing where inStart is advanced; this will
simplify addition of better tracing support.

getParamDescriptions had grown similar processing somewhere along
the way (not in 92785dac2; I didn't track down just when), but it's
actually buggy because its handling of corrupt-message cases seems to
have been copied from the v2 logic where we lacked a known message
length.  The cases where we "goto not_enough_data" should not simply
return EOF, because then we won't consume the message, potentially
creating an infinite loop.  That situation now represents a
definitively corrupt message, and we should report it as such.

Although no field reports of getParamDescriptions getting stuck in
a loop have been seen, it seems appropriate to back-patch that fix.
I chose to back-patch all of this to keep the logic looking more alike
in supported branches.

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

Drop other index behind pg_upgrade test issue.

Fix the test failure by dropping the index in question.  Missed by
commit 57ae7885.

Per buildfarm member crake.

Backpatch: 11-12 only

Drop index behind pg_upgrade test issue.

The vacuum_cleanup_index_scale_factor storage parameter was set in a
btree index that was previously left behind in the regression test
database.  As a result, the index gets tested within pg_dump and
pg_restore tests, as well as pg_upgrade testing.  This won't work when
upgrading to Postgres 14, though, because the storage parameter was
removed on that version by commit 9f3665fb.

Fix the test failure by dropping the index in question.

Per buildfarm member crake.

Discussion: https://postgr.es/m/CAH2-WzmeXYBWdhF7BMhNjhq9exsk=E1ohqBFAwzPdXJZ1XDMUA@mail.gmail.com
Backpatch: 11-12 only

tutorial:  land height is "elevation", not "altitude"

This is a follow-on patch to 92c12e46d5.  In that patch, we renamed
"altitude" to "elevation" in the docs, based on these details:

   https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude

This renames the tutorial SQL files to match the documentation.

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

Backpatch-through: 9.6

Doc: improve introductory information about procedures.

Clarify the discussion in "User-Defined Procedures", by laying out
the key differences between functions and procedures in a bulleted
list.  Notably, this avoids burying the lede about procedures being
able to do transaction control.  Make the back-link in the CREATE
FUNCTION reference page more prominent, and add one in CREATE
PROCEDURE.

Per gripe from Guyren Howe.  Thanks to David Johnston for discussion.

Discussion: https://postgr.es/m/BYAPR03MB4903C53A8BB7EFF5EA289674A6949@BYAPR03MB4903.namprd03.prod.outlook.com

Validate the OID argument of pg_import_system_collations().

"SELECT pg_import_system_collations(0)" caused an assertion failure.
With a random nonzero argument --- or indeed with zero, in non-assert
builds --- it would happily make pg_collation entries with garbage
values of collnamespace.  These are harmless as far as I can tell
(unless maybe the OID happens to become used for a schema, later on?).
In any case this isn't a security issue, since the function is
superuser-only.  But it seems like a gotcha for unwary DBAs, so let's
add a check that the given OID belongs to some schema.

Back-patch to v10 where this function was introduced.

Clarify the usage of max_replication_slots on the subscriber side.

It was not clear in the docs that the max_replication_slots is also used
to track replication origins on the subscriber side.

Author: Paul Martinez
Reviewed-by: Amit Kapila
Backpatch-through: 10 where logical replication was introduced
Discussion: https://postgr.es/m/CACqFVBZgwCN_pHnW6dMNCrOS7tiHCw6Retf_=U2Vvj3aUSeATw@mail.gmail.com

Use native path separators to pg_ctl in initdb

On Windows, CMD.EXE allegedly does not run a command that uses forward slashes,
so let's convert the path to use backslashes instead.

Backpatch to 10.

Author: Nitin Jadhav 
Reviewed-by: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/CAMm1aWaNDuaPYFYMAqDeJrZmPtNvLcJRS++CcZWY8LT6KcoBZw@mail.gmail.com

Fix duplicated test case in TAP tests of reindexdb

The same test for REINDEX (VERBOSE) was done twice, while it is clear
that the second test should use --concurrently.  Issue introduced in
5dc92b8, for what looks like a copy-paste mistake.

Reviewed-by: Mark Dilger
Discussion: https://postgr.es/m/A7AE97EA-F4B0-4CAB-8FFF-3FECD31F9D63@enterprisedb.com
Backpatch-through: 12

Fix use-after-free bug with AfterTriggersTableData.storeslot

AfterTriggerSaveEvent() wrongly allocates the slot in execution-span
memory context, whereas the correct thing is to allocate it in
a transaction-span context, because that's where the enclosing
AfterTriggersTableData instance belongs into.

Backpatch to 12 (the test back to 11, where it works well with no code
changes, and it's good to have to confirm that the case was previously
well supported); this bug seems introduced by commit ff11e7f4b9ae.

Reported-by: Bertrand Drouvot
Author: Amit Langote 
Discussion: https://postgr.es/m/39a71864-b120-5a5c-8cc5-c632b6f16761@amazon.com

Doc: further clarify libpq's description of connection string URIs.

Break the synopsis into named parts to make it less confusing.
Make more than zero effort at applying SGML markup.  Do a bit
of copy-editing of nearby text.

The synopsis revision is by Alvaro Herrera and Paul Förster,
the rest is my fault.  Back-patch to v10 where multi-host
connection strings appeared.

Discussion: https://postgr.es/m/6E752D6B-487C-463E-B6E2-C32E7FB007EA@gmail.com

doc: Mention PGDATABASE as supported by pgbench

PGHOST, PGPORT and PGUSER were already mentioned, but not PGDATABASE.
Like 5aaa584, backpatch down to 12.

Reported-by: Christophe Courtois
Discussion: https://postgr.es/m/161399398648.21711.15387267201764682579@wrigleys.postgresql.org
Backpatch-through: 12

Fix some typos, grammar and style in docs and comments

The portions fixing the documentation are backpatched where needed.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20210210235557[email protected]
backpatch-through: 9.6

Reinstate HEAP_XMAX_LOCK_ONLY|HEAP_KEYS_UPDATED as allowed

Commit 866e24d47db1 added an assert that HEAP_XMAX_LOCK_ONLY and
HEAP_KEYS_UPDATED cannot appear together, on the faulty assumption that
the latter necessarily referred to an update and not a tuple lock; but
that's wrong, because SELECT FOR UPDATE can use precisely that
combination, as evidenced by the amcheck test case added here.

Remove the Assert(), and also patch amcheck's verify_heapam.c to not
complain if the combination is found.  Also, out of overabundance of
caution, update (across all branches) README.tuplock to be more explicit
about this.

Author: Julien Rouhaud 
Reviewed-by: Mahendra Singh Thalor
Reviewed-by: Dilip Kumar
Discussion: https://postgr.es/m/20210124061758.GA11756@nol

Fix psql's ON_ERROR_ROLLBACK so that it handles COMMIT AND CHAIN.

When ON_ERROR_ROLLBACK is enabled, psql releases a temporary savepoint
if it's idle in a valid transaction block after executing a query. But psql
doesn't do that after RELEASE or ROLLBACK is executed because a temporary
savepoint has already been destroyed in that case.

This commit changes psql's ON_ERROR_ROLLBACK so that it doesn't release
a temporary savepoint also when COMMIT AND CHAIN is executed. A temporary
savepoint doesn't need to be released in that case because
COMMIT AND CHAIN also destroys any savepoints defined within the transaction
to commit. Otherwise psql tries to release the savepoint that
COMMIT AND CHAIN has already destroyed and cause an error
"ERROR:  savepoint "pg_psql_temporary_savepoint" does not exist".

Back-patch to v12 where transaction chaining was added.

Reported-by: Arthur Nascimento
Author: Arthur Nascimento
Reviewed-by: Fujii Masao, Vik Fearing
Discussion: https://postgr.es/m/16867-3475744069228158@postgresql.org

Fix bug in COMMIT AND CHAIN command.

This commit fixes COMMIT AND CHAIN command so that it starts new transaction
immediately even if savepoints are defined within the transaction to commit.
Previously COMMIT AND CHAIN command did not in that case because
commit 280a408b48 forgot to make CommitTransactionCommand() handle
a transaction chaining when the transaction state was TBLOCK_SUBCOMMIT.

Also this commit adds the regression test for COMMIT AND CHAIN command
when savepoints are defined.

Back-patch to v12 where transaction chaining was added.

Reported-by: Arthur Nascimento
Author: Fujii Masao
Reviewed-by: Arthur Nascimento, Vik Fearing
Discussion: https://postgr.es/m/16867-3475744069228158@postgresql.org

Fix another ancient bug in parsing of BRE-mode regular expressions.

While poking at the regex code, I happened to notice that the bug
squashed in commit afcc8772e had a sibling: next() failed to return
a specific value associated with the '}' token for a "\{m,n\}"
quantifier when parsing in basic RE mode.  Again, this could result
in treating the quantifier as non-greedy, which it never should be in
basic mode.  For that to happen, the last character before "\}" that
sets "nextvalue" would have to set it to zero, or it'd have to have
accidentally been zero from the start.  The failure can be provoked
repeatably with, for example, a bound ending in digit "0".

Like the previous patch, back-patch all the way.

Fix typo

Author: Daniel Gustafsson 
Discussion: https://postgr.es/m/0CF087FC-BEAD-4010-8BB9-3CDD74DC9060@yesql.se

Make ExecGetInsertedCols() and friends more robust and improve comments.

If ExecGetInsertedCols(), ExecGetUpdatedCols() or ExecGetExtraUpdatedCols()
were called with a ResultRelInfo that's not in the range table and isn't a
partition routing target, the functions would dereference a NULL pointer,
relinfo->ri_RootResultRelInfo. Such ResultRelInfos are created when firing
RI triggers in tables that are not modified directly. None of the current
callers of these functions pass such relations, so this isn't a live bug,
but let's make them more robust.

Also update comment in ResultRelInfo; after commit 6214e2b228,
ri_RangeTableIndex is zero for ResultRelInfos created for partition tuple
routing.

Noted by Coverity. Backpatch down to v11, like commit 6214e2b228.

Reviewed-by: Tom Lane, Amit Langote

Default to wal_sync_method=fdatasync on FreeBSD.

FreeBSD 13 gained O_DSYNC, which would normally cause wal_sync_method to
choose open_datasync as its default value.  That may not be a good
choice for all systems, and performs worse than fdatasync in some
scenarios.  Let's preserve the existing default behavior for now.

Like commit 576477e73c4, which did the same for Linux, back-patch to all
supported releases.

Discussion: https://postgr.es/m/CA%2BhUKGLsAMXBQrCxCXoW-JsUYmdOL8ALYvaX%3DCrHqWxm-nWbGA%40mail.gmail.com

Hold interrupts while running dsm_detach() callbacks.

While cleaning up after a parallel query or parallel index creation that
created temporary files, we could be interrupted by a statement timeout.
The error handling path would then fail to clean up the files when it
ran dsm_detach() again, because the callback was already popped off the
list.  Prevent this hazard by holding interrupts while the cleanup code
runs.

Thanks to Heikki Linnakangas for this suggestion, and also to Kyotaro
Horiguchi, Masahiko Sawada, Justin Pryzby and Tom Lane for discussion of
this and earlier ideas on how to fix the problem.

Back-patch to all supported releases.

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

pg_attribute_no_sanitize_alignment() macro

Modern gcc and clang compilers offer alignment sanitizers, which help to detect
pointer misalignment.  However, our codebase already contains x86-specific
crc32 computation code, which uses unalignment access.  Thankfully, those
compilers also support the attribute, which disables alignment sanitizers at
the function level.  This commit adds pg_attribute_no_sanitize_alignment(),
which wraps this attribute, and applies it to pg_comp_crc32c_sse42() function.

Back-patch of commits 993bdb9f9 and ad2ad698a, to enable doing
alignment testing in all supported branches.

Discussion: https://postgr.es/m/CAPpHfdsne3%3DT%3DfMNU45PtxdhSL_J2PjLTeS8rwKnJzUR4YNd4w%40mail.gmail.com
Discussion: https://postgr.es/m/475514.1612745257%40sss.pgh.pa.us
Author: Alexander Korotkov, revised by Tom Lane
Reviewed-by: Tom Lane

Avoid divide-by-zero in regex_selectivity() with long fixed prefix.

Given a regex pattern with a very long fixed prefix (approaching 500
characters), the result of pow(FIXED_CHAR_SEL, fixed_prefix_len) can
underflow to zero.  Typically the preceding selectivity calculation
would have underflowed as well, so that we compute 0/0 and get NaN.
In released branches this leads to an assertion failure later on.
That doesn't happen in HEAD, for reasons I've not explored yet,
but it's surely still a bug.

To fix, just skip the division when the pow() result is zero, so
that we'll (most likely) return a zero selectivity estimate.  In
the edge cases where "sel" didn't yet underflow, perhaps this
isn't desirable, but I'm not sure that the case is worth spending
a lot of effort on.  The results of regex_selectivity_sub() are
barely worth the electrons they're written on anyway :-(

Per report from Alexander Lakhin.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/6de0a0c3-ada9-cd0c-3e4e-2fa9964b41e3@gmail.com

Fix ORDER BY clause in new regression test of REINDEX CONCURRENTLY

Oversight in bd12080.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20210210065805[email protected]
Backpatch-through: 12

Preserve pg_attribute.attstattarget across REINDEX CONCURRENTLY

For an index, attstattarget can be updated using ALTER INDEX SET
STATISTICS.  This data was lost on the new index after REINDEX
CONCURRENTLY.

The update of this field is done when the old and new indexes are
swapped to make the fix back-patchable.  Another approach we could look
after in the long-term is to change index_create() to pass the wanted
values of attstattarget when creating the new relation, but, as this
would cause an ABI breakage this can be done only on HEAD.

Reported-by: Ronan Dunklau
Author: Michael Paquier
Reviewed-by: Ronan Dunklau, Tomas Vondra
Discussion: https://postgr.es/m/16628084.uLZWGnKmhe@laptop-ronand
Backpatch-through: 12

Stamp 12.6.

Translation updates

Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 08f1c10dca3d7b8efc365107c737b87c1c3a82ee

Last-minute updates for release notes.

Security: CVE-2021-3393, CVE-2021-20229

Fix permission checks on constraint violation errors on partitions.

If a cross-partition UPDATE violates a constraint on the target partition,
and the columns in the new partition are in different physical order than
in the parent, the error message can reveal columns that the user does not
have SELECT permission on. A similar bug was fixed earlier in commit
804b6b6db4.

The cause of the bug is that the callers of the
ExecBuildSlotValueDescription() function got confused when constructing
the list of modified columns. If the tuple was routed from a parent, we
converted the tuple to the parent's format, but the list of modified
columns was grabbed directly from the child's RTE entry.

ExecUpdateLockMode() had a similar issue. That lead to confusion on which
columns are key columns, leading to wrong tuple lock being taken on tables
referenced by foreign keys, when a row is updated with INSERT ON CONFLICT
UPDATE. A new isolation test is added for that corner case.

With this patch, the ri_RangeTableIndex field is no longer set for
partitions that don't have an entry in the range table. Previously, it was
set to the RTE entry of the parent relation, but that was confusing.

NOTE: This modifies the ResultRelInfo struct, replacing the
ri_PartitionRoot field with ri_RootResultRelInfo. That's a bit risky to
backpatch, because it breaks any extensions accessing the field. The
change that ri_RangeTableIndex is not set for partitions could potentially
break extensions, too. The ResultRelInfos are visible to FDWs at least,
and this patch required small changes to postgres_fdw. Nevertheless, this
seem like the least bad option. I don't think these fields widely used in
extensions; I don't think there are FDWs out there that uses the FDW
"direct update" API, other than postgres_fdw. If there is, you will get a
compilation error, so hopefully it is caught quickly.

Backpatch to 11, where support for both cross-partition UPDATEs, and unique
indexes on partitioned tables, were added.

Reviewed-by: Amit Langote
Security: CVE-2021-3393

Release notes for 13.2, 12.6, 11.11, 10.16, 9.6.21, 9.5.25.

Revert "Propagate CTE property flags when copying a CTE list into a rule."

This reverts commit ed290896335414c6c069b9ccae1f3dcdd2fac6ba and
equivalent back-branch commits.  The issue is subtler than I thought,
and it's far from new, so just before a release deadline is no time
to be fooling with it.  We'll consider what to do at a bit more
leisure.

Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com

Propagate CTE property flags when copying a CTE list into a rule.

rewriteRuleAction() neglected this step, although it was careful to
propagate other similar flags such as hasSubLinks or hasRowSecurity.
Omitting to transfer hasRecursive is just cosmetic at the moment,
but omitting hasModifyingCTE is a live bug, since the executor
certainly looks at that.

The proposed test case only fails back to v10, but since the executor
examines hasModifyingCTE in 9.x as well, I suspect that a test case
could be devised that fails in older branches.  Given the nearness
of the release deadline, though, I'm not going to spend time looking
for a better test.

Report and patch by Greg Nancarrow, cosmetic changes by me

Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com

Disallow converting an inheritance child table to a view.

Generally, members of inheritance trees must be plain tables (or,
in more recent versions, foreign tables).  ALTER TABLE INHERIT
rejects creating an inheritance relationship that has a view at
either end.  When DefineQueryRewrite attempts to convert a relation
to a view, it already had checks prohibiting doing so for partitioning
parents or children as well as traditional-inheritance parents ...
but it neglected to check that a traditional-inheritance child wasn't
being converted.  Since the planner assumes that any inheritance
child is a table, this led to making plans that tried to do a physical
scan on a view, causing failures (or even crashes, in recent versions).

One could imagine trying to support such a case by expanding the view
normally, but since the rewriter runs before the planner does
inheritance expansion, it would take some very fundamental refactoring
to make that possible.  There are probably a lot of other parts of the
system that don't cope well with such a situation, too.  For now,
just forbid it.

Per bug #16856 from Yang Lin.  Back-patch to all supported branches.
(In versions before v10, this includes back-patching the portion of
commit 501ed02cf that added has_superclass().  Perhaps the lack of
that infrastructure partially explains the missing check.)

Discussion: https://postgr.es/m/16856-0363e05c6e1612fd@postgresql.org

Fix backslash-escaping multibyte chars in COPY FROM.

If a multi-byte character is escaped with a backslash in TEXT mode input,
and the encoding is one of the client-only encodings where the bytes after
the first one can have an ASCII byte "embedded" in the char, we didn't
skip the character correctly. After a backslash, we only skipped the first
byte of the next character, so if it was a multi-byte character, we would
try to process its second byte as if it was a separate character. If it
was one of the characters with special meaning, like '\n', '\r', or
another '\\', that would cause trouble.

One such exmple is the byte sequence '\x5ca45c2e666f6f' in Big5 encoding.
That's supposed to be [backslash][two-byte character][.][f][o][o], but
because the second byte of the two-byte character is 0x5c, we incorrectly
treat it as another backslash. And because the next character is a dot, we
parse it as end-of-copy marker, and throw an "end-of-copy marker corrupt"
error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi

Avoid crash when rolling back within a prepared statement.

If a portal is used to run a prepared CALL or DO statement that
contains a ROLLBACK, PortalRunMulti fails because the portal's
statement list gets cleared by the rollback.  (Since the grammar
doesn't allow CALL/DO in PREPARE, the only easy way to get to this is
via extended query protocol, which treats all inputs as prepared
statements.)  It's difficult to avoid resetting the portal early
because of resource-management issues, so work around this by teaching
PortalRunMulti to be wary of portal->stmts having suddenly become NIL.

The crash has only been seen to occur in v13 and HEAD (as a
consequence of commit 1cff1b95a having added an extra touch of
portal->stmts).  But even before that, the code involved touching a
List that the portal no longer has any claim on.  In the test case at
hand, the List will still exist because of another refcount on the
cached plan; but I'm far from convinced that it's impossible for the
cached plan to have been dropped by the time control gets back to
PortalRunMulti.  Hence, backpatch to v11 where nested transactions
were added.

Thomas Munro and Tom Lane, per bug #16811 from James Inform

Discussion: https://postgr.es/m/16811-c1b599b2c6c2d622@postgresql.org

pg_dump: Fix dumping of inherited generated columns

Generation expressions of generated columns are always inherited, so
there is no need to set them separately in child tables, and there is
no syntax to do so either.  The code previously used the code paths
for the handling of default values, for which different rules apply;
in particular it might want to set a default value explicitly for an
inherited column.  This resulted in unrestorable dumps.  For generated
columns, just skip them in inherited tables.

Reviewed-by: Tom Lane
Discussion: https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us

Remove extra increment of plpgsql's statement counter for FOR loops.

This left gaps in the internal statement numbering, which is not
terribly harmful (else we'd have noticed sooner), but it's not
great either.

Oversight in bbd5c207b; backpatch to v12 where that came in.

Pavel Stehule

Discussion: https://postgr.es/m/CAFj8pRDXyQaJmpotNTQVc-t-WxdWZC35V2PnmwOaV1-taidFWA@mail.gmail.com

Fix ancient memory leak in contrib/auto_explain.

The ExecutorEnd hook is invoked in a context that could be quite
long-lived, not the executor's own per-query context as I think
we were sort of assuming.  Thus, any cruft generated while producing
the EXPLAIN output could accumulate over multiple queries.  This can
result in spectacular leakage if log_nested_statements is on, and
even without that I'm surprised nobody complained before.

To fix, just switch into the executor's context so that anything we
allocate will be released when standard_ExecutorEnd frees the executor
state.  We might as well nuke the code's retail pfree of the explain
output string, too; that's laughably inadequate to the need.

Japin Li, per report from Jeff Janes.  This bug is old, so
back-patch to all supported branches.

Discussion: https://postgr.es/m/CAMkU=1wCVtbeRn0s9gt12KwQ7PLXovbpM8eg25SYocKW3BT4hg@mail.gmail.com

Fix CREATE INDEX CONCURRENTLY for simultaneous prepared transactions.

In a cluster having used CREATE INDEX CONCURRENTLY while having enabled
prepared transactions, queries that use the resulting index can silently
fail to find rows.  Fix this for future CREATE INDEX CONCURRENTLY by
making it wait for prepared transactions like it waits for ordinary
transactions.  This expands the VirtualTransactionId structure domain to
admit prepared transactions.  It may be necessary to reindex to recover
from past occurrences.  Back-patch to 9.5 (all supported versions).

Andrey Borodin, reviewed (in earlier versions) by Tom Lane and Michael
Paquier.

Discussion: https://postgr.es/m/2E712143-97F7-4890-B470-4A35142ABC82@yandex-team.ru

Document behavior of the .** jsonpath accessor in the lax mode

When the .** jsonpath accessor handles the array, it selects both array and
each of its elements.  When using lax mode, subsequent accessors automatically
unwrap arrays.  So, the content of each array element may be selected twice.

Even though this behavior is counterintuitive, it's correct because everything
works as designed.  This commit documents it.

Backpatch to 12 where the jsonpath language was introduced.

Reported-by: Thomas Kellerer
Bug: #16828
Discussion: https://postgr.es/m/16828-2b0229babfad2d8c%40postgresql.org
Discussion: https://postgr.es/m/CAPpHfdtS-nNidT%3DEqZbAYOPcnNOWh_sd6skVdu2CAQUGdvpT8Q%40mail.gmail.com
Author: Alexandex Korotkov, revised by Tom Lane
Reviewed-by: Alvaro Herrera, Thomas Kellerer, Tom Lane
Backpatch-through: 12

Silence another gcc 11 warning.

Per buildfarm and local experimentation, bleeding-edge gcc isn't
convinced that the MemSet in reorder_function_arguments() is safe.
Shut it up by adding an explicit check that pronargs isn't negative,
and by changing MemSet to memset.  (It appears that either change is
enough to quiet the warning at -O2, but let's do both to be sure.)

Fix hash partition pruning with asymmetric partition sets.

perform_pruning_combine_step() was not taught about the number of
partition indexes used in hash partitioning; more embarrassingly,
get_matching_hash_bounds() also had it wrong.  These errors are masked
in the common case where all the partitions have the same modulus
and no partition is missing.  However, with missing or unequal-size
partitions, we could erroneously prune some partitions that need
to be scanned, leading to silently wrong query answers.

While a minimal-footprint fix for this could be to export
get_partition_bound_num_indexes and make the incorrect functions use it,
I'm of the opinion that that function should never have existed in the
first place.  It's not reasonable data structure design that
PartitionBoundInfoData lacks any explicit record of the length of
its indexes[] array.  Perhaps that was all right when it could always
be assumed equal to ndatums, but something should have been done about
it as soon as that stopped being true.  Putting in an explicit
"nindexes" field makes both partition_bounds_equal() and
partition_bounds_copy() simpler, safer, and faster than before,
and removes explicit knowledge of the number-of-partition-indexes
rules from some other places too.

This change also makes get_hash_partition_greatest_modulus obsolete.
I left that in place in case any external code uses it, but no core
code does anymore.

Per bug #16840 from Michał Albrycht.  Back-patch to v11 where the
hash partitioning code came in.  (In the back branches, add the new
field at the end of PartitionBoundInfoData to minimize ABI risks.)

Discussion: https://postgr.es/m/16840-571a22976f829ad4@postgresql.org

Make ecpg's rjulmdy() and rmdyjul() agree with their declarations.

We had "short *mdy" in the extern declarations, but "short mdy[3]"
in the actual function definitions.  Per C99 these are equivalent,
but recent versions of gcc have started to issue warnings about
the inconsistency.  Clean it up before the warnings get any more
widespread.

Back-patch, in case anyone wants to build older PG versions with
bleeding-edge compilers.

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

pgbench: Remove dead code

doConnect() never returns connections in state CONNECTION_BAD, so
checking for that is pointless.  Remove the code that does.

This code has been dead since ba708ea3dc84, 20 years ago.

Discussion: https://postgr.es/m/20210126195224[email protected]
Reviewed-by: Tom Lane

Don't add bailout adjustment for non-strict deserialize calls.

When building aggregate expression steps, strict checks need a bailout
jump for when a null value is encountered, so there is a list of steps
that require later adjustment. Adding entries to that list for steps
that aren't actually strict would be harmless, except that there is an
Assert which catches them. This leads to spurious errors on asserts
builds, for data sets that trigger parallel aggregation of an
aggregate with a non-strict deserialization function (no such
aggregates exist in the core system).

Repair by not adding the adjustment entry when it's not needed.

Backpatch back to 11 where the code was introduced.

Per a report from Darafei (Komzpa) of the PostGIS project; analysis
and patch by me.

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

doc: Remove reference to views for TRUNCATE privilege

The page about privilege rights mentioned that TRUNCATE could be applied
to views or even other relation types.  This is confusing as this
command can be used only on tables and on partitioned tables.

Oversight in afc4a78.

Reported-by: Harisai Hari
Reviewed-by: Laurenz Albe
Discussion: https://postgr.es/m/161157636877.14625.15340884663716426087@wrigleys.postgresql.org
Backpatch-through: 12

Report the true database name on connection errors

When reporting connection errors, we might show a database name in the
message that's not the one we actually tried to connect to, if the
database was taken from libpq defaults instead of from user parameters.
Fix such error messages to use PQdb(), which reports the correct name.

(But, per commit 2930c05634bc, make sure not to try to print NULL.)

Apply to branches 9.5 through 13.  Branch master has already been
changed differently by commit 58cd8dca3de0.

Reported-by: Robert Haas
Discussion: https://postgr.es/m/CA+TgmobssJ6rS22dspWnu-oDxXevGmhMD8VcRBjmj-b9UDqRjw@mail.gmail.com

Code review for psql's helpSQL() function.

The loops to identify word boundaries could access past the end of
the input string.  Likely that would never result in an actual
crash, but it makes valgrind unhappy.

The logic to try different numbers of words didn't work when the
input has two words but we only have a match to the first, eg
"\h with select".  (We must "continue" the pass loop, not "break".)

The logic to compute nl_count was bizarrely managed, and in at
least two code paths could end up calling PageOutput with
nl_count = 0, resulting in failing to paginate output that should
have been fed to the pager.  Also, in v12 and up, the nl_count
calculation hadn't been updated to account for the addition of a URL.

The PQExpBuffer holding the command syntax details wasn't freed,
resulting in a session-lifespan memory leak.

While here, improve some comments, choose a more descriptive name
for a variable, fix inconsistent datatype choice for another variable.

Per bug #16837 from Alexander Lakhin.  This code is very old,
so back-patch to all supported branches.

Kyotaro Horiguchi and Tom Lane

Discussion: https://postgr.es/m/16837-479bcd56040c71b3@postgresql.org

Don't clobber the calling user's credentials cache in Kerberos test.

Embarrassing oversight in this test script, which fortunately is not
run by default.

Report and patch by Jacob Champion.

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

Fix broken ruleutils support for function TRANSFORM clauses.

I chanced to notice that this dumped core due to a faulty Assert.
To add insult to injury, the output has been misformatted since v11.
Obviously we need some regression testing here.

Discussion: https://postgr.es/m/d1cc628c-3953-4209-957b-29427acc38c8@www.fastmail.com

Fix hypothetical bug in heap backward scans

Both heapgettup() and heapgettup_pagemode() incorrectly set the first page
to scan in a backward scan in which the number of pages to scan was
specified by heap_setscanlimits().  The code incorrectly started the scan
at the end of the relation when startBlk was 0, or otherwise at
startBlk - 1, neither of which is correct when only scanning a subset of
pages.

The fix here checks if heap_setscanlimits() has changed the number of
pages to scan and if so we set the first page to scan as the final page in
the specified range during backward scans.

Proper adjustment of this code was forgotten when heap_setscanlimits() was
added in 7516f5259 back in 9.5.  However, practice, nowhere in core code
performs backward scans after having used heap_setscanlimits(), yet, it is
possible an extension uses the heap functions in this way, hence
backpatch.

An upcoming patch does use heap_setscanlimits() with backward scans, so
this must be fixed before that can go in.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvpGc9h0_oVD2CtgBcxCS1N-qDYZSeBRnUh+0CWJA9cMaA@mail.gmail.com
Backpatch-through: 9.5, all supported versions

Update time zone data files to tzdata release 2021a.

DST law changes in Russia (Volgograd zone) and South Sudan.
Historical corrections for Australia, Bahamas, Belize, Bermuda,
Ghana, Israel, Kenya, Nigeria, Palestine, Seychelles, and Vanuatu.
Notably, the Australia/Currie zone has been corrected to the point
where it is identical to Australia/Hobart.

Doc: improve directions for building on macOS.

In light of recent discussions, we should instruct people to
install Apple's command line tools; installing Xcode is secondary.

Also, fix sample command for finding out the default sysroot,
as we now know that the command originally recommended can give
a result that doesn't match your OS version.

Also document the workaround to use if you really don't want
configure to select a sysroot at all.

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

Doc: remove misleading claim in documentation of PQreset().

This text claimed that the reconnection would occur "to the same
server", but there is no such guarantee in the code, nor would
insisting on that be an improvement.

Back-patch to v10 where multi-host connection strings were added.

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

Fix pull_varnos' miscomputation of relids set for a PlaceHolderVar.

Previously, pull_varnos() took the relids of a PlaceHolderVar as being
equal to the relids in its contents, but that fails to account for the
possibility that we have to postpone evaluation of the PHV due to outer
joins.  This could result in a malformed plan.  The known cases end up
triggering the "failed to assign all NestLoopParams to plan nodes"
sanity check in createplan.c, but other symptoms may be possible.

The right value to use is the join level we actually intend to evaluate
the PHV at.  We can get that from the ph_eval_at field of the associated
PlaceHolderInfo.  However, there are some places that call pull_varnos()
before the PlaceHolderInfos have been created; in that case, fall back
to the conservative assumption that the PHV will be evaluated at its
syntactic level.  (In principle this might result in missing some legal
optimization, but I'm not aware of any cases where it's an issue in
practice.)  Things are also a bit ticklish for calls occurring during
deconstruct_jointree(), but AFAICS the ph_eval_at fields should have
reached their final values by the time we need them.

The main problem in making this work is that pull_varnos() has no
way to get at the PlaceHolderInfos.  We can fix that easily, if a
bit tediously, in HEAD by passing it the planner "root" pointer.
In the back branches that'd cause an unacceptable API/ABI break for
extensions, so leave the existing entry points alone and add new ones
with the additional parameter.  (If an old entry point is called and
encounters a PHV, it'll fall back to using the syntactic level,
again possibly missing some valid optimization.)

Back-patch to v12.  The computation is surely also wrong before that,
but it appears that we cannot reach a bad plan thanks to join order
restrictions imposed on the subquery that the PlaceHolderVar came from.
The error only became reachable when commit 4be058fe9 allowed trivial
subqueries to be collapsed out completely, eliminating their join order
restrictions.

Per report from Stephan Springl.

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

Further tweaking of PG_SYSROOT heuristics for macOS.

It emerges that in some phases of the moon (perhaps to do with
directory entry order?), xcrun will report that the SDK path is
  /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk
which is normally a symlink to a version-numbered sibling directory.
Our heuristic to skip non-version-numbered pathnames was rejecting
that, which is the wrong thing to do.  We'd still like to end up
with a version-numbered PG_SYSROOT value, but we can have that by
dereferencing the symlink.

Like the previous fix, back-patch to all supported versions.

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

Disable vacuum page skipping in selected test cases.

By default VACUUM will skip pages that it can't immediately get
exclusive access to, which means that even activities as harmless
and unpredictable as checkpoint buffer writes might prevent a page
from being processed.  Ordinarily this is no big deal, but we have
a small number of test cases that examine the results of VACUUM's
processing and therefore will fail if the page of interest is skipped.
This seems to be the explanation for some rare buildfarm failures.
To fix, add the DISABLE_PAGE_SKIPPING option to the VACUUM commands
in tests where this could be an issue.

In passing, remove a duplicated query in pageinspect/sql/page.sql.

Back-patch as necessary (some of these cases are as old as v10).

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

Fix bug in detecting concurrent page splits in GiST insert

In commit 9eb5607e699, I got the condition on checking for split or
deleted page wrong: I used && instead of ||. The comment correctly said
"concurrent split _or_ deletion".

As a result, GiST insertion could miss a concurrent split, and insert to
wrong page. Duncan Sands demonstrated this with a test script that did a
lot of concurrent inserts.

Backpatch to v12, where this was introduced. REINDEX is required to fix
indexes that were affected by this bug.

Backpatch-through: 12
Reported-by: Duncan Sands
Discussion: https://www.postgresql.org/message-id/a9690483-6c6c-3c82-c8ba-dc1a40848f11%40deepbluecap.com

Fix ALTER DEFAULT PRIVILEGES with duplicated objects

Specifying duplicated objects in this command would lead to unique
constraint violations in pg_default_acl or "tuple already updated by
self" errors.  Similarly to GRANT/REVOKE, increment the command ID after
each subcommand processing to allow this case to work transparently.

A regression test is added by tweaking one of the existing queries of
privileges.sql to stress this case.

Reported-by: Andrus
Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/ae2a7dc1-9d71-8cba-3bb9-e4cb7eb1f44e@hot.ee
Backpatch-through: 9.5

Remove faulty support for MergeAppend plan with WHERE CURRENT OF.

Somebody extended search_plan_tree() to treat MergeAppend exactly
like Append, which is 100% wrong, because unlike Append we can't
assume that only one input node is actively returning tuples.
Hence a cursor using a MergeAppend across a UNION ALL or inheritance
tree could falsely match a WHERE CURRENT OF query at a row that
isn't actually the cursor's current output row, but coincidentally
has the same TID (in a different table) as the current output row.

Delete the faulty code; this means that such a case will now return
an error like 'cursor "foo" is not a simply updatable scan of table
"bar"', instead of silently misbehaving.  Users should not find that
surprising though, as the same cursor query could have failed that way
already depending on the chosen plan.  (It would fail like that if the
sort were done with an explicit Sort node instead of MergeAppend.)

Expand the clearly-inadequate commentary to be more explicit about
what this code is doing, in hopes of forestalling future mistakes.

It's been like this for awhile, so back-patch to all supported
branches.

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

doc:  adjust alignment of doc file list for "pg_waldump.sgml"

Backpatch-through: 10

Avoid crash with WHERE CURRENT OF and a custom scan plan.

execCurrent.c's search_plan_tree() assumed that ForeignScanStates
and CustomScanStates necessarily have a valid ss_currentRelation.
This is demonstrably untrue for postgres_fdw's remote join and
remote aggregation plans, and non-leaf custom scans might not have
an identifiable scan relation either.  Avoid crashing by ignoring
such nodes when the field is null.

This solution will lead to errors like 'cursor "foo" is not a
simply updatable scan of table "bar"' in cases where maybe we
could have allowed WHERE CURRENT OF to work.  That's not an issue
for postgres_fdw's usages, since joins or aggregations would render
WHERE CURRENT OF invalid anyway.  But an otherwise-transparent
upper level custom scan node might find this annoying.  When and if
someone cares to expend work on such a scenario, we could invent a
custom-scan-provider callback to determine what's safe.

Report and patch by David Geier, commentary by me.  It's been like
this for awhile, so back-patch to all supported branches.

Discussion: https://postgr.es/m/0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com

Fix pg_dump for GRANT OPTION among initial privileges.

The context is an object that no longer bears some aclitem that it bore
initially.  (A user issued REVOKE or GRANT statements upon the object.)
pg_dump is forming SQL to reproduce the object ACL.  Since initdb
creates no ACL bearing GRANT OPTION, reaching this bug requires an
extension where the creation script establishes such an ACL.  No PGXN
extension does that.  If an installation did reach the bug, pg_dump
would have omitted a semicolon, causing a REVOKE and the next SQL
statement to fail.  Separately, since the affected code exists to
eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT
OPTION FOR.  Back-patch to 9.6, where commit
23f34fa4ba358671adab16773e79c17c92cbc870 first appeared.

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

Prevent excess SimpleLruTruncate() deletion.

Every core SLRU wraps around.  With the exception of pg_notify, the wrap
point can fall in the middle of a page.  Account for this in the
PagePrecedes callback specification and in SimpleLruTruncate()'s use of
said callback.  Update each callback implementation to fit the new
specification.  This changes SerialPagePrecedesLogically() from the
style of asyncQueuePagePrecedes() to the style of CLOGPagePrecedes().
(Whereas pg_clog and pg_serial share a key space, pg_serial is nothing
like pg_notify.)  The bug fixed here has the same symptoms and user
followup steps as 592a589a04bd456410b853d86bd05faa9432cbbb.  Back-patch
to 9.5 (all supported versions).

Reviewed by Andrey Borodin and (in earlier versions) by Tom Lane.

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

Disallow CREATE STATISTICS on system catalogs

Add a check that CREATE STATISTICS does not add extended statistics on
system catalogs, similarly to indexes etc.  It can be overriden using
the allow_system_table_mods GUC.

This bug exists since 7b504eb282c, adding the extended statistics, so
backpatch all the way back to PostgreSQL 10.

Author: Tomas Vondra
Reported-by: Dean Rasheed
Backpatch-through: 10
Discussion: https://postgr.es/m/CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb%2BrQ%40mail.gmail.com

Improve our heuristic for selecting PG_SYSROOT on macOS.

In cases where Xcode is newer than the underlying macOS version,
asking xcodebuild for the SDK path will produce a pointer to the
SDK shipped with Xcode, which may end up building code that does
not work on the underlying macOS version.  It appears that in
such cases, xcodebuild's answer also fails to match the default
behavior of Apple's compiler: assuming one has installed Xcode's
"command line tools", there will be an SDK for the OS's own version
in /Library/Developer/CommandLineTools, and the compiler will
default to using that.  This is all pretty poorly documented,
but experimentation suggests that "xcrun --show-sdk-path" gives
the sysroot path that the compiler is actually using, at least
in some cases.  Hence, try that first, but revert to xcodebuild
if xcrun fails (in very old Xcode, it is missing or lacks the
--show-sdk-path switch).

Also, "xcrun --show-sdk-path" may give a path that is valid but lacks
any OS version identifier.  We don't really want that, since most
of the motivation for wiring -isysroot into the build flags at all
is to ensure that all parts of a PG installation are built against
the same SDK, even when considering extensions built later and/or on
a different machine.  Insist on finding "N.N" in the directory name
before accepting the result.  (Adding "--sdk macosx" to the xcrun
call seems to produce the same answer as xcodebuild, but usually
more quickly because it's cached, so we also try that as a fallback.)

The core reason why we don't want to use Xcode's default SDK in cases
like this is that Apple's technology for introducing new syscalls
does not play nice with Autoconf: for example, configure will think
that preadv/pwritev exist when using a Big Sur SDK, even when building
on an older macOS version where they don't exist.  It'd be nice to
have a better solution to that problem, but this patch doesn't attempt
to fix that.

Per report from Sergey Shinderuk.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/ed3b8e5d-0da8-6ebd-fd1c-e0ac80a4b204@postgrespro.ru

Fix calculation of how much shared memory is required to store a TOC.

Commit ac883ac453 refactored shm_toc_estimate() but changed its calculation
of shared memory size for TOC incorrectly. Previously this could cause too
large memory to be allocated.

Back-patch to v11 where the bug was introduced.

Author: Takayuki Tsunakawa
Discussion: https://postgr.es/m/TYAPR01MB2990BFB73170E2C4921E2C4DFEA80@TYAPR01MB2990.jpnprd01.prod.outlook.com

pg_dump: label PUBLICATION TABLE ArchiveEntries with an owner.

This is the same fix as commit 9eabfe300 applied to INDEX ATTACH
entries, but for table-to-publication attachments.  As in that
case, even though the backend doesn't record "ownership" of the
attachment, we still ought to label it in the dump archive with
the role name that should run the ALTER PUBLICATION command.
The existing behavior causes the ALTER to be done by the original
role that started the restore; that will usually work fine, but
there may be corner cases where it fails.

The bulk of the patch is concerned with changing struct
PublicationRelInfo to include a pointer to the associated
PublicationInfo object, so that we can get the owner's name
out of that when the time comes.  While at it, I rewrote
getPublicationTables() to do just one query of pg_publication_rel,
not one per table.

Back-patch to v10 where this code was introduced.

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

Prevent drop of tablespaces used by partitioned relations

When a tablespace is used in a partitioned relation (per commits
ca4103025dfe in pg12 for tables and 33e6c34c3267 in pg11 for indexes),
it is possible to drop the tablespace, potentially causing various
problems.  One such was reported in bug #16577, where a rewriting ALTER
TABLE causes a server crash.

Protect against this by using pg_shdepend to keep track of tablespaces
when used for relations that don't keep physical files; we now abort a
tablespace if we see that the tablespace is referenced from any
partitioned relations.

Backpatch this to 11, where this problem has been latent all along.  We
don't try to create pg_shdepend entries for existing partitioned
indexes/tables, but any ones that are modified going forward will be
protected.

Note slight behavior change: when trying to drop a tablespace that
contains both regular tables as well as partitioned ones, you'd
previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll
get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST.  Arguably, the latter is more
correct.

It is possible to add protecting pg_shdepend entries for existing
tables/indexes, by doing
  ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default;
  ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace;
for each partitioned table/index that is not in the database default
tablespace.  Because these partitioned objects do not have storage, no
file needs to be actually moved, so it shouldn't take more time than
what's required to acquire locks.

This query can be used to search for such relations:
SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16577-881633a9f9894fd5@postgresql.org
Author: Álvaro Herrera 
Reviewed-by: Michael Paquier

Call out vacuum considerations in create index docs

Backpatch to pg12, which is as far as it goes without conflicts.

Author: James Coleman 
Reviewed-by: "David G. Johnston"
Discussion: https://postgr.es/m/CAAaqYe9oEfbz7AxXq7OX+FFVi5w5p1e_Of8ON8ZnKO9QqBfmjg@mail.gmail.com

Disallow a digit as the first character of a variable name in pgbench.

The point of this restriction is to avoid trying to substitute variables
into timestamp literal values, which may contain strings like '12:34'.

There is a good deal more that should be done to reduce pgbench's
tendency to substitute where it shouldn't.  But this is sufficient to
solve the case complained of by Jaime Soler, and it's simple enough
to back-patch.

Back-patch to v11; before commit 9d36a3866, pgbench had a slightly
different definition of what a variable name is, and anyway it seems
unwise to change long-stable branches for this.

Fabien Coelho

Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2006291740420.805678@pseudo

Doc: clarify behavior of back-half options in pg_dump.

Options that change how the archive data is converted to SQL text
are ignored when dumping to archive formats.  The documentation
previously said "not meaningful", which is not helpful.

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

Fix memory leak in SnapBuildSerialize.

The memory for the snapshot was leaked while serializing it to disk during
logical decoding. This memory will be freed only once walsender stops
streaming the changes. This can lead to a huge memory increase when master
logs Standby Snapshot too frequently say when the user is trying to create
many replication slots.

Reported-by: [email protected]
Diagnosed-by: [email protected]
Author: Amit Kapila
Backpatch-through: 9.5
Discussion: https://postgr.es/m/033ab54c-6393-42ee-8ec9-2b399b5d8cde[email protected]

pg_dump: label INDEX ATTACH ArchiveEntries with an owner.

Although a partitioned index's attachment to its parent doesn't
have separate ownership, the ArchiveEntry for it needs to be
marked with an owner anyway, to ensure that the ALTER command
is run by the appropriate role when restoring with
--use-set-session-authorization.  Without this, the ALTER will
be run by the role that started the restore session, which will
usually work but it's formally the wrong thing.

Back-patch to v11 where this type of ArchiveEntry was added.
In HEAD, add equivalent commentary to the just-added TABLE ATTACH
case, which I'd made do the right thing already.

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

Doc: fix description of privileges needed for ALTER PUBLICATION.

Adding a table to a publication requires ownership of the table
(in addition to ownership of the publication).  This was mentioned
nowhere.

Fix thinko in comment

This comment has been wrong since its introduction in commit
2c03216d8311.

Author: Masahiko Sawada 
Discussion: https://postgr.es/m/CAD21AoAzz6qipFJBbGEaHmyWxvvNDp8httbwLR9tUQWaTjUs2Q@mail.gmail.com

doc:  expand description of how non-SELECT queries are processed

The previous description of how the executor processes non-SELECT
queries was very dense, causing lack of clarity.  This expanded text
spells it out more simply.

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

Backpatch-through: 9.5

Fix ancient bug in parsing of BRE-mode regular expressions.

brenext(), when parsing a '*' quantifier, forgot to return any "value"
for the token; per the equivalent case in next(), it should return
value 1 to indicate that greedy rather than non-greedy behavior is
wanted.  The result is that the compiled regexp could behave like 'x*?'
rather than the intended 'x*', if we were unlucky enough to have
a zero in v->nextvalue at this point.  That seems to happen with some
reliability if we have '.*' at the beginning of a BRE-mode regexp,
although that depends on the initial contents of a stack-allocated
struct, so it's not guaranteed to fail.

Found by Alexander Lakhin using valgrind testing.  This bug seems
to be aboriginal in Spencer's code, so back-patch all the way.

Discussion: https://postgr.es/m/16814-6c5e3edd2bdf0d50@postgresql.org

Adjust createdb TAP tests to work on recent OpenBSD.

We found last February that the error-case tests added by commit
008cf0409 failed on OpenBSD, because that platform doesn't really
check locale names.  At the time it seemed that that was only an issue
for LC_CTYPE, but testing on a more recent version of OpenBSD shows
that it's now equally lax about LC_COLLATE.

Rather than dropping the LC_COLLATE test too, put back LC_CTYPE
(reverting c4b0edb07), and adjust these tests to accept the different
error message that we get if setlocale() doesn't reject a bogus locale
name.  The point of these tests is not really what the backend does
with the locale name, but to show that createdb quotes funny locale
names safely; so we're not losing test reliability this way.

Back-patch as appropriate.

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

Further second thoughts about idle_session_timeout patch.

On reflection, the order of operations in PostgresMain() is wrong.
These timeouts ought to be shut down before, not after, we do the
post-command-read CHECK_FOR_INTERRUPTS, to guarantee that any
timeout error will be detected there rather than at some ill-defined
later point (possibly after having wasted a lot of work).

This is really an error in the original idle_in_transaction_timeout
patch, so back-patch to 9.6 where that was introduced.

doc: Remove reference to recovery params for divergence lookup in pg_rewind

The documentation of pg_rewind mentioned the use of restore_command and
primary_conninfo as options available at startup to fetch missing WAL
segments that could be used to find the point of divergence for the
rewind.  This is confusing because when finding the point of divergence
the target cluster is offline, so this option is not available.

Issue introduced by 878bd9a, so backpatch down to 9.6.  The
documentation of 13 and HEAD was already right as this sentence has been
changed by a7e8ece when introducing -c/--restore-target-wal.

Reported-by: Amine Tengilimoglu
Discussion: https://postgr.es/m/CADTdw-w_0MP=iQrfizeU4QU5fcZb+w8P_oPeLL+WznWf0kbn3w@mail.gmail.com
Backpatch-through: 9.6

Detect the deadlocks between backends and the startup process.

The deadlocks that the recovery conflict on lock is involved in can
happen between hot-standby backends and the startup process.
If a backend takes an access exclusive lock on the table and which
finally triggers the deadlock, that deadlock can be detected
as expected. On the other hand, previously, if the startup process
took an access exclusive lock and which finally triggered the deadlock,
that deadlock could not be detected and could remain even after
deadlock_timeout passed. This is a bug.

The cause of this bug was that the code for handling the recovery
conflict on lock didn't take care of deadlock case at all. It assumed
that deadlocks involving the startup process and backends were able
to be detected by the deadlock detector invoked within backends.
But this assumption was incorrect. The startup process also should
have invoked the deadlock detector if necessary.

To fix this bug, this commit makes the startup process invoke
the deadlock detector if deadlock_timeout is reached while handling
the recovery conflict on lock. Specifically, in that case, the startup
process requests all the backends holding the conflicting locks to
check themselves for deadlocks.

Back-patch to v9.6. v9.5 has also this bug, but per discussion we decided
not to back-patch the fix to v9.5. Because v9.5 doesn't have some
infrastructure codes (e.g., 37c54863cf) that this bug fix patch depends on.
We can apply those codes for the back-patch, but since the next minor
version release is the final one for v9.5, it's risky to do that. If we
unexpectedly introduce new bug to v9.5 by the back-patch, there is no
chance to fix that. We determined that the back-patch to v9.5 would give
more risk than gain.

Author: Fujii Masao
Reviewed-by: Bertrand Drouvot, Masahiko Sawada, Kyotaro Horiguchi
Discussion: https://postgr.es/m/4041d6b6-cf24-a120-36fa-1294220f8243@oss.nttdata.com

doc: Fix description about default behavior of recovery_target_timeline.

The default value of recovery_target_timeline was changed in v12,
but the description about the default behavior of that was not updated.

Back-patch to v12 where the default behavior of recovery_target_timeline
was changed.

Author: Benoit Lobréau
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAPE8EZ7c3aruEmM24GYkj8y8WmHKD1m9TtPtgCF0nQ3zw4LCkQ@mail.gmail.com

doc:  improve NLS instruction wording

Reported-by: "Tang, Haiying"
Discussion: https://postgr.es/m/bbbccf7a3c2d436e85d45869d612fd6b@G08CNEXMBPEKD05.g08.fujitsu.local

Author: "Tang, Haiying"

Backpatch-through: 9.5

Add an explicit cast to double when using fabs().

Commit bc43b7c2c0 used fabs() directly on an int variable, which
apparently requires an explicit cast on some platforms.

Per buildfarm.

Fix numeric_power() when the exponent is INT_MIN.

In power_var_int(), the computation of the number of significant
digits to use in the computation used log(Abs(exp)), which isn't safe
because Abs(exp) returns INT_MIN when exp is INT_MIN. Use fabs()
instead of Abs(), so that the exponent is cast to a double before the
absolute value is taken.

Back-patch to 9.6, where this was introduced (by 7d9a4737c2).

Discussion: https://postgr.es/m/CAEZATCVd6pMkz=BrZEgBKyqqJrt2xghr=fNc8+Z=5xC6cgWrWA@mail.gmail.com

Fix integer-overflow corner cases in substring() functions.

If the substring start index and length overflow when added together,
substring() misbehaved, either throwing a bogus "negative substring
length" error on a case that should succeed, or failing to complain that
a negative length is negative (and instead returning the whole string,
in most cases).  Unsurprisingly, the text, bytea, and bit variants of
the function all had this issue.  Rearrange the logic to ensure that
negative lengths are always rejected, and add an overflow check to
handle the other case.

Also install similar guards into detoast_attr_slice() (nee
heap_tuple_untoast_attr_slice()), since it's far from clear that
no other code paths leading to that function could pass it values
that would overflow.

Patch by myself and Pavel Stehule, per bug #16804 from Rafi Shamim.

Back-patch to v11.  While these bugs are old, the common/int.h
infrastructure for overflow-detecting arithmetic didn't exist before
commit 4d6ad3125, and it doesn't seem like these misbehaviors are bad
enough to justify developing a standalone fix for the older branches.

Discussion: https://postgr.es/m/16804-f4eeeb6c11ba71d4@postgresql.org

Update copyright for 2021

Backpatch-through: 9.5

Doc: improve explanation of EXTRACT(EPOCH) for timestamp without tz.

Try to be clearer about what computation is actually happening here.

Per bug #16797 from Dana Burd.

Discussion: https://postgr.es/m/16797-f264b0b980b53b8b@postgresql.org

Get heap page max offset with buffer lock held.

On further reflection it seems better to call PageGetMaxOffsetNumber()
after acquiring a buffer lock on the page.  This shouldn't really
matter, but doing it this way is cleaner.

Follow-up to commit 42288174.

Backpatch: 12-, just like commit 42288174

Fix index deletion latestRemovedXid bug.

The logic for determining the latest removed XID for the purposes of
generating recovery conflicts in REDO routines was subtly broken.  It
failed to follow links from HOT chains, and so failed to consider all
relevant heap tuple headers in some cases.

To fix, expand the loop that deals with LP_REDIRECT line pointers to
also deal with HOT chains.  The new version of the loop is loosely based
on a similar loop from heap_prune_chain().

The impact of this bug is probably quite limited, since the horizon code
necessarily deals with heap tuples that are pointed to by LP_DEAD-set
index tuples.  The process of setting LP_DEAD index tuples (e.g. within
the kill_prior_tuple mechanism) is highly correlated with opportunistic
pruning of pointed-to heap tuples.  Plus the question of generating a
recovery conflict usually comes up some time after index tuple LP_DEAD
bits were initially set, unlike heap pruning, where a latestRemovedXid
is generated at the point of the pruning operation (heap pruning has no
deferred "would-be page split" style processing that produces conflicts
lazily).

Only backpatch to Postgres 12, the first version where this logic runs
during original execution (following commit 558a9165e08).  The index
latestRemovedXid mechanism has had the same bug since it first appeared
over 10 years ago (in commit a760893d), but backpatching to all
supported versions now seems like a bad idea on balance.  Running the
new improved code during recovery seems risky, especially given the lack
of complaints from the field.

Author: Peter Geoghegan 
Discussion: https://postgr.es/m/CAH2-Wz=Eib393+HHcERK_9MtgNS7Ew1HY=RDC_g6GL46zM5C6Q@mail.gmail.com
Backpatch: 12-

Doc: spell out comparison behaviors for the date/time types.

The behavior of cross-type comparisons among date/time data types was
not really explained anywhere.  You could probably infer it if you
recognized the applicability of comments elsewhere about datatype
conversions, but it seems worthy of explicit documentation.

Per bug #16797 from Dana Burd.

Discussion: https://postgr.es/m/16797-f264b0b980b53b8b@postgresql.org

Fix up usage of krb_server_keyfile GUC parameter.

secure_open_gssapi() installed the krb_server_keyfile setting as
KRB5_KTNAME unconditionally, so long as it's not empty.  However,
pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already,
leading to a troubling inconsistency: in theory, clients could see
different sets of server principal names depending on whether they
use GSSAPI encryption.  Always using krb_server_keyfile seems like
the right thing, so make both places do that.  Also fix up
secure_open_gssapi()'s lack of a check for setenv() failure ---
it's unlikely, surely, but security-critical actions are no place
to be sloppy.

Also improve the associated documentation.

This patch does nothing about secure_open_gssapi()'s use of setenv(),
and indeed causes pg_GSS_recvauth() to use it too.  That's nominally
against project portability rules, but since this code is only built
with --with-gssapi, I do not feel a need to do something about this
in the back branches.  A fix will be forthcoming for HEAD though.

Back-patch to v12 where GSSAPI encryption was introduced.  The
dubious behavior in pg_GSS_recvauth() goes back further, but it
didn't have anything to be inconsistent with, so let it be.

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

In pg_upgrade cross-version test, handle lack of oldstyle_length().

This suffices for testing v12 -> v13; some other version pairs need more
changes.  Back-patch to v10, which removed the function.

doc: Improve some grammar and sentences

90fbf7c has taken care of that for HEAD.  This includes the portion of
the fixes that applies to the documentation, where needed depending on
the branch.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20201227202604[email protected]
Backpatch-through: 9.5

Improve log messages related to pg_hba.conf not matching a connection.

Include details on whether GSS encryption has been activated;
since we added "hostgssenc" type HBA entries, that's relevant info.

Kyotaro Horiguchi and Tom Lane.  Back-patch to v12 where
GSS encryption was introduced.

Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se