Commit Graph

39563 Commits

Author SHA1 Message Date
Tom Lane ad0867314d Disable debug_discard_caches in test_oat_hooks test.
The test output varies when debug_discard_caches is enabled,
because that causes extra executions of recomputeNamespacePath.
Maybe putting a hook in that was a bad idea, but as a stopgap,
just turn off debug_discard_caches in this test.

Per buildfarm (now that we have debug_discard_caches coverage
again).  Back-patch to v15 where this module was added.

Discussion: https://postgr.es/m/2267406.1668804934@sss.pgh.pa.us
2022-11-19 13:42:53 -05:00
Tom Lane 8e3f104440 Doc: sync src/tutorial/basics.source with SGML documentation.
basics.source is supposed to be pretty closely in step with
the examples in chapter 2 of the tutorial, but I forgot to
update it in commit f05a5e000.  Fix that, and adjust a couple
of other discrepancies that had crept in over time.

(I notice that advanced.source is nowhere near being in sync
with chapter 3, but I lack the ambition to do something
about that right now.)
2022-11-19 13:09:14 -05:00
Andrew Dunstan df4e93bea4 Fix version comparison in Version.pm
Version strings with unequal numbers of parts were being compared
incorrectly. We cure this by treating a missing part in the shorter
version as 0.

per complaint from Jehan-Guillaume de Rorthais, but the fix is mine, not
his.

Discussion: https://postgr.es/m/20220628225325.53d97b8d@karst

Backpatch to release 14 where this code was introduced.
2022-11-18 08:47:31 -05:00
Alvaro Herrera 3d45edcef0
Fix MERGE tuple count with DO NOTHING
Reporting tuples for which nothing is done is useless and goes against
the documented behavior, so don't do it.

Backpatch to 15.

Reported by: Luca Ferrari
Discussion: https://postgr.es/m/CAKoxK+42MmACUh6s8XzASQKizbzrtOGA6G1UjzCP75NcXHsiNw@mail.gmail.com
2022-11-17 18:56:11 +01:00
Noah Misch 41afaa1ed4 Account for IPC::Run::result() Windows behavior change.
This restores compatibility with the not-yet-released successor of
version 20220807.0.  Back-patch to 9.4, which introduced this code.

Reviewed by Andrew Dunstan.

Discussion: https://postgr.es/m/20221117061805.GA4020280@rfd.leadboat.com
2022-11-17 07:35:11 -08:00
Alvaro Herrera cefe182533
Fix outdated comment in ExecDelete
This commend references a struct that disappeared before MERGE was
merged ... and ExecDelete is not called by the committed MERGE anyway.
Revert to the original wording.

Backpatch to 15
2022-11-17 12:52:20 +01:00
Daniel Gustafsson 1eaa48e998 doc: Fix wording of MERGE actions in README
UPDATE was listed twice and DELETE was omitted, replace one UPDATE
with DELETE instead.

Backpatch through v15 where MERGE was added.

Author: Myo Wai Thant <myo.waithant@fujitsu.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/OSAPR01MB43247E46931E9E9CFC4AA0F29A079@OSAPR01MB4324.jpnprd01.prod.outlook.com
Backpatch-through: 15
2022-11-17 10:07:06 +01:00
Amit Kapila e49e191815 Fix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.
During XLOG_HASH_SPLIT_ALLOCATE_PAGE replay, we were checking for a
cleanup lock on the new bucket page after acquiring an exclusive lock on
it and raising a PANIC error on failure. However, it is quite possible
that checkpointer can acquire the pin on the same page before acquiring a
lock on it, and then the replay will lead to an error. So instead, directly
acquire the cleanup lock on the new bucket page during
XLOG_HASH_SPLIT_ALLOCATE_PAGE replay operation.

Reported-by: Andres Freund
Author: Robert Haas
Reviewed-By: Amit Kapila, Andres Freund, Vignesh C
Backpatch-through: 11
Discussion: https://postgr.es/m/20220810022617.fvjkjiauaykwrbse@awork3.anarazel.de
2022-11-14 10:32:47 +05:30
Andrew Dunstan 0086ee356f Use installed postgresql.conf.sample for GUC sanity TAP test
The current code looks for the sample file in the source directory, but
it seems better to test against the installed sample file.

Backpatch to release 15 where the test was introduced.

Discussion: https://postgr.es/m/73eea68e-3b6f-5f63-6024-25ed26b52016@dunslane.net

