This commit reverts d3d55ce5713 and subsequent fixes 2b26a694554, 93c85db3b5b,
b44a1708abe, b7f315c9d7d, 8a8ed916f73, b5fb6736ed3, 0a93f803f45, e0477837ce4,
a7928a57b9f, 5ef34a8fc38, 30b4955a466, 8c441c08279, 028b15405b4, fe093994db4,
489072ab7a9, and 466979ef031.
We are quite late in the release cycle and new bugs continue to appear. Even
though we have fixes for all known bugs, there is a risk of throwing many
bugs to end users.
The plan for self-join elimination would be to do more review and testing,
then re-commit in the early v18 cycle.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/2422119.1714691974%40sss.pgh.pa.us
Both the initdb --help output and the reference page have a section
for options and a section for less commonly used options. But some
recently added options were sprinkled around inconsistently. Fix that
by making the documentation match the --help output.
We support changing NO INHERIT constraint to INHERIT for constraints in
child relations when adding a constraint to some ancestor relation, and
also during pg_upgrade's schema restore; but other than those special
cases, command ALTER TABLE ADD CONSTRAINT should not be allowed to
change an existing constraint from NO INHERIT to INHERIT, as that would
require to process child relations so that they also acquire an
appropriate constraint, which we may not be in a position to do. (It'd
also be surprising behavior.)
It is conceivable that we want to allow ALTER TABLE SET NOT NULL to make
such a change; but in that case some more code is needed to implement it
correctly, so for now I've made that throw the same error message.
Also, during the prep phase of ALTER TABLE ADD CONSTRAINT, acquire locks
on all descendant tables; otherwise we might operate on child tables on
which no locks are held, particularly in the mode where a primary key
causes not-null constraints to be created on children.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/7d923a66-55f0-3395-cd40-81c142b5448b@gmail.com
Commit 61461a300c1 accidentally misspelled the PGcancelConn struct
using the PQ prefix (which admittedly is a very easy typo to make).
Reported off-list.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
The documentation said that you need to pick a suitable LC_COLLATE
setting in addition to setting the DETERMINISTIC flag. This would
have been correct if the libc provider supported nondeterministic
collations, but since it doesn't, you actually need to set the LOCALE
option.
Reviewed-by: Kashif Zeeshan <kashi.zeeshan@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/a71023c2-0ae0-45ad-9688-cf3b93d0d65b%40eisentraut.org
This commit makes new partitions created by ALTER TABLE ... SPLIT PARTITION
and ALTER TABLE ... MERGE PARTITIONS commands inherit the paret table access
method.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/84ada05b-be5c-473e-6d1c-ebe5dd21b190%40gmail.com
Reviewed-by: Pavel Borisov
The createPartitionTable() function is responsible for creating new partitions
for ALTER TABLE ... MERGE PARTITIONS, and ALTER TABLE ... SPLIT PARTITION
commands. It emulates the behaviour of CREATE TABLE ... (LIKE ...), where
new table persistence should be specified by the user. In the table
partitioning persistent of the partition and its parent must match. So, this
commit makes createPartitionTable() copy the persistence of the parent
partition.
Also, this commit makes createPartitionTable() recheck the persistence after
the new table creation. This is needed because persistence might be affected
by pg_temp in search_path.
This commit also changes the signature of createPartitionTable() making it
take the parent's Relation itself instead of the name of the parent relation,
and return the Relation of new partition. That doesn't lead to
complications, because both callers have the parent table open and need to
open the new partition.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com
Author: Dmitry Koval
Reviewed-by: Alexander Korotkov, Robert Haas, Justin Pryzby, Pavel Borisov
If an ACL recorded in pg_init_privs mentions a non-pinned role,
that reference must also be noted in pg_shdepend so that we know
that the role can't go away without removing the ACL reference.
Otherwise, DROP ROLE could succeed and leave dangling entries
behind, which is what's causing the recent upgrade-check failures
on buildfarm member copperhead.
This has been wrong since pg_init_privs was introduced, but it's
escaped notice because typical pg_init_privs entries would only
mention the bootstrap superuser (pinned) or at worst the owner
of the extension (who can't go away before the extension does).
We lack even a representation of such a role reference for
pg_shdepend. My first thought for a solution was entries listing
pg_init_privs in classid, but that doesn't work because then there's
noplace to put the granted-on object's classid. Rather than adding
a new column to pg_shdepend, let's add a new deptype code
SHARED_DEPENDENCY_INITACL. Much of the associated boilerplate
code can be cribbed from code for SHARED_DEPENDENCY_ACL.
A lot of the bulk of this patch just stems from the new need to pass
the object's owner ID to recordExtensionInitPriv, so that we can
consult it while updating pg_shdepend. While many callers have that
at hand already, a few places now need to fetch the owner ID of an
arbitrary privilege-bearing object. For that, we assume that there
is a catcache on the relevant catalog's OID column, which is an
assumption already made in ExecGrant_common so it seems okay here.
We do need an entirely new routine RemoveRoleFromInitPriv to perform
cleanup of pg_init_privs ACLs during DROP OWNED BY. It's analogous
to RemoveRoleFromObjectACL, but we can't share logic because that
function operates by building a command parsetree and invoking
existing GRANT/REVOKE infrastructure. There is of course no SQL
command that would update pg_init_privs entries when we're not in
process of creating their extension, so we need a routine that can
do the updates directly.
catversion bump because this changes the expected contents of
pg_shdepend. For the same reason, there's no hope of back-patching
this, even though it fixes a longstanding bug. Fortunately, the
case where it's a problem seems to be near nonexistent in the field.
If it weren't for the buildfarm breakage, I'd have been content to
leave this for v18.
Patch by me; thanks to Daniel Gustafsson for review and discussion.
Discussion: https://postgr.es/m/1745535.1712358659@sss.pgh.pa.us
The paragraph in the docs and the comment applied to
sslnegotiaton=direct, but not sslnegotiation=requiredirect. In
'requiredirect' mode, negotiated SSL is never used. Move the paragraph
in the docs under the description of 'direct' mode, and rephrase it.
Also the comment's reference to reusing a plaintext connection was
bogus. Authentication failure in plaintext mode only happens after
sending the startup packet, so the connection cannot be reused.
Reported-by: Jacob Champion
Discussion: https://www.postgresql.org/message-id/CAOYmi+=sj+1uydS0NR4nYzw-LRWp3Q-s5speBug5UCLSPMbvGA@mail.gmail.com
The --tablespace-mapping option was specified with required_argument
rather than no_argument, which is wrong. Since the actual argument
string passed to getopt_long() included "T:", the single-character
form of the option still worked, but the long form did not. Repair.
The call to getopt_long() erroneously included "P", which doesn't
correspond to any supported option. Remove.
The help message used "do not" in one place and "don't" in another.
Standardize on "do not".
The documentation erroneously stated that the tablespace mappings
would be applied relative to the pathnames in the first backup
specified on the command line, rather than the final one. Fix.
Thanks to Tomas Vondra and Daniel Gustafsson for alerting me to
these mistakes.
Discussion: http://postgr.es/m/CA+TgmoYFznwwaZhHSF1Ze7JeyBv-1yOoSrucKMw37WpF=7RP8g@mail.gmail.com
If not all backups have the same checksum status, but the final backup
has checksums enabled, then the output directory may include pages
with invalid checksums. Document this limitation and explain how to
work around it.
In a future release, we may want to teach pg_combinebackup to
recompute page checksums when required, but as feature freeze has come
and gone, it seems a bit too late to do that for this release.
Patch by me, reviewed by Daniel Gustafsson
Discussion: http://postgr.es/m/CA+TgmoZugzOSmgkx97u3pc0M7U8LycWvugqoyWBv6j15a4hE5g@mail.gmail.com
Allow pg_sync_replication_slots() to error out during promotion of standby.
This makes the behavior of the SQL function consistent with the slot sync
worker. We also ensured that pg_sync_replication_slots() cannot be
executed if sync_replication_slots is enabled and the slotsync worker is
already running to perform the synchronization of slots. Previously, it
would have succeeded in cases when the worker is idle and failed when it
is performing sync which could confuse users.
This patch fixes another issue in the slot sync worker where
SignalHandlerForShutdownRequest() needs to be registered *before* setting
SlotSyncCtx->pid, otherwise, the slotsync worker could miss handling
SIGINT sent by the startup process(ShutDownSlotSync) if it is sent before
worker could register SignalHandlerForShutdownRequest(). To be consistent,
all signal handlers' registration is moved to a prior location before we
set the worker's pid.
Ensure that we clean up synced temp slots at the end of
pg_sync_replication_slots() to avoid such slots being left over after
promotion.
Ensure that ShutDownSlotSync() captures SlotSyncCtx->pid under spinlock to
avoid accessing invalid value as it can be reset by concurrent slot sync
exit due to an error.
Author: Shveta Malik
Reviewed-by: Hou Zhijie, Bertrand Drouvot, Amit Kapila, Masahiko Sawada
Discussion: https://postgr.es/m/CAJpy0uBefXUS_TSz=oxmYKHdg-fhxUT0qfjASW3nmqnzVC3p6A@mail.gmail.com
The paragraph describing the JavaScript string literals allowed in
jsonpath expressions unnecessarily mentions JSON by erroneously
listing \v as allowed by JSON and mentioning the \xNN and \u{N...}
backslash escapes as deviations from JSON when in fact both are
accepted by ECMAScript/JavaScript. Fix this by only referring to
JavaScript.
Author: Erik Wienhold <ewie@ewie.name>
Discussion: https://www.postgresql.org/message-id/flat/1EB17DF9-2636-484B-9DD0-3CAB19C4F5C4@justatheory.com
The paragraph on SSL compression is largely describing events which
took place many years ago, so reword with past tense.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/eac70d46-e61c-4d71-a1e1-78e2bfa19485@eisentraut.org
Do not allow ALTER SUBSCRIPTION ... SET (failover = on|off) in a
transaction block as the changed failover option of the slot can't be
rolled back. For the same reason, we refrain from altering the replication
slot's failover property if the subscription is created with a valid
slot_name and create_slot=false.
Reprted-by: Kuroda Hayato
Author: Hou Zhijie
Reviewed-by: Shveta Malik, Bertrand Drouvot, Kuroda Hayato
Discussion: https://postgr.es/m/OS0PR01MB57165542B09DFA4943830BF294082@OS0PR01MB5716.jpnprd01.prod.outlook.com
The documents were clear that queryid should not be assumed to be stable
between major versions but said nothing about minor versions and left
the reader to guess if that was implied by the mention of the
instability of queryid between major versions.
Here we give minor versions an explicit mention to indicate queryid can
generally be assumed stable between minor versions.
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAApHDvpYGE6h0cD9UO-eHySPynPj1L3J%3DHxT%2BA7Ud8_Yo6AuzA%40mail.gmail.com
Backpatch-through: 12
The optimization for inserts into BRIN indexes added by c1ec02be1d79
relies on a cache that needs to be explicitly released after calling
index_insert(). The commit however failed to invoke the cleanup in
validate_index(), which calls index_insert() indirectly through
table_index_validate_scan().
After inspecting index_insert() callers, it seems unique_key_recheck()
is missing the call too.
Fixed by adding the two missing index_insert_cleanup() calls.
The commit does two additional improvements. The aminsertcleanup()
signature is modified to have the index as the first argument, to make
it more like the other AM callbacks. And the aminsertcleanup() callback
is invoked even if the ii_AmCache is NULL, so that it can decide if the
cleanup is necessary.
Author: Alvaro Herrera, Tomas Vondra
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/202401091043.e3nrqiad6gb7@alvherre.pgsql
These operators were removed by 2f70fdb0644c in the v14 cycle but they were
accidentally left in the table of build-in operator classes. Backpatch down
to v14 where the operators where removed.
Author: Aleksander Alekseev <aleksander@timescale.com>
Reported-by: Colin Caine <cmcaine@gmail.com>
Discussion: https://postgr.es/m/CADwQTQbbr2UQ_fpbyc+8ay=RwEYgYk=TZxH3+RHDqAQfoG+EWA@mail.gmail.com
Backpatch-through: v14
This updates the link from pg_createsubscriber to initial data sync
to actually link to the subsection in question as opposed to the
main logical replication section.
Author: Pavel Luzanov <p.luzanov@postgrespro.ru>
Discussion: https://postgr.es/m/a4af555a-ac60-4416-877d-0440d29b8763@postgrespro.ru
This fixes various typos, duplicated words, and tiny bits of whitespace
mainly in code comments but also in docs.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Heikki Linnakangas <hlinnaka@iki.fi>
Author: Alexander Lakhin <exclusion@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
We don't want users to think that pg_combinebackup is trying to check
the validity of individual backups, because it isn't. Adjust the wording
about sanity checks to make it clear that verification of individual
backups is the job of pg_verifybackup, and that the checks performed
by pg_combinebackup are around the relationships between the backups.
Per discussion with David Steele.
Discussion: http://postgr.es/m/e6f930c3-590c-47b9-b094-217bb2a3e22e@pgmasters.net
This addresses some post-commit review comments for commits 6185c973,
de3600452, and 9425c596a0, with the following changes:
* Fix JSON_TABLE() syntax documentation to use the term
"path_expression" for JSON path expressions instead of
"json_path_specification" to be consistent with the other SQL/JSON
functions.
* Fix a typo in the example code in JSON_TABLE() documentation.
* Rewrite some newly added comments in jsonpath.h.
* In JsonPathQuery(), add missing cast to int before printing an enum
value.
Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com
The code associated with EXEC SQL DEFINE was unreadable and full of
bugs, notably:
* It'd attempt to free a non-malloced string if the ecpg program
tries to redefine a macro that was defined on the command line.
* Possible memory stomp if user writes "-D=foo".
* Undef'ing or redefining a macro defined on the command line would
change the state visible to the next file, when multiple files are
specified on the command line. (While possibly that could have been
an intentional choice, the code clearly intends to revert to the
original macro state; it's just failing to consider this interaction.)
* Missing "break" in defining a new macro meant that redefinition
of an existing name would cause an extra entry to be added to the
definition list. While not immediately harmful, a subsequent undef
would result in the prior entry becoming visible again.
* The interactions with input buffering are subtle and were entirely
undocumented.
It's not that surprising that we hadn't noticed these bugs,
because there was no test coverage at all of either the -D
command line switch or multiple input files. This patch adds
such coverage (in a rather hacky way I guess).
In addition to the code bugs, the user documentation was confused
about whether the -D switch defines a C macro or an ecpg one, and
it failed to mention that you can write "-Dsymbol=value".
These problems are old, so back-patch to all supported branches.
Discussion: https://postgr.es/m/998011.1713217712@sss.pgh.pa.us
Previously, we had consecutive, very short chapters called "Generic
WAL" and "Custom WAL Resource Managers," explaining different approaches
to the same problem. Merge them into a single chapter. Explain most
of the differences between the approaches in the chapter's
introductory text, rather than in the individual sections.
Discussion: http://postgr.es/m/46ac50c1-6b2a-404f-a683-b67af6ab56e9@eisentraut.org
The documentation for SET ROLE states that superusers who switch to
a non-superuser role lose their superuser privileges. While this
is true for most commands, there are exceptions such as SET ROLE
and SET SESSION AUTHORIZATION, which continue to use the current
session user and the authenticated user, respectively.
Furthermore, the description of this command already describes its
effect, so it is arguably unnecessary to include this special case.
This commit removes the note about the superuser case and adds a
sentence about the aforementioned exceptions to the description.
Co-authored-by: Yurii Rashkovskii
Reviewed-by: Shubham Khanna, Robert Haas, Michael Paquier
Discussion: https://postgr.es/m/CA%2BRLCQysHtME0znk2KUMJN343ksboSRQSU-hCnOjesX6VK300Q%40mail.gmail.com
This addresses a few problems with commit e5da0fe3c22 ("Catalog domain
not-null constraints").
In CREATE DOMAIN, a NOT NULL constraint looks like
CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL
(Before e5da0fe3c22, the constraint name was accepted but ignored.)
But in ALTER DOMAIN, a NOT NULL constraint looks like
ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE
where VALUE is where for a table constraint the column name would be.
(This works as of e5da0fe3c22. Before e5da0fe3c22, this syntax
resulted in an internal error.)
But for domains, this latter syntax is confusing and needlessly
inconsistent between CREATE and ALTER. So this changes it to just
ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL
(None of these syntaxes are per SQL standard; we are just living with
the bits of inconsistency that have built up over time.)
In passing, this also changes the psql \dD output to not show not-null
constraints in the column "Check", since it's already shown in the
column "Nullable". This has also been off since e5da0fe3c22.
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
In the libpq encryption negotiation tests, don't run the GSSAPI tests
unless PG_TEST_EXTRA='kerberos' is also set. That makes it possible to
still run most of the tests when GSSAPI support is compiled in, but
there's no MIT Kerberos installation.
The tools.ietf.org site has been decommissioned and replaced by a
number of sites serving various purposes. Links to RFCs and BCPs
are now 301 redirected to their new respective IETF sites. Since
this serves no purpose and only adds network overhead, update our
links to the new locations.
Backpatch to all supported versions.
Discussion: https://postgr.es/m/3C1CEA99-FCED-447D-9858-5A579B4C6687@yesql.se
Backpatch-through: v12
