Currently, the error message is produced by a system of complex substitutions
making it quite untranslatable and hard to read. This commit splits this into
4 plain error messages suitable for translation.
Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20240408.152402.1485994009160660141.horikyota.ntt%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
The name collision happens when the name of the new partition is the same as
the name of one of the merging partitions. Currently, ATExecMergePartitions()
first gives the new partition a temporary name and then renames it when old
partitions are deleted. That negatively influences the naming of related
objects like indexes and constrains, which could inherit a temporary name.
This commit changes the implementation in the following way. A merging
partition gets renamed first, then the new partition is created with the
right name immediately. This resolves the issue of the naming of related
objects.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/edfbd846-dcc1-42d1-ac26-715691b687d3%40postgrespro.ru
Author: Dmitry Koval, Alexander Korotkov
Reviewed-by: Robert Haas, Justin Pryzby, Pavel Borisov
I'd not checked that this iteration of the test actually worked
with a bootstrap superuser not named 'postgres'. It didn't,
because the coercion rules for CASE caused us to try to cast
the 'postgres' literal to regrole. Mea culpa.
Per buildfarm (via Alexander Korotkov)
Discussion: https://postgr.es/m/CAPpHfdsV=iTvH6B858hnH1bLgewYH6cdTnO_eOOw9EOa8kehkA@mail.gmail.com
This had been disabled because the test "doesn't delete its user".
It doesn't seem like a great idea for the meson tests to act
differently from the makefile tests, though, and the makefiles
had no such exception (which is how come only copperhead noticed
the problem just fixed in 534287403). In any case, the premise
is false since 936e3fa37, so let's remove the restriction.
Discussion: https://postgr.es/m/2857513.1713733688@sss.pgh.pa.us
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
Repeating loads of inplace-updated fields tends to cause bugs like the
one from the previous commit. While there's no bug to fix in these code
sites, adopt the load-once style. This improves the chance of future
copy/paste finding the safe style.
Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com
vac_update_datfrozenxid() did multiple loads of relfrozenxid and
relminmxid from buffer memory, and it assumed each would get the same
value. Not so if a concurrent vac_update_relstats() did an inplace
update. Commit 2d2e40e3be fixed the same
kind of bug in vac_truncate_clog(). Today's bug could cause the
rel-level field and XIDs in the rel's rows to precede the db-level
field. A cluster having such values should VACUUM affected tables.
Back-patch to v12 (all supported versions).
Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com
If the client supports ALPN but tries to use some other protocol, like
HTTPS, reject the connection in the server. That is surely a confusion
of some sort. Furthermore, the ALPN RFC 7301 says:
> In the event that the server supports no protocols that the client
> advertises, then the server SHALL respond with a fatal
> "no_application_protocol" alert.
This commit makes the server follow that advice.
In the client, specifically check for the OpenSSL error code for the
"no_application_protocol" alert. Otherwise you got a cryptic "SSL
error: SSL error code 167773280" error if you tried to connect to a
non-PostgreSQL server that rejects the connection with
"no_application_protocol". ERR_reason_error_string() returns NULL for
that code, which frankly seems like an OpenSSL bug to me, but we can
easily print a better message ourselves.
Reported-by: Jacob Champion
Discussion: https://www.postgresql.org/message-id/6aedcaa5-60f3-49af-a857-2c76ba55a1f3@iki.fi
These messages were lost in commit 05fd30c0e7. Put them back.
This makes one change in the error message behavior compared to v16,
in the case that the server responds to GSSRequest with an error
instead of rejecting it with 'N'. Previously, libpq would hide the
error that the server sent, assuming that you got the error because
the server is an old pre-v12 version that doesn't understand the
GSSRequest message. A v11 server sends a "FATAL: unsupported frontend
protocol 1234.5680: server supports 2.0 to 3.0" error if you try to
connect to it with GSS. That was a reasonable assumption when the
feature was introduced, but v12 was released a long time ago and I
don't think it's the most probable cause anymore. The attached patch
changes things so that libpq prints the error message that the server
sent in that case, making the "server responds with error to
GSSRequest" case behave the same as the "server responds with error to
SSLRequest" case.
Reported-by: Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/bb3b94da-afc7-438d-8940-cb946e553d9d@eisentraut.org
The ECPG and main 2PC tests have been using rather-generic names for the
prepared transactions they generate. This commit switches the 2PC
transactions to use more complex GIDs, reducing the risk of naming
conflicts.
The main 2PC tests also include scans of pg_prepared_xacts that do not
apply filters on the GID of the prepared transactions, making it
possible to fail the test when any 2PC transaction runs concurrently.
The CI has been able to see such failures with an installcheck
running the ECPG and the main regression test suites in parallel. The
queries on pg_prepared_xacts gain quals to only look after the GIDs
generated locally.
The race is very hard to reproduce, so no backbatch is done for now.
Reported-by: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4-mWCGbbE_bne5=AfqjYGDaUZmjCw2+soLjrdNA0xUDFw@mail.gmail.com
The documentation says that PQsslAttribute(conn, "alpn") returns an
empty string if ALPN is not used, but the code actually returned
NULL. Fix the code to match the documentation.
Reported-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/ZideNHji0G4gxmc3@paquier.xyz
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
ALTER COLUMN TYPE wasn't expecting to find any pg_proc objects
depending on the column whose type is to be altered. That indeed
wasn't possible when this code was written, but it is possible
since we introduced new-style SQL function bodies.
It's about as difficult to fix this case as it is to fix dependent
views, and we've been punting on those for years, so I don't feel
too awful about punting for functions too. (I sure wouldn't risk
back-patching such code.) So just throw a more user-facing error.
Also, adjust some of the existing comments to reflect that these
are all pretty much the same issue.
(This patch also fixes it so we will tolerate finding such a
dependency during ALTER COLUMN SET EXPRESSION; in that, we need
not do anything to the function, so no error is wanted. That
problem is new in HEAD.)
Per bug #18449 from Alexander Lakhin. Back-patch to v14 where
we added new-style SQL functions.
Discussion: https://postgr.es/m/18449-f8248467aaa294d5@postgresql.org
In commit 25cd2d640 I (tgl) opined that "The additions of the months
and microseconds fields could also overflow, of course. However,
I believe we need no additional checks there; the existing range
checks should catch such cases". This is demonstrably wrong however
for the microseconds field, and given that discovery it seems prudent
to be paranoid about the months addition as well.
Report and patch by Joseph Koshakow. As before, back-patch to all
supported branches. (However, the test case doesn't work before
v15 because we didn't allow wider-than-int32 numbers in interval
literals. A variant test could probably be built that fits within
that restriction, but it didn't seem worth the trouble.)
Discussion: https://postgr.es/m/CAAvxfHf77sRHKoEzUw9_cMYSpbpNS2C+J_+8Dq4+0oi8iKopeA@mail.gmail.com
JsonNonTerminal and JsonParserSem were added in commit 3311ea86ed
These names of these two enums are not actually used, so there is no
need for typedefs. Instead use plain enums to declare the constants.
Noticed by Alvaro Herera.
- Bring memory context names in line with other naming
- Fix typos, reported off-list by Alexander Lakhin
- Remove copy-paste errors from comments
- Remove duplicate #undef
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
In e6927270cd I added a 'touch meson.build' to configure.ac, to ensure
conflicts between in-tree configure based builds and meson builds are
automatically detected. Unfortunately I omitted spaces around the condition
restricting this to in-tree builds, leading to touch meson.build to also be
executed in vpath builds. While the only consequence of this buglet is an
unnecessary empty file in build directories, it seems worth backpatching.
Reported-by: Christoph Berg <myon@debian.org>
Discussion: https://postgr.es/m/20240417230002.mb2gv3hyetyn67gk@awork3.anarazel.de
Backpatch: 16-, where the meson based build was added
Also, fix a memory leak when updating from non-embeddable to
embeddable. Both were unreachable without adding C code.
Reported-by: Noah Misch
Author: Noah Misch
Reviewed-by: Masahiko Sawada, John Naylor
Discussion: https://postgr.es/m/20240424210319.4c.nmisch%40google.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
We missed performing table sync if the invalidation happened while the
non-ready tables list was being prepared. This occurs because the sync
state was set to valid at the end of non-ready table list preparation
irrespective of the invalidations processed while the list is being
prepared.
Fix it by changing the boolean variable to a tri-state enum and by setting
table state to valid only if no invalidations have occurred while the list
is being prepared.
Reprted-by: Alexander Lakhin
Diagnosed-by: Alexander Lakhin
Author: Vignesh C
Reviewed-by: Hou Zhijie, Alexander Lakhin, Ajin Cherian, Amit Kapila
Backpatch-through: 15
Discussion: https://postgr.es/m/711a6afe-edb7-1211-cc27-1bef8239eec7@gmail.com
This field is not used directly in the code, but it is important for
query jumbling to be able to make a difference between a named
DEALLOCATE and DEALLOCATE ALL (see bb45156f34). This behavior is
tracked in the regression tests of pg_stat_statements, but the reason
why this field is important can be easily missed, as a recent discussion
has proved, so let's improve its comment to document the reason why it
needs to be around.
Wording has been suggested by Tom Lane
Discussion: https://postgr.es/m/Zih1ATt37YFda8_p@paquier.xyz
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 SSL_R_VERSION_TOO_LOW error reason is supported in LibreSSL since
LibreSSL 3.6.3, shipped in OpenBSD 7.2. SSL_R_VERSION_TOO_HIGH is on
the other hand not supported in any version of LibreSSL. Previously
we only checked for SSL_R_VERSION_TOO_HIGH and then applied both under
that guard since OpenSSL has only ever supported both at the same time.
This breaks the check into one per reason to allow SSL_R_VERSION_TOO_LOW
to work when using LibreSSL.
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
LibreSSL doesn't support the SSL_OP_NO_RENEGOTIATION macro which is
used by OpenSSL, instead it has invented a similar one for client-
side renegotiation: SSL_OP_NO_CLIENT_RENEGOTIATION. This has been
supported since LibreSSL 2.5.1 which by now can be considered well
below the minimum requirement.
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
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
HAVE_LONG_LONG_INT was not added to ecpg_config.h.in by the meson
build system, but rather than add it there, we decided to remove it
from the makefile build system, to make both consistent that way.
There is no documentation or examples that suggest that the presence
of this symbol was publicly advertised, and of course the feature is
required by C99 (but we don't necessarily require C99 for ecpg user
code). ecpg core code and ecpg tests use the symbol
HAVE_LONG_LONG_INT_64 instead, which is still there.
Discussion: https://www.postgresql.org/message-id/flat/bf35d032-02fc-4173-9f4f-840999cc3ef3%40eisentraut.org
. Add missing copytight notices
. improve code coverage
. put work files in a temp directory in the standard location
. improve error checking in C code
. indent perl files with perltidy
. add some comments
per comments from Michael Paquier
Discussion: https://postgr.es/m/ZiC3-cdFys4-6xSk@paquier.xyz
JsonExprState.input_finfo is only assigned to, never read, and
it's really fairly useless since the value can be gotten out of
the adjacent input_fcinfo field. Let's remove it before someone
starts to depend on it.
While here, also remove TidScanState.tss_htup and AggState.combinedproj,
which are referenced nowhere. Those should have been removed by the
commits that caused them to become disused, but were not.
I don't think a catversion bump is necessary here, since plan trees
are never stored on disk.
Matthias van de Meent
Discussion: https://postgr.es/m/CAEze2WjsY4d0TBymLNGK4zpttUcg_YZaTjyWz2VfDUV6YH8wXQ@mail.gmail.com