Reviewed by Tom Lane, Alvaro Herrera, Michael Paquier.
2022-11-13 09:10:24 -05:00
Andrew Dunstan 7a387f513d Make PostgreSQL::Test::Cluster::config_data more flexible
Currently this only allows for one argument, which must be present, and
always returns a single string. With this change the following now all
work:

  $all_config = $node->config_data;
  %config_map = ($node->config_data);
  $incdir = $node->config_data('--include-dir');
  ($incdir, $sharedir) = $node->config_data(
      qw(--include-dir --share-dir));

Backpatch to release 15 where this was introduced.

Discussion: https://postgr.es/m/73eea68e-3b6f-5f63-6024-25ed26b52016@dunslane.net

Reviewed by Tom Lane, Alvaro Herrera, Michael Paquier.
2022-11-13 09:02:05 -05:00
Noah Misch e5f94d42eb If wait_for_catchup fails under has_wal_read_bug, skip balance of test.
Test files should now ignore has_wal_read_bug() so long as
wait_for_catchup() is their only known way of reaching the bug.  That's
at least five files today, a number expected to grow over time.  This
commit removes skip logic from three.  By doing so, systems having the
bug regain the ability to catch other kinds of defects via those three
tests.  The other two, 002_databases.pl and 031_recovery_conflict.pl,
have been unprotected.  Back-patch to v15, where done_testing() first
became our standard.

Discussion: https://postgr.es/m/20221030031639.GA3082137@rfd.leadboat.com
2022-11-12 11:19:56 -08:00
Jeff Davis 7bf713dd2d Fix theoretical torn page hazard.
The original report was concerned with a possible inconsistency
between the heap and the visibility map, which I was unable to
confirm. The concern has been retracted.

However, there did seem to be a torn page hazard when using
checksums. By not setting the heap page LSN during redo, the
protections of minRecoveryPoint were bypassed. Fixed, along with a
misleading comment.

It may have been impossible to hit this problem in practice, because
it would require a page tear between the checksum and the flags, so I
am marking this as a theoretical risk. But, as discussed, it did
violate expectations about the page LSN, so it may have other
consequences.

Backpatch to all supported versions.

Reported-by: Konstantin Knizhnik
Reviewed-by: Konstantin Knizhnik
Discussion: https://postgr.es/m/fed17dac-8cb8-4f5b-d462-1bb4908c029e@garret.ru
Backpatch-through: 11
2022-11-11 12:46:11 -08:00
Tom Lane 9c1a4fc891 Fix alter_table.sql test case to test what it claims to.
The stanza "SET STORAGE may need to add a TOAST table" does not
test what it's supposed to, and hasn't done so since we added
the ability to store constant column default values as metadata.
We need to use a non-constant default to get the expected table
rewrite to actually happen.

Fix that, and add the missing checks that would have exposed the
problem to begin with.

Noted while reviewing a patch that made changes in this test case.
Back-patch to v11 where the problem came in.
2022-11-10 17:24:26 -05:00
Tom Lane 576506303c Re-allow building on Microsoft Visual Studio 2013.
In commit 450ee7012 I supposed that all platforms we now care about have
snprintf(), since that's required by C99.  Turns out that Microsoft did
not get around to adding that until VS2015.  We've dropped support for
VS2013 as of HEAD (cf 6203583b7), but not in the back branches, so add
a hack for this in the back branches only.

There's no easy shortcut to an exact emulation of standard snprintf
in VS2013, but fortunately we don't need one: this code was just fine
with using sprintf before 450ee7012, so we can make it do so again
on that platform (and any others where the problem might crop up).

Per bug #17681 from Daisuke Higuchi.  Back-patch to v12, like the
previous patch.

Discussion: https://postgr.es/m/17681-485ba2ec13e7f392@postgresql.org
2022-11-10 10:23:49 -05:00
Amit Kapila daadb42e92 Fix comments atop ReorderBufferAddInvalidations.
The comments atop seem to indicate that we always accumulate invalidation
messages in a top-level transaction which is neither required nor matches
with the code.

Author: Amit Kapila
Reviewd by: Masahiko Sawada
Backpatch-through: 14, where it was introduced in commit c55040ccd0
Discussion: https://postgr.es/m/CAA4eK1LxGgnUroPz8STb6OfjVU1yaHoSA+T63URwmGCLdMJ0LA@mail.gmail.com
2022-11-10 17:08:27 +05:30
Michael Paquier 5962c8cbe5 Fix comment of SimpleLruInit() in slru.c
sync_handler was not mentioned in the comment block of the function.

Oversight in dee663f.

Author: Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TPUd9BwNY47TtMxaijLHSbyHNdhu=kvbGnvO_bi+oC6_Q@mail.gmail.com
Backpatch-through: 14
2022-11-10 16:33:52 +09:00
Tom Lane 7b6610508d Apply a better fix to mdunlinkfork().
Replace the stopgap fix I made in 0e758ae89 with a cleaner one.

The real problem with 4ab5dae94 is that it contorted this function's
logic substantially, by introducing a third code path that required
different behavior in the function's main loop.  That seems quite
unnecessary on closer inspection: the new IsBinaryUpgrade case can
just share the behavior of the other immediate-unlink cases.  Hence,
revert 4ab5dae94 and most of 0e758ae89 (keeping the latter's
save/restore errno fix), and add IsBinaryUpgrade to the set of
conditions tested to choose immediate unlink.

Also fix some additional places with sloppy handling of errno,
to ensure we have an invariant that we always continue processing
after any non-ENOENT failure of do_truncate.  I doubt that that's
fixing any bug of field importance, so I don't feel it necessary to
back-patch; but we might as well get it right while we're here.

Also improve the comments, which had drifted a bit from what the
code actually does, and neglected to mention some important
considerations.

Back-patch to v15, not because this is fixing any bug but because
it doesn't seem like a good idea for v15's mdunlinkfork logic to be
significantly different from both v14 and v16.

Discussion: https://postgr.es/m/3797575.1667924888@sss.pgh.pa.us
2022-11-09 14:15:38 -05:00
Tom Lane e70cd16f22 Doc: add comments about PreventInTransactionBlock/IsInTransactionBlock.
Add a little to the header comments for these functions to make it
clearer what guarantees about commit behavior are provided to callers.
(See commit f92944137 for context.)

Although this is only a comment change, it's really documentation
aimed at authors of extensions, so it seems appropriate to back-patch.

Yugo Nagata and Tom Lane, per further discussion of bug #17434.

Discussion: https://postgr.es/m/17434-d9f7a064ce2a88a3@postgresql.org
2022-11-09 11:08:52 -05:00
Alvaro Herrera 0bc9872b11
Translation updates
Source-Git-URL: ssh://git@git.postgresql.org/pgtranslation/messages.git
Source-Git-Hash: 0a578288026cfaae6b3d120b3ecf719aaa94dfdc
2022-11-07 19:22:54 +01:00
Tom Lane 5fe0ab4201 Fix failure to remove non-first segments of temporary tables.
Commit 4ab5dae94 broke mdunlinkfork's logic for removing additional
segments of a multi-gigabyte table, because it neglected to advance
"segno" after unlinking the first segment, in the code path where it
chooses to unlink that one immediately.  Then the main remove loop
gets ENOENT at segment zero and figures it's done, so we never remove
whatever additional segments might exist.

The main problem here is with large temporary tables, but WAL replay
of a drop of a large regular table would also fail to remove extra
segments.  The third case where this path is taken is for non-main
forks; but I doubt it matters for those since they probably never
exceed 1GB.

The simplest fix is just to increment segno after that unlink().
(Probably this logic could do with a more thorough rethink, but not
with mere hours to go before 15.1 wraps.)

While here, also fix an incautious assumption that
register_forget_request cannot change errno.  I don't think that
that has any really bad consequences, as we'd end up trying to unlink
the zero'th segment either way, but it greatly complicates reasoning
about what could happen here.  Also make a couple of other cosmetic
fixes.

Per bug #17679 from Balazs Szilfai.  Back-patch into v15, as the
faulty patch was.

Discussion: https://postgr.es/m/17679-1095d04450cf6a6e@postgresql.org
2022-11-07 11:36:45 -05:00
Peter Eisentraut 7134af1149 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: f491e594cbaa7be0f786199e48f44bf0d55c9c8b
2022-11-07 14:04:05 +01:00
Tom Lane 2c6d43650d Fix CREATE DATABASE so we can pg_upgrade DBs with OIDs above 2^31.
Commit aa0105141 repeated one of the oldest mistakes in our book:
thinking that OID is the same as int32.  It isn't of course, and
unsurprisingly the first person who came along with a database
OID above 2 billion broke it.  Repair.

Per bug #17677 from Sergey Pankov.  Back-patch to v15.

Discussion: https://postgr.es/m/17677-a99fa067d7ed71c9@postgresql.org
2022-11-04 10:39:52 -04:00
Etsuro Fujita 8117326444 Correct error message for row-level triggers with transition tables on partitioned tables.
"Triggers on partitioned tables cannot have transition tables." is
incorrect as we allow statement-level triggers on partitioned tables to
have transition tables.

This has been wrong since commit 86f575948; back-patch to v11 where that
commit came in.

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/CAPmGK17gk4vXLzz2iG%2BG4LWRWCoVyam70nZ3OuGm1hMJwDrhcg%40mail.gmail.com
2022-11-04 19:15:01 +09:00
Alvaro Herrera c301e1c0c0
Create FKs properly when attaching table as partition
Commit f56f8f8da6 added some code in CloneFkReferencing that's way too
lax about a Constraint node it manufactures, not initializing enough
struct members -- initially_valid in particular was forgotten.  This
causes some FKs in partitions added by ALTER TABLE ATTACH PARTITION to
be marked as not validated.  Set initially_valid true, which fixes the
bug.

While at it, make the struct initialization more complete.  Very similar
code was added in two other places by the same commit; make them all
follow the same pattern for consistency, though no bugs are apparent
there.

This bug has never been reported: I only happened to notice while
working on commit 614a406b4f.  The test case that was added there with
the improper result is repaired.

Backpatch to 12.

Discussion: https://postgr.es/m/20221005105523.bhuhkdx4olajboof@alvherre.pgsql
2022-11-03 20:40:21 +01:00
Tom Lane f2dc7f9e35 Avoid crash after function syntax error in a replication worker.
If a syntax error occurred in a SQL-language or PL/pgSQL-language
CREATE FUNCTION or DO command executed in a logical replication worker,
we'd suffer a null pointer dereference or assertion failure.  That
seems like a rather contrived case, but nonetheless worth fixing.

The cause is that function_parse_error_transpose assumes it must be
executing within the context of a Portal, but logical/worker.c
doesn't create a Portal since it's not running the standard executor.
We can just back off the hard Assert check and make it fail gracefully
if there's not an ActivePortal.  (I have a feeling that the aggressive
check here was my fault originally, probably because I wasn't sure if
the case would always hold and wanted to find out.  Well, now we know.)

The hazard seems to exist in all branches that have logical replication,
so back-patch to v10.

Maxim Orlov, Anton Melnikov, Masahiko Sawada, Tom Lane

Discussion: https://postgr.es/m/b570c367-ba38-95f3-f62d-5f59b9808226@inbox.ru
Discussion: https://postgr.es/m/adf0452f-8c6b-7def-d35e-ab516c80088e@inbox.ru
2022-11-03 12:01:57 -04:00
Tom Lane 725cd4d2e4 Add casts to simplehash.h to silence C++ warnings.
Casting the result of palloc etc. to the intended type is more per
project style anyway.

(The fact that cpluspluscheck doesn't notice these problems is
because it doesn't expand any macros, which seems like a troubling
shortcoming.  Don't have a good idea about improving that.)

Back-patch to v13, which is as far as the patch applies cleanly;
doesn't seem worth working harder.

David Geier

Discussion: https://postgr.es/m/aa5d88a3-71f4-3455-11cf-82de0372c941@gmail.com
2022-11-03 10:47:31 -04:00
Tom Lane a5737e765d Allow use of __sync_lock_test_and_set for spinlocks on any machine.
If we have no special-case code in s_lock.h for the current platform,
but the compiler has __sync_lock_test_and_set, use that instead of
failing.  It's unlikely that anybody's __sync_lock_test_and_set
would be so awful as to be worse than our semaphore-based fallback,
but if it is, they can (continue to) use --disable-spinlocks.

This allows removal of the RISC-V special case installed by commit
c32fcac56, which generated exactly the same code but only on that
platform.  Usefully, the RISC-V buildfarm animals should now test
at least the int variant of this patch.

I've manually tested both variants on ARM by dint of removing the
ARM-specific stanza.  We don't want to drop that, because it already
has some special knowledge and is likely to grow more over time.
Likewise, this is not meant to preclude installing special cases
for other arches if that proves worthwhile.

Per discussion of a request to install the same code for loongarch64.
Like the previous patch, we might as well back-patch to supported
branches.

Discussion: https://postgr.es/m/761ac43d44b84d679ba803c2bd947cc0@HSMAILSVR04.hs.handsome.com.cn
2022-11-02 17:37:26 -04:00
Tom Lane 414d29a826 Defend against unsupported partition relkind in logical replication worker.
Since partitions can be foreign tables not only plain tables, but
logical replication only supports plain tables, we'd better check the
relkind of a partition after we find it.  (There was some discussion
of checking this when adding a partitioned table to a subscription;
but that would be inadequate since the troublesome partition could be
added later.)  Without this, the situation leads to a segfault or
assertion failure.

In passing, add a separate variable for the target Relation of
a cross-partition UPDATE; reusing partrel seemed mighty confusing
and error-prone.

Shi Yu and Tom Lane, per report from Ilya Gladyshev.  Back-patch
to v13 where logical replication into partitioned tables became
a thing.

Discussion: https://postgr.es/m/6b93e3748ba43298694f376ca8797279d7945e29.camel@gmail.com
2022-11-02 12:29:39 -04:00
Tom Lane 0eede96256 pg_dump: fix failure to dump comments on constraints in some cases.
Thinko in commit 5209c0ba0: I checked the wrong object's
DUMP_COMPONENT_COMMENT bit in two places.

Per bug #17675 from Franz-Josef Färber.

Discussion: https://postgr.es/m/17675-c69c001e06390867@postgresql.org
2022-11-02 11:30:04 -04:00
Etsuro Fujita d5e1748f02 Fix copy-and-pasteo in comment. 2022-11-02 18:15:01 +09:00
David Rowley 23f4427612 Fix outdated comment in tuplesort.h
This was outdated by 77bae396d.

Backpatch-through: 15, where 77bae396d was added
2022-11-02 15:29:49 +13:00
Tom Lane c3d16eb3d5 Update time zone data files to tzdata release 2022f.
DST law changes in Chile, Fiji, Iran, Jordan, Mexico, Palestine,
and Syria.  Historical corrections for Chile, Crimea, Iran, and
Mexico.

Also, the Europe/Kiev zone has been renamed to Europe/Kyiv
(retaining the old name as a link).

The following zones have been merged into nearby, more-populous zones
whose clocks have agreed since 1970: Antarctica/Vostok, Asia/Brunei,
Asia/Kuala_Lumpur, Atlantic/Reykjavik, Europe/Amsterdam,
Europe/Copenhagen, Europe/Luxembourg, Europe/Monaco, Europe/Oslo,
Europe/Stockholm, Indian/Christmas, Indian/Cocos, Indian/Kerguelen,
Indian/Mahe, Indian/Reunion, Pacific/Chuuk, Pacific/Funafuti,
Pacific/Majuro, Pacific/Pohnpei, Pacific/Wake and Pacific/Wallis.
(This indirectly affects zones that were already links to one of
these: Arctic/Longyearbyen, Atlantic/Jan_Mayen, Iceland,
Pacific/Ponape, Pacific/Truk, and Pacific/Yap.)  America/Nipigon,
America/Rainy_River, America/Thunder_Bay, Europe/Uzhgorod, and
Europe/Zaporozhye were also merged into nearby zones after discovering
that their claimed post-1970 differences from those zones seem to have
been errors.

While the IANA crew have been working on merging zones that have no
post-1970 differences for some time, this batch of changes affects
some zones that are significantly more populous than those merged
in the past, notably parts of Europe.  The loss of pre-1970 timezone
history for those zones may be troublesome for applications
expecting consistency of timestamptz display.  As an example, the
stored value '1944-06-01 12:00 UTC' would previously display as
'1944-06-01 13:00:00+01' if the Europe/Stockholm zone is selected,
but now it will read out as '1944-06-01 14:00:00+02'.

There exists a "packrat" option that will build the timezone data
files with this old data preserved, but the problem is that it also
resurrects a bunch of other, far less well-attested data; so much so
that actually more zones' contents change from 2022a with that option
than without it.  I have chosen not to do that here, for that reason
and because it appears that no major OS distributions are using the
"packrat" option, so that doing so would cause Postgres' behavior
to diverge significantly depending on whether it was built with
--with-system-tzdata.  However, for anyone for whom these changes pose
significant problems, there is a solution: build a set of timezone
files with the "packrat" option and use those with Postgres.
2022-11-01 17:08:49 -04:00
Tom Lane 1f1865e908 Fix planner failure with extended statistics on partitioned tables.
Some cases would result in "cache lookup failed for statistics object",
due to trying to fetch inherited statistics when only non-inherited
ones are available or vice versa.

Richard Guo and Justin Pryzby

Discussion: https://postgr.es/m/20221030170520.GM16921@telsasoft.com
2022-11-01 14:34:44 -04:00
Noah Misch 3395cc1dba Under has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.
Per buildfarm member kittiwake.  Back-patch to v15, where this test
first appeared.

Discussion: https://postgr.es/m/20220116210241.GC756210@rfd.leadboat.com
2022-10-29 10:42:19 -07:00
Michael Paquier ca4070f2b4 Fix ordering issue with WAL operations in GIN fast insert path
Contrary to what is documented in src/backend/access/transam/README,
ginHeapTupleFastInsert() had a few ordering issues with the way it does
its WAL operations when inserting items in its fast path.

First, when using a separate list, XLogBeginInsert() was being always
called before START_CRIT_SECTION(), and in this case a second thing was
wrong when merging lists, as an exclusive lock was taken on the tail
page *before* calling XLogBeginInsert().  Finally, when inserting items
into a tail page, the order of XLogBeginInsert() and
START_CRIT_SECTION() was reversed.  This commit addresses all these
issues by moving the calls of XLogBeginInsert() after all the pages
logged are locked and pinned, within a critical section.

This has been applied first only on HEAD as of 56b6625, but as per
discussion with Tom Lane and Álvaro Herrera, a backpatch is preferred to
keep all the branches consistent and to respect the transam's README
where we can.

Author:  Matthias van de Meent, Zhang Mingli
Discussion: https://postgr.es/m/CAEze2WhL8uLMqynnnCu1LAPwxD5RKEo0nHV+eXGg_N6ELU88HQ@mail.gmail.com
Backpatch-through: 10
2022-10-26 09:41:13 +09:00
Alvaro Herrera fb2a83b2b7
Update some comments that should've covered MERGE
Oversight in 7103ebb7aa.  Backpatch to 15.

Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48gnDjZXq3-b56dVpQCNUJ5hD9kdtWN4QFwKCEapspNsA@mail.gmail.com
2022-10-24 12:52:43 +02:00
Peter Eisentraut 4a6de748d3 psql: Fix exit status when query is canceled
Because of a small thinko in 7844c9918a,
psql -c would exit successfully when a query is canceled.  Fix this so
that it exits with a nonzero status, just like for all other errors.
2022-10-22 09:41:38 +02:00
Robert Haas 5c013e620c pg_basebackup: Fix cross-platform tablespace relocation.
Specifically, when pg_basebackup is invoked with -Tx=y, don't error
out if x could plausibly be an absolute path either on Windows or on
non-Windows systems. We don't know whether the remote system is
running the same OS as the local system, so it's not appropriate to
assume that our local rule about absolute pathnames is the same as
the rule on the remote system.

Patch by me, reviewed by Tom Lane, Andrew Dunstan, and
Davinder Singh.

Discussion: http://postgr.es/m/CA+TgmoY+jC3YiskomvYKDPK3FbrmsDU7_8+wMHt02HOdJeRb0g@mail.gmail.com
2022-10-21 08:31:39 -04:00
Amit Kapila 10eaa97501 Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.
Previously in commit 42681dffaf, we added CFI during decoding changes but
missed another similar case that can happen while restoring changes
spilled to disk back into memory in a loop.

Reported-by: Robert Haas
Author: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/CA+TgmoaLObg0QbstbC8ykDwOdD1bDkr4AbPpB=0DPgA2JW0mFg@mail.gmail.com
2022-10-21 12:43:28 +05:30
Amit Kapila 343afa9671 Fix executing invalidation messages generated by subtransactions during decoding.
This problem has been introduced by commit 272248a0c1 where we started
assigning the subtransactions to the top-level transaction when we mark
both the top-level transaction and its subtransactions as containing
catalog changes. After we assign subtransactions to the top-level
transaction, we were not allowed to execute any invalidations associated
with it when we decide to skip the transaction.

The reason to assign the subtransactions to the top-level transaction was
to avoid the assertion failure in AssertTXNLsnOrder() as they have the
same LSN when we sometimes start accumulating transaction changes for
partial transactions after the restart. Now that with commit 64ff0fe4e8,
we skip this assertion check until we reach the LSN at which we start
decoding the contents of the transaction, so, there is no reason for such
an assignment anymore.

The assignment change was introduced in 15 and prior versions but this bug
doesn't exist in branches prior to 14 since we don't add invalidation
messages to subtransactions. We decided to backpatch through 11 for
consistency but not for 10 since its final release is near.

Reported-by: Kuroda Hayato
Author: Masahiko Sawada
Reviewed-by: Amit Kapila
Backpatch-through: 11
Discussion: https://postgr.es/m/TYAPR01MB58660803BCAA7849C8584AA4F57E9%40TYAPR01MB5866.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com
2022-10-21 10:03:35 +05:30
Amit Kapila 64ff0fe4e8 Fix assertion failures while processing NEW_CID record in logical decoding.
When the logical decoding restarts from NEW_CID, since there is no
association between the top transaction and its subtransaction, both are
created as top transactions and have the same LSN. This caused the
assertion failure in AssertTXNLsnOrder().

This patch skips the assertion check until we reach the LSN at which we
start decoding the contents of the transaction, specifically
start_decoding_at LSN in SnapBuild. This is okay because we don't
guarantee to make the association between top transaction and
subtransaction until we try to decode the actual contents of transaction.
The ordering of the records prior to the start_decoding_at LSN should have
been checked before the restart.

The other assertion failure is due to the reason that we forgot to track
that we have considered top-level transaction id in the list of catalog
changing transactions that were committed when one of its subtransactions
is marked as containing catalog change.

Reported-by: Tomas Vondra, Osumi Takamichi
Author: Masahiko Sawada, Kuroda Hayato
Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada
Backpatch-through: 10
Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com
Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com
2022-10-20 09:43:59 +05:30
Thomas Munro af64846e1c Track LLVM 15 changes.
Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque
pointers still exists and we can request that on our context.  We have
until LLVM 16 to move to opaque pointers, a much larger change.

Back-patch to 11, where LLVM support arrived.

Author: Thomas Munro <thomas.munro@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAMHz58Sf_xncdyqsekoVsNeKcruKootLtVH6cYXVhhUR1oKPCg%40mail.gmail.com
2022-10-19 22:18:54 +13:00
Michael Paquier 5d2a47a292 Rework shutdown callback of archiver modules
As currently designed, with a callback registered in a ERROR_CLEANUP
block, the shutdown callback would get called twice when updating
archive_library on SIGHUP, which is something that we want to avoid to
ease the life of extension writers.

Anyway, an ERROR in the archiver process is treated as a FATAL, stopping
it immediately, hence there is no need for a ERROR_CLEANUP block.
Instead of that, the shutdown callback is not called upon
before_shmem_exit(), giving to the modules the opportunity to do any
cleanup actions before the server shuts down its subsystems.

While on it, this commit adds some testing coverage for the shutdown
callback.  Neither shell_archive nor basic_archive have been using it,
and one is added to shell_archive, whose trigger is checked in a TAP
test through a shutdown sequence.

Author: Nathan Bossart, Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20221015221328.GB1821022@nathanxps13
Backpatch-through: 15
2022-10-19 14:07:01 +09:00
Alvaro Herrera 25fb9579bb
Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION
The original hint says to use SET PUBLICATION when really ADD/DROP
PUBLICATION is called for, so this is arguably a bug fix.

Also, a very similar message elsewhere was using an inconsistent
SQLSTATE.

While at it, unwrap some strings.

Backpatch to 15.

Author: Hou zj <houzj.fnst@fujitsu.com>
Discussion: https://postgr.es/m/OS0PR01MB57160AD0E7386547BA978EB394299@OS0PR01MB5716.jpnprd01.prod.outlook.com
2022-10-18 11:46:58 +02:00
Michael Paquier f2f7e509e6 Rename SetSingleFuncCall() to InitMaterializedSRF()
Per discussion, the existing routine name able to initialize a SRF
function with materialize mode is unpopular, so rename it.  Equally, the
flags of this function are renamed, as of:
- SRF_SINGLE_USE_EXPECTED -> MAT_SRF_USE_EXPECTED_DESC
- SRF_SINGLE_BLESS -> MAT_SRF_BLESS
The previous function and flags introduced in 9e98583 are kept around
for compatibility purposes, so as any extension code already compiled
with v15 continues to work as-is.  The declarations introduced here for
compatibility will be removed from HEAD in a follow-up commit.

The new names have been suggested by Andres Freund and Melanie
Plageman.

Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de
Backpatch-through: 15
2022-10-18 10:22:40 +09:00
Tom Lane 4a41a069e7 Reject non-ON-SELECT rules that are named "_RETURN".
DefineQueryRewrite() has long required that ON SELECT rules be named
"_RETURN".  But we overlooked the converse case: we should forbid
non-ON-SELECT rules that are named "_RETURN".  In particular this
prevents using CREATE OR REPLACE RULE to overwrite a view's _RETURN
rule with some other kind of rule, thereby breaking the view.

Per bug #17646 from Kui Liu.  Back-patch to all supported branches.

Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
2022-10-17 12:14:39 -04:00
Tom Lane 2e3326929b Guard against table-AM-less relations in planner.
The executor will dump core if it's asked to execute a seqscan on
a relation having no table AM, such as a view.  While that shouldn't
really happen, it's possible to get there via catalog corruption,
such as a missing ON SELECT rule.  It seems worth installing a defense
against that.  There are multiple plausible places for such a defense,
but I picked the planner's get_relation_info().

Per discussion of bug #17646 from Kui Liu.  Back-patch to v12 where
the tableam APIs were introduced; in older versions you won't get a
SIGSEGV, so it seems less pressing.

Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
2022-10-17 11:35:23 -04:00
Tom Lane d4abb0bc5a Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value.
If the non-recursive term of a SEARCH BREADTH FIRST recursive
query has only constants in its target list, the planner will
fold the starting RowExpr added by rewrite into a simple Const
of type RECORD.  The executor doesn't have any problem with
that --- but EXPLAIN VERBOSE will encounter the Const as the
ultimate source of truth about what the field names of the
SET column are, and it didn't know what to do with that.
Fortunately, we can pull the identifying typmod out of the
Const, in much the same way that record_out would.

For reasons that remain a bit obscure to me, this only fails
with SEARCH BREADTH FIRST, not SEARCH DEPTH FIRST or CYCLE.
But I added regression test cases for both of those options
too, just to make sure we don't break it in future.

Per bug #17644 from Matthijs van der Vleuten.  Back-patch
to v14 where these constructs were added.

Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org
2022-10-16 19:18:08 -04:00
Tom Lane 24c4c26171 Rename parser token REF to REF_P to avoid a symbol conflict.
In the latest version of Apple's macOS SDK, <sys/socket.h>
fails to compile if "REF" is #define'd as something.
Apple may or may not agree that this is a bug, and even if
they do accept the bug report I filed, they probably won't
fix it very quickly.  In the meantime, our back branches will all
fail to compile gram.y.  v15 and HEAD currently escape the problem
thanks to the refactoring done in 98e93a1fc, but that's purely
accidental.  Moreover, since that patch removed a widely-visible
inclusion of <netdb.h>, back-patching it seems too likely to break
third-party code.

Instead, change the token's code name to REF_P, following our usual
convention for naming parser tokens that are likely to have symbol
conflicts.  The effects of that should be localized to the grammar
and immediately surrounding files, so it seems like a safer answer.

Per project policy that we want to keep recently-out-of-support
branches buildable on modern systems, back-patch all the way to 9.2.

Discussion: https://postgr.es/m/1803927.1665938411@sss.pgh.pa.us
2022-10-16 15:27:04 -04:00
Tom Lane bd4b2926ec Use libc's snprintf, not sprintf, for special cases in snprintf.c.
snprintf.c has always fallen back on libc's *printf implementation
when printing pointers (%p) and floats.  When this code originated,
we were still supporting some platforms that lacked native snprintf,
so we used sprintf for that.  That's not actually unsafe in our usage,
but nonetheless builds on macOS are starting to complain about sprintf
being unconditionally deprecated; and I wouldn't be surprised if other
platforms follow suit.  There seems little reason to believe that any
platform supporting C99 wouldn't have standards-compliant snprintf,
so let's just use that instead to suppress such warnings.

Back-patch to v12, which is where we started to require C99.  It's
also where we started to use our snprintf.c everywhere, so this
wouldn't be enough to suppress the warning in older branches anyway
--- that is, in older branches these aren't necessarily all our
usages of libc's sprintf.  It is enough in v12+ because any
deprecation annotation attached to libc's sprintf won't apply to
pg_sprintf.  (Whether all our usages of pg_sprintf are adequately
safe is not a matter I intend to address here, but perhaps it could
do with some review.)

Per report from Andres Freund and local testing.

Discussion: https://postgr.es/m/20221015211955.q4cwbsfkyk3c4ty3@awork3.anarazel.de
2022-10-16 11:47:44 -04:00