Commit Graph

56956 Commits

Author SHA1 Message Date
Daniel Gustafsson 6614d38d87 Close socket in case of errors in setting non-blocking
If configuring the newly created socket non-blocking fails we
error out and return INVALID_SOCKET, but the socket that had
been created wasn't closed. Fix by issuing closesocket in the
errorpath.

Backpatch to all supported branches.

Author: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQApmU5CrKefH85VbNYE2y8H=-qqEJbg6RAPU65+vCe+89A@mail.gmail.com
Backpatch-through: v12
2024-01-17 11:24:11 +01:00
Daniel Gustafsson a1d50b69b5 Decorate WITH with literal markup tags
One instance of "WITH clause" was not using <literal> tags around
WITH, while others were, so add markup to the last one to ensure
consistency.  Backpatch to v15 where MERGE was added.

Reported-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxGJKY9ZCPV2WDM6xFsXq9C8r7r3vU6AkScN+p9k6CEpMw@mail.gmail.com
Backpatch-through: v15
2024-01-16 13:51:15 +01:00
Alvaro Herrera 39aab11085
Don't test already-referenced pointer for nullness
Commit b8ba7344e9 added in PQgetResult a derefence to a pointer
returned by pqPrepareAsyncResult(), before some other code that was
already testing that pointer for nullness.  But since commit
618c16707a (in Postgres 15), pqPrepareAsyncResult() doesn't ever
return NULL (a statically-allocated result is returned if OOM).  So in
branches 15 and up, we can remove the redundant pointer check with no
harm done.

However, in branch 14, pqPrepareAsyncResult() can indeed return NULL if
it runs out of memory.  Fix things there by adding a null pointer check
before dereferencing the pointer.  This should hint Coverity that the
preexisting check is not redundant but necessary.

Backpatch to 14, like b8ba7344e9.

Per Coverity.
2024-01-16 12:27:52 +01:00
Tom Lane 23e0ba59c7 Prevent access to an unpinned buffer in BEFORE ROW UPDATE triggers.
When ExecBRUpdateTriggers switches to a new target tuple as a result
of the EvalPlanQual logic, it must form a new proposed update tuple.
Since commit 86dc90056, that tuple (the result of
ExecGetUpdateNewTuple) has been a virtual tuple that might contain
pointers to by-ref fields of the new target tuple (in "oldslot").
However, immediately after that we materialize oldslot, causing it to
drop its buffer pin, whereupon the by-ref pointers are unsafe to use.
This is a live bug only when the new target tuple is in a different
page than the original target tuple, since we do still hold a pin on
the original one.  (Before 86dc90056, there was no bug because the
EPQ plantree would hold a pin on the new target tuple; but now that's
not assured.)  To fix, forcibly materialize the new tuple before we
materialize oldslot.  This costs nothing since we would have done that
shortly anyway.

The real-world impact of this is probably minimal.  A visible failure
could occur if the new target tuple's buffer were recycled for some
other page in the short interval before we materialize newslot within
the trigger-calling loop; but that's quite unlikely given that we'd
just touched that page.  There's a larger hazard that some other
process could prune and repack that page within the window.  We have
lock on the new target tuple, but that wouldn't prevent it being moved
on the page.

Alexander Lakhin and Tom Lane, per bug #17798 from Alexander Lakhin.
Back-patch to v14 where 86dc90056 came in.

Discussion: https://postgr.es/m/17798-0907404928dcf0dd@postgresql.org
2024-01-14 12:38:41 -05:00
Tom Lane 56dcd71dec Re-pgindent catcache.c after previous commit.
Discussion: https://postgr.es/m/1393953.1698353013@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com
2024-01-13 13:54:11 -05:00
Tom Lane 7e2561e1a2 Cope with catcache entries becoming stale during detoasting.
We've long had a policy that any toasted fields in a catalog tuple
should be pulled in-line before entering the tuple in a catalog cache.
However, that requires access to the catalog's toast table, and we'll
typically do AcceptInvalidationMessages while opening the toast table.
So it's possible that the catalog tuple is outdated by the time we
finish detoasting it.  Since no cache entry exists yet, we can't
mark the entry stale during AcceptInvalidationMessages, and instead
we'll press forward and build an apparently-valid cache entry.  The
upshot is that we have a race condition whereby an out-of-date entry
could be made in a backend's catalog cache, and persist there
indefinitely causing indeterminate misbehavior.

To fix, use the existing systable_recheck_tuple code to recheck
whether the catalog tuple is still up-to-date after we finish
detoasting it.  If not, loop around and restart the process of
searching the catalog and constructing cache entries from the top.
The case is rare enough that this shouldn't create any meaningful
performance penalty, even in the SearchCatCacheList case where
we need to tear down and reconstruct the whole list.

Indeed, the case is so rare that AFAICT it doesn't occur during
our regression tests, and there doesn't seem to be any easy way
to build a test that would exercise it reliably.  To allow
testing of the retry code paths, add logic (in USE_ASSERT_CHECKING
builds only) that randomly pretends that the recheck failed about
one time out of a thousand.  This is enough to ensure that we'll
pass through the retry paths during most regression test runs.

By adding an extra level of looping, this commit creates a need
to reindent most of SearchCatCacheMiss and SearchCatCacheList.
I'll do that separately, to allow putting those changes in
.git-blame-ignore-revs.

Patch by me; thanks to Alexander Lakhin for having built a test
case to prove the bug is real, and to Xiaoran Wang for review.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/1393953.1698353013@sss.pgh.pa.us
Discussion: https://postgr.es/m/CAGjhLkOoBEC9mLsnB42d3CO1vcMx71MLSEuigeABbQ8oRdA6gw@mail.gmail.com
2024-01-13 13:46:27 -05:00
Jeff Davis 41820e640a Fix memory leak in connection string validation.
Introduced in commit c3afe8cf5a.

Discussion: https://postgr.es/m/066a65233d3cb4ea27a9e0778d2f1d0dc764b222.camel@j-davis.com
Reviewed-by: Nathan Bossart, Tom Lane
Backpatch-through: 16
2024-01-12 21:39:35 -08:00
Jeff Davis 4c03ac7e2b Re-validate connection string in libpqrcv_connect().
A superuser may create a subscription with password_required=true, but
which uses a connection string without a password.

Previously, if the owner of such a subscription was changed to a
non-superuser, the non-superuser was able to utilize a password from
another source (like a password file or the PGPASSWORD environment
variable), which should not have been allowed.

This commit adds a step to re-validate the connection string before
connecting.

Reported-by: Jeff Davis
Author: Vignesh C
Reviewed-by: Peter Smith, Robert Haas, Amit Kapila
Discussion: https://www.postgresql.org/message-id/flat/e5892973ae2a80a1a3e0266806640dae3c428100.camel%40j-davis.com
Backpatch-through: 16
2024-01-12 13:42:09 -08:00
Alvaro Herrera 9c00e4c775
Added literal tag for RETURNING
This is an old mistake (92e38182d7); backpatch all the way back.

Author: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/c0aa00b60a16c0ea2a4c5123b013acb9@oss.nttdata.com
2024-01-12 12:44:20 +01:00
Michael Paquier 506c77f9b2 pg_regress: Disable autoruns for cmd.exe on Windows
This is similar to 9886744a36, to prevent the execution of other
programs due to autorun configurations which could influence the
postmaster startup.

This was originally applied on HEAD as of 83c75ac7fb69 without a
backpatch, but the patch has survived CI and buildfarm cycles.  I have
checked that cmd /d exists down to Windows XP, which should make this
change work correctly in the oldest branches still supported.

Discussion: https://postgr.es/m/20230922.161551.320043332510268554.horikyota.ntt@gmail.com
Backpatch-through: 12
2024-01-12 13:59:51 +09:00
Michael Paquier 714bfb7813 pg_ctl: Disable autoruns for cmd.exe on Windows
On Windows, cmd.exe is used to launch the postmaster process to ease its
redirection setup.  However, cmd.exe may execute other programs at
startup due to autorun configurations, which could influence the
postmaster startup.  This patch adds /D flag to the launcher cmd.exe
command line to disable autorun settings written in the registry.

This was originally applied on HEAD as of 9886744a36 without a
backpatch, but the patch has survived CI and buildfarm cycles.  I have
checked that cmd /d exists down to Windows XP, which should make this
change work correctly in the oldest branches still supported.

Reported-by: Hayato Kuroda
Author: Kyotaro Horiguchi
Reviewed-by: Robert Haas, Michael Paquier
Discussion: https://postgr.es/m/20230922.161551.320043332510268554.horikyota.ntt@gmail.com
Backpatch-through: 12
2024-01-12 13:53:07 +09:00
Tom Lane 80bece312c Allow subquery pullup to wrap a PlaceHolderVar in another one.
The code for wrapping subquery output expressions in PlaceHolderVars
believed that if the expression already was a PlaceHolderVar, it was
never necessary to wrap that in another one.  That's wrong if the
expression is underneath an outer join and involves a lateral
reference to outside that scope: failing to add an additional PHV
risks evaluating the expression at the wrong place and hence not
forcing it to null when the outer join should do so.  This is an
oversight in commit 9e7e29c75, which added logic to forcibly wrap
lateral-reference Vars in PlaceHolderVars, but didn't see that the
adjacent case for PlaceHolderVars needed the same treatment.

The test case we have for this doesn't fail before 4be058fe9, but now
that I see the problem I wonder if it is possible to demonstrate
related errors before that.  That's moot though, since all such
branches are out of support.

Per bug #18284 from Holger Reise.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18284-47505a20c23647f8@postgresql.org
2024-01-11 15:28:13 -05:00
Magnus Hagander af36f1993d Fix omission in partitioning limitation documentation
UNIQUE and PRIMARY KEY constraints can be created on ONLY the
partitioned table.  We already had an example demonstrating that,
but forgot to mention it in the documentation of the limits of
partitioning.

Author: Laurenz Albe
Reviewed-By: shihao zhong, Shubham Khanna, Ashutosh Bapat
Backpatch-through: 12
Discussion: https://postgr.es/m/167299368731.659.16130012959616771853@wrigleys.postgresql.org
2024-01-11 14:35:43 +01:00
Tom Lane ba33775fd0 Restore initdb's old behavior of always setting the lc_xxx GUCs.
In commit 3e51b278d I (tgl) caused initdb to leave lc_messages and
other lc_xxx GUCs commented-out in the installed postgresql.conf file
if they were going to be set to 'C'.  This was a hack for cosmetic
purposes, and it was buggy because lc_messages' wired-in default is
not 'C' but '' (empty string).  That led to --no-locale not having
the expected effect, since the postmaster would then obtain
lc_messages from its startup environment.

Let's just revert to the prior behavior of always de-commenting the
lc_xxx entries; the argument for changing that longstanding behavior
was weak in the first place.

Also, fix postgresql.conf.sample's erroneous claim that the default
value of lc_messages is 'C'.  I suspect that was what misled me into
making this mistake in the first place.

Report and patch by Kyotaro Horiguchi.  Back-patch to v16 where
the problem was introduced.

Discussion: https://postgr.es/m/20231122.162700.1995154567625541112.horikyota.ntt@gmail.com
2024-01-10 18:09:29 -05:00
Tom Lane ee95532186 Handle WindowClause.runCondition in tree walker/mutator functions.
Commit 9d9c02ccd, which added the notion of a "run condition" for
window functions, neglected to teach nodeFuncs.c to process the new
field.  Remarkably, that doesn't seem to have had any ill effects
before we invented Var.varnullingrels, but now it can cause visible
failures in join-removal scenarios.

I have no faith that there's not reachable problems in v15 too,
so back-patch the code change to v15 where 9d9c02ccd came in.
The test case seems irrelevant to v15, though.

Per bug #18277 from Zuming Jiang.  Diagnosis and patch by
Richard Guo.

Discussion: https://postgr.es/m/18277-089ead83b329a2fd@postgresql.org
2024-01-10 13:36:34 -05:00
Bruce Momjian 65ee2ed0d0 doc: clarify who owns the initdb-created cluster, by default
Discussion: https://postgr.es/m/ZZoTBSdKeBG_HZ7A@momjian.us

Backpatch-through: 16
2024-01-10 11:20:15 -05:00
Tatsuo Ishii a12774ba0e Doc: fix character_sets view.
The note regarding character encoding form in "The Information Schema"
said that LATIN1 character repertoires only use one encoding form
LATIN1. This is not correct because LATIN1 has another encoding form
ISO-2022-JP-2. To fix this, replace LATIN1 with LATIN2, which is not
supported by ISO-2022-JP-2, thus it can be said that LATIN2 only uses
one encoding form.

Back-patch to supported branches.

Author: Tatsuo Ishii
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/flat/20240102.153925.1147403616414525145.t-ishii%40sranhm.sra.co.jp
2024-01-09 19:46:36 +09:00
Alexander Korotkov 20a566cd43 Fix indentation in ExecParallelHashIncreaseNumBatches()
Backpatch-through: 12
2024-01-08 19:57:09 +02:00
Tom Lane cf6f802bf5 Fix integer-overflow problem in intarray's g_int_decompress().
An array element equal to INT_MAX gave this code indigestion,
causing an infinite loop that surely ended in SIGSEGV.  We fixed
some nearby problems awhile ago (cf 757c5182f) but missed this.

Report and diagnosis by Alexander Lakhin (bug #18273); patch by me

Discussion: https://postgr.es/m/18273-9a832d1da122600c@postgresql.org
2024-01-07 15:19:50 -05:00
Alexander Korotkov 714a987bc1 Fix oversized memory allocation in Parallel Hash Join
During the calculations of the maximum for the number of buckets, take into
account that later we round that to the next power of 2.

Reported-by: Karen Talarico
Bug: #16925
Discussion: https://postgr.es/m/16925-ec96d83529d0d629%40postgresql.org
Author: Thomas Munro, Andrei Lepikhov, Alexander Korotkov
Reviewed-by: Alena Rybakina
Backpatch-through: 12
2024-01-07 09:32:58 +02:00
Michael Paquier 37c5516633 Fix corruption of local buffer state during extend of temp relation
A typo has been introduced by 31966b151e when updating the state of a
local buffer when a temporary relation is extended, for the case of a
block included in the relation range extended, when it is already found
in the hash table holding the local buffers.  In this case, BM_VALID
should be cleared, but the buffer state was changed so as BM_VALID
remained while clearing the other flags.

As reported on the thread, it was possible to corrupt the state of the
local buffers on ENOSPC, but the states would be corrupted on any kind
of ERROR during the relation extend (like partial writes or some other
errno).

Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Richard Guo, Alexander Lakhin, Michael Paquier
Discussion: https://postgr.es/m/18259-6e256429825dd435@postgresql.org
Backpatch-through: 16
2024-01-05 20:10:46 +09:00
David Rowley 6298673f4b Fix use of incorrect TupleTableSlot in DISTINCT aggregates
1349d2790 added code to allow DISTINCT and ORDER BY aggregates to work
more efficiently by using presorted input.  That commit added some code
that made use of the AggState's tmpcontext and adjusted the
ecxt_outertuple and ecxt_innertuple slots before checking if the current
row is distinct from the previously seen row.  That code forgot to set the
TupleTableSlots back to what they were originally, which could result in
errors such as:

ERROR:  attribute 1 of type record has wrong type

This only affects aggregate functions which have multiple arguments when
DISTINCT is used.  For example: string_agg(DISTINCT col, ', ')

Thanks to Tom Lane for identifying the breaking commit.

Bug: #18264
Reported-by: Vojtěch Beneš
Discussion: https://postgr.es/m/18264-e363593d7e9feb7d@postgresql.org
Backpatch-through: 16, where 1349d2790 was added
2024-01-04 20:40:11 +13:00
Bruce Momjian 79a2af3e72 Update copyright for 2024
Reported-by: Michael Paquier

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

Backpatch-through: 12
2024-01-03 20:49:04 -05:00
Tom Lane fb464a1ae3 Avoid masking EOF (no-password-supplied) conditions in auth.c.
CheckPWChallengeAuth() would return STATUS_ERROR if the user does not
exist or has no password assigned, even if the client disconnected
without responding to the password challenge (as libpq often will,
for example).  We should return STATUS_EOF in that case, and the
lower-level functions do, but this code level got it wrong since the
refactoring done in 7ac955b34.  This breaks the intent of not logging
anything for EOF cases (cf. comments in auth_failed()) and might
also confuse users of ClientAuthentication_hook.

Per report from Liu Lang.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/b725238c-539d-cb09-2bff-b5e6cb2c069c@esgyn.cn
2024-01-03 17:40:38 -05:00
Tom Lane 65218693d0 Doc: Python's control flow construct is try/except not try/catch.
Very ancient thinko, dating evidently to 22690719e.
Spotted by gweatherby.

Discussion: https://postgr.es/m/170423637139.1288848.11840082988774620003@wrigleys.postgresql.org
2024-01-03 12:22:00 -05:00
Tom Lane b2c9936a71 In pg_dump, don't dump a stats object unless dumping underlying table.
If the underlying table isn't being dumped, it's useless to dump
an extended statistics object; it'll just cause errors at restore.
We have always applied similar policies to, say, indexes.

(When and if we get cross-table stats objects, it might be profitable
to think a little harder about what to do with them.  But for now
there seems no point in considering a stats object as anything but
an appendage of its table.)

Rian McGuire and Tom Lane, per report from Rian McGuire.
Back-patch to supported branches.

Discussion: https://postgr.es/m/7075d3aa-3f05-44a5-b68f-47dc6a8a0550@buildkite.com
2023-12-29 10:57:11 -05:00
Michael Paquier a78b0b3dcf doc: Mention AttributeRelationId in FDW validator function description
The documentation has been missing one value in the list of catalog OIDs
that can be given to the validator function of a FDW, as of
AttributeRelationId, when changing the attribute options of a foreign
table.

Author: Ian Lawrence Barwick
Discussion: https://postgr.es/m/CAB8KJ=i16t2yJU_Pq2Z+hnNGWFhagp_bJmzxHZu3ZkOjZm-+rQ@mail.gmail.com
Backpatch-through: 12
2023-12-28 20:09:26 +09:00
Tom Lane d02cceb85c Doc: specify aclitem syntax more clearly.
The previous wording here relied solely on an example to explain
aclitem output format.  Add an actual syntax synopsis and
explanation of the elements to make it clearer.

David Johnston and Tom Lane, per gripe from Eugen Konkov.

Discussion: https://postgr.es/m/170326116972.1876499.18357820037829248593@wrigleys.postgresql.org
2023-12-27 13:52:12 -05:00
Tom Lane e87252ceb8 Fix failure to verify PGC_[SU_]BACKEND GUCs in pg_file_settings view.
set_config_option() bails out early if it detects that the option to
be set is PGC_BACKEND or PGC_SU_BACKEND class and we're reading the
config file in a postmaster child; we don't want to apply any new
value in such a case.  That's fine as far as it goes, but it fails
to consider the requirements of the pg_file_settings view: for that,
we need to check validity of the value even though we have no
intention to apply it.  Because we didn't, even very silly values
for affected GUCs would be reported as valid by the view.  There
are only half a dozen such GUCs, which perhaps explains why this
got overlooked for so long.

Fix by continuing when changeVal is false; this parallels the logic
in some other early-exit paths.

Also, the check added by commit 924bcf4f1 to prevent GUC changes in
parallel workers seems a few bricks shy of a load: it's evidently
assuming that ereport(elevel, ...) won't return.  Make sure we
bail out if it does.  The lack of trouble reports suggests that
this is only a latent bug, i.e. parallel workers don't actually
reach here with elevel < ERROR.  (Per the code coverage report,
we never reach here at all in the regression suite.)  But we clearly
don't want to risk proceeding if that does happen.

Per report from Rıdvan Korkmaz.  These are ancient bugs, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/2089235.1703617353@sss.pgh.pa.us
2023-12-26 17:57:48 -05:00
Tom Lane c72049dbc3 Fix mistaken file name in plpython's meson recipe.
Brown-paper-bag bug in commit 58c3151bb.  Per buildfarm.
2023-12-26 17:03:24 -05:00
Tom Lane b0115e7e20 Hide warnings from Python headers when using gcc-compatible compiler.
Like commit 388e80132, use "#pragma GCC system_header" to silence
warnings appearing within the Python headers, since newer Python
versions no longer worry about some restrictions we still use like
-Wdeclaration-after-statement.

This patch improves on 388e80132 by inventing a separate wrapper
header file, allowing the pragma to be tightly scoped to just
the Python headers and not other stuff we have laying about in
plpython.h.  I applied the same technique to plperl for the same
reason: the original patch suppressed warnings for a good deal
of our own code, not only the Perl headers.

Like the previous commit, back-patch to supported branches.

Peter Eisentraut and Tom Lane

Discussion: https://postgr.es/m/ae523163-6d2a-4b81-a875-832e48dec502@eisentraut.org
2023-12-26 16:16:29 -05:00
Amit Kapila 06bbe73541 Doc: Add missing pgoutput options.
We forgot to update the docs while adding new options in pgoutput.

Author: Emre Hasegeli
Reviewed-by: Peter Smith, Amit Kapila
Backpatch-through: 12
Discussion: https://postgr.es/m/CAE2gYzwdwtUbs-tPSV-QBwgTubiyGD2ZGsSnAVsDfAGGLDrGOA%40mail.gmail.com
2023-12-26 11:18:05 +05:30
Tom Lane 11652f919d Set readline-relevant ENV vars in interactive_psql(), not caller.
Commit 664d75753 pulled 010_tab_completion.pl's infrastructure for
invoking an interactive psql session out into a generally-useful test
function, but it didn't move enough stuff.  We need to set up various
environment variables that readline will look at, both to ensure
stability of test results and to prevent test actions from cluttering
the calling user's ~/.psql_history.  Expecting calling scripts to
remember to do that is too failure-prone: the other existing caller
001_password.pl did not do it.  Hence, remove those initialization
steps from 010_tab_completion.pl and put them into interactive_psql().
Since interactive_psql was already making a local ENV hash, this has
no effect on calling scripts.

Discussion: https://postgr.es/m/794610.1703182896@sss.pgh.pa.us
2023-12-23 11:50:33 -05:00
Tom Lane 0977bd64e2 Avoid trying to fetch metapage of an SPGist partitioned index.
This is necessary when spgcanreturn() is invoked on a partitioned
index, and the failure might be reachable in other scenarios as
well.  The rest of what spgGetCache() does is perfectly sensible
for a partitioned index, so we should allow it to go through.

I think the main takeaway from this is that we lack sufficient test
coverage for non-btree partitioned indexes.  Therefore, I added
simple test cases for brin and gin as well as spgist (hash and
gist AMs were covered already in indexing.sql).

Per bug #18256 from Alexander Lakhin.  Although the known test case
only fails since v16 (3c569049b), I've got no faith at all that there
aren't other ways to reach this problem; so back-patch to all
supported branches.

Discussion: https://postgr.es/m/18256-0b0e1b6e4a620f1b@postgresql.org
2023-12-21 12:43:36 -05:00
Dean Rasheed 7f07384dc2 Fix BEFORE ROW trigger handling in cross-partition MERGE update.
Fix a bug during MERGE if a cross-partition update is attempted on a
partitioned table with a BEFORE DELETE ROW trigger that returns NULL,
to prevent the update. This would cause an error to be thrown, or an
assert failure in an assert-enabled build.

This was an oversight in 9321c79c86, which failed to properly
distinguish a DELETE prevented by a trigger from one prevented by a
concurrent update. Fix by having ExecDelete() return the TM_Result
status to ExecCrossPartitionUpdate(), so that it can distinguish the
two cases, and make ExecCrossPartitionUpdate() return the TM_Result
status to ExecUpdateAct(), so that it can return the correct status
from a concurrent update.

In addition, ensure that the command tag is correctly updated by
having ExecMergeMatched() pass canSetTag to ExecUpdateAct(), rather
than passing false, so that it updates the command tag if it does a
cross-partition update, making this code path in ExecMergeMatched()
consistent with ExecUpdate().

Per bug #18238 from Alexander Lakhin. Back-patch to v15, where MERGE
was introduced.

Dean Rasheed, reviewed by Richard Guo and Jian He.

Discussion: https://postgr.es/m/18238-2f2bdc7f720180b9%40postgresql.org
2023-12-21 12:53:48 +00:00
Daniel Gustafsson 5b5db413d2 Fix unchecked return value from strdup
The pg_dump compression was using strdup() instead of pg_strdup()
and failed to check the returned pointer for out-of-memory before
dereferencing it. Fix by using pg_strdup() instead which probably
was the intention here in the original patch.

Backpatch to v16 where pg_dump compression was introduced.

Reviewed-by: Tristan Partin <tristan@neon.tech>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/CC661D60-6F4C-474D-B9CF-E789ACA3CEFC@yesql.se
Backpatch-through: 16
2023-12-20 22:37:28 +01:00
Daniel Gustafsson 94415b04ed doc: Fix syntax in ALTER FOREIGN DATA WRAPPER example
The example for dropping an option was incorrectly quoting the
option key thus making it a value turning the command into an
unqualified ADD operation. The result of dropping became adding
a new key/value pair instead:

 d=# alter foreign data wrapper f options (drop 'b');
 ALTER FOREIGN DATA WRAPPER
 d=# select fdwoptions from pg_foreign_data_wrapper where fdwname='f';
  fdwoptions
 ------------
  {drop=b}
 (1 row)

This has been incorrect for a long time so backpatch to all
supported branches.

Author: Tim <tim.needham2@gmail.com>
Discussion: https://postgr.es/m/170292280173.1876505.5204623074024041738@wrigleys.postgresql.org
2023-12-19 14:13:50 +01:00
Michael Paquier 7f68b14629 pageinspect: Fix failure with hash_bitmap_info() for partitioned indexes
This function reads directly a page from a relation, relying on
index_open() to open the index to read from.  Unfortunately, this would
crash when using partitioned indexes, as these can be opened with
index_open() but they have no physical pages.

Alexander has fixed the module, while I have written the test.

Author: Alexander Lakhin, Michael Paquier
Discussion: https://postgr.es/m/18246-f4d9ff7cb3af77e6@postgresql.org
Backpatch-through: 12
2023-12-19 18:19:15 +09:00
Michael Paquier b4948ed666 pgstattuple: Fix failure with pgstathashindex() for partitioned indexes
As coded, the function relied on index_open() when opening an index
relation, allowing partitioned indexes to be processed by
pgstathashindex().  This was leading to a "could not open file" error
because partitioned indexes have no physical files, or to a crash with
an assertion failure (like on HEAD).

This issue is fixed by applying the same checks as the other stat
functions for indexes, with a lookup at both RELKIND_INDEX and the index
AM expected.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/18246-f4d9ff7cb3af77e6@postgresql.org
Backpatch-through: 12
2023-12-19 15:20:48 +09:00
Tom Lane 8b9d392548 Doc: add a bit to indices.sgml about what is an indexable clause.
We didn't explain this clearly until somewhere deep in the
"Extending SQL" chapter, but really it ought to be mentioned
in the introductory material too.

Discussion: https://postgr.es/m/4097442.1694967650@sss.pgh.pa.us
2023-12-17 16:49:44 -05:00
Tom Lane 152bfc0af8 Fix bugs in manipulation of large objects.
In v16 and up (since commit afbfc0298), large object ownership
checking has been broken because object_ownercheck() didn't take care
of the discrepancy between our object-address representation of large
objects (classId == LargeObjectRelationId) and the catalog where their
ownership info is actually stored (LargeObjectMetadataRelationId).
This resulted in failures such as "unrecognized class ID: 2613"
when trying to update blob properties as a non-superuser.

Poking around for related bugs, I found that AlterObjectOwner_internal
would pass the wrong classId to the PostAlterHook in the no-op code
path where the large object already has the desired owner.  Also,
recordExtObjInitPriv checked for the wrong classId; that bug is only
latent because the stanza is dead code anyway, but as long as we're
carrying it around it should be less wrong.  These bugs are quite old.

In HEAD, we can reduce the scope for future bugs of this ilk by
changing AlterObjectOwner_internal's API to let the translation happen
inside that function, rather than requiring callers to know about it.

A more bulletproof fix, perhaps, would be to start using
LargeObjectMetadataRelationId as the dependency and object-address
classId for blobs.  However that has substantial risk of breaking
third-party code; even within our own code, it'd create hassles
for pg_dump which would have to cope with a version-dependent
representation.  For now, keep the status quo.

Discussion: https://postgr.es/m/2650449.1702497209@sss.pgh.pa.us
2023-12-15 13:55:05 -05:00
Michael Paquier db69101a1d Fix description of I/O timing info for shared buffers in EXPLAIN (BUFFERS)
This fixes an error introduced by efb0ef909f, that changed the
description of this field to "shared/local" while these I/O timings
relate to shared buffers.  This information is available when
track_io_timing is enabled.  Note that HEAD has added new counters for
local buffers in 295c36c0c1, so there is no need to touch it.  The
description is updated to "shared" to be compatible with HEAD.

Per discussion with Nazir Bilal Yavuz and Hubert Depesz Lubaczewski,
whose EXPLAIN analyzer tool was not actually able to parse the previous
term because of the slash character.

Discussion: https://postgr.es/m/ZTCTiUqm_H3iBihl@paquier.xyz
Backpatch-through: 15
2023-12-14 09:59:47 +01:00
Peter Eisentraut 8ae87728c5 Fix typo
Reported-by: Junwang Zhao <zhjwpku@gmail.com>
2023-12-14 09:49:15 +01:00
Michael Paquier 0e2c05af90 Prevent tuples to be marked as dead in subtransactions on standbys
Dead tuples are ignored and are not marked as dead during recovery, as
it can lead to MVCC issues on a standby because its xmin may not match
with the primary.  This information is tracked by a field called
"xactStartedInRecovery" in the transaction state data, switched on when
starting a transaction in recovery.

Unfortunately, this information was not correctly tracked when starting
a subtransaction, because the transaction state used for the
subtransaction did not update "xactStartedInRecovery" based on the state
of its parent.  This would cause index scans done in subtransactions to
return inconsistent data, depending on how the xmin of the primary
and/or the standby evolved.

This is broken since the introduction of hot standby in efc16ea520, so
backpatch all the way down.

Author: Fei Changhong
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/tencent_C4D907A5093C071A029712E73B43C6512706@qq.com
Backpatch-through: 12
2023-12-12 17:05:27 +01:00
Daniel Gustafsson 810c9609f8 Fix typo in comment
Commit 98e675ed7a accidentally mistyped IDENTIFY_SYSTEM as
IDENTIFY_SERVER. Backpatch to all supported branches.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/68138521-5345-8780-4390-1474afdcba1f@gmail.com
2023-12-12 12:16:38 +01:00
Tom Lane ebbd499d4b Be more wary about OpenSSL not setting errno on error.
OpenSSL will sometimes return SSL_ERROR_SYSCALL without having set
errno; this is apparently a reflection of recv(2)'s habit of not
setting errno when reporting EOF.  Ensure that we treat such cases
the same as read EOF.  Previously, we'd frequently report them like
"could not accept SSL connection: Success" which is confusing, or
worse report them with an unrelated errno left over from some
previous syscall.

To fix, ensure that errno is zeroed immediately before the call,
and report its value only when it's not zero afterwards; otherwise
report EOF.

For consistency, I've applied the same coding pattern in libpq's
pqsecure_raw_read().  Bare recv(2) shouldn't really return -1 without
setting errno, but in case it does we might as well cope.

Per report from Andres Freund.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/20231208181451.deqnflwxqoehhxpe@awork3.anarazel.de
2023-12-11 11:51:56 -05:00
Amit Kapila 01cc92fa62 Fix an undetected deadlock due to apply worker.
The apply worker needs to update the state of the subscription tables to
'READY' during the synchronization phase which requires locking the
corresponding subscription. The apply worker also waits for the
subscription tables to reach the 'SYNCDONE' state after holding the locks
on the subscription and the wait is done using WaitLatch. The 'SYNCDONE'
state is changed by tablesync workers again by locking the corresponding
subscription. Both the state updates use AccessShareLock mode to lock the
subscription, so they can't block each other. However, a backend can
simultaneously try to acquire a lock on the same subscription using
AccessExclusiveLock mode to alter the subscription. Now, the backend's
wait on a lock can sneak in between the apply worker and table sync worker
causing deadlock.

In other words, apply_worker waits for tablesync worker which waits for
backend, and backend waits for apply worker. This is not detected by the
deadlock detector because apply worker uses WaitLatch.

The fix is to release existing locks in apply worker before it starts to
wait for tablesync worker to change the state.

Reported-by: Tomas Vondra
Author: Shlok Kyal
Reviewed-by: Amit Kapila, Peter Smith
Backpatch-through: 12
Discussion: https://postgr.es/m/d291bb50-12c4-e8af-2af2-7bb9bb4d8e3e@enterprisedb.com
2023-12-11 08:36:17 +05:30
Thomas Munro 8ca56620ca Fix potential pointer overflow in xlogreader.c.
While checking if a record could fit in the circular WAL decoding
buffer, the coding from commit 3f1ce973 used arithmetic that could
overflow.  64 bit systems were unaffected for various technical reasons,
which probably explains the lack of problem reports.  Likewise for 32
bit systems running known 32 bit kernels.  The systems at risk of
problems appear to be 32 bit processes running on 64 bit kernels, with
unlucky placement in memory.

Per complaint from GCC -fsanitize=undefined -m32, while testing
variations of 039_end_of_wal.pl.

Back-patch to 15.

Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKH0oRPOX7DhiQ_b51sM8HqcPp2J3WA-Oen%3DdXog%2BAGGQ%40mail.gmail.com
2023-12-08 16:10:23 +13:00
Michael Paquier a09aa18eaa Fix path of regress shared library in pg_upgrade test
During a pg_upgrade test using an old dump, all references to the old
regress shared library path (so, dylib or dll) are updated to point to
the library path used by the new build, to ensure a consistent
comparison between the old and new dumps.

The test previously relied on a hardcoded value of "src/test/regress/"
to build the new path value, which would point to an incorrect location
for the meson and vpath builds.  This is replaced by REGRESS_SHLIB, able
to point to the correct location of the regress shared library.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/a628d8ad-a08a-2eab-4ca9-641bc82d3193@gmail.com
Backpatch-through: 15
2023-12-08 10:37:34 +09:00
Michael Paquier 6248a2bb92 Fix compilation on Windows with WAL_DEBUG
This has been broken since b060dbe000 that has reworked the callback
mechanism of XLogReader, most likely unnoticed because any form of
development involving WAL happens on platforms where this compiles fine.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVF14WKQMFwcJ=3okVDhiXpuK5f7YdT+BdYXbbypMHqWA@mail.gmail.com
Backpatch-through: 13
2023-12-06 14:11:41 +09:00