aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Fix comment in pg_upgrade.h.Nathan Bossart2024-06-19
| | | | | | | | | Contrary to what the comment for the "check" struct member claims, 'pg_upgrade --check' performs only the checks and does not ask the user for permission to make changes. Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/ZnHk7ci5IuTWVc_c%40nathan
* SQL/JSON: Correctly enforce the default ON EMPTY behaviorAmit Langote2024-06-19
| | | | | | | | | | | | | | Currently, when the ON EMPTY clause is not present, the ON ERROR clause (implicit or explicit) dictates the behavior when jsonpath evaluation in ExecEvalJsonExprPath() results in an empty sequence. That is an oversight in the commit 6185c9737c. This commit fixes things so that a NULL is returned instead in that case which is the default behavior when the ON EMPTY clause is not present. Reported-by: Markus Winand Discussion: https://postgr.es/m/F7DD1442-265C-4220-A603-CB0DEB77E91D%40winand.at
* SQL/JSON: Correct jsonpath variable name matchingAmit Langote2024-06-19
| | | | | | | | | | | | | | | Previously, GetJsonPathVar() allowed a jsonpath expression to reference any prefix of a PASSING variable's name. For example, the following query would incorrectly work: SELECT JSON_QUERY(context_item, jsonpath '$xy' PASSING val AS xyz); The fix ensures that the length of the variable name mentioned in a jsonpath expression matches exactly with the name of the PASSING variable before comparing the strings using strncmp(). Reported-by: Alvaro Herrera (off-list) Discussion: https://postgr.es/m/CA+HiwqFGkLWMvELBH6E4SQ45qUHthgcRH6gCJL20OsYDRtFx_w@mail.gmail.com
* doc PG 17 relnotes: properly wrap SGML textBruce Momjian2024-06-18
| | | | Backpatch-through: master
* doc PG 17 relnotes: add links to documentation sectionsBruce Momjian2024-06-18
| | | | | | Also slightly improve markup instructions. Indentation is still needed. Backpatch-through: master
* Fix possible Assert failure in cost_memoize_rescanDavid Rowley2024-06-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | In cost_memoize_rescan(), when calculating the hit_ratio using the calls and ndistinct estimations, if the value that was set in MemoizePath.calls had not been processed through clamp_row_est(), then it was possible that it was set to some non-integer value which could result in ndistinct being 1 higher than calls due to estimate_num_groups() performing clamp_row_est() on its input_rows. This could result in hit_ratio values slightly below 0.0, which would cause an Assert failure. The value of MemoizePath.calls comes from the final parameter in the create_memoize_path() function, of which we only have one true caller of. That caller passes outer_path->rows. All the core code I looked at always seems to call clamp_row_est() on the Path.rows, so there might have been no issues with any core Paths causing troubles here. The bug report was about a CustomPath with a non-clamped row estimated. The misbehavior as a result of this seems to be mostly limited to the Assert() failing. Aside from that, it seems the Memoize costs would just come out slightly higher than they should have, which is likely fairly harmless. Reported-by: Kohei KaiGai <kaigai@heterodb.com> Discussion: https://postgr.es/m/CAOP8fzZnTU+N64UYJYogb1hN-5hFP+PwTb3m_cnGAD7EsQwrKw@mail.gmail.com Reviewed-by: Richard Guo Backpatch-through: 14, where Memoize was introduced
* Fix incorrect punctuation in error messagePeter Eisentraut2024-06-18
|
* Fix typo in 029_stats_restart.plMichael Paquier2024-06-18
| | | | | Oversight in 16acf7f1aaea, where the test has been introduced. Issue noticed while scanning this area of the tree.
* doc PG 17 relnotes: update to currentBruce Momjian2024-06-17
| | | | Backpatch-through: master
* doc PG 17 relnotes: Fix sslnegotation typoAndres Freund2024-06-17
| | | | I was confused with copy-pasting the parameter name didn't work...
* Fix insertion of SP-GiST REDIRECT tuples during REINDEX CONCURRENTLY.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reconstruction of an SP-GiST index by REINDEX CONCURRENTLY may insert some REDIRECT tuples. This will typically happen in a transaction that lacks an XID, which leads either to assertion failure in spgFormDeadTuple or to insertion of a REDIRECT tuple with zero xid. The latter's not good either, since eventually VACUUM will apply GlobalVisTestIsRemovableXid() to the zero xid, resulting in either an assertion failure or a garbage answer. In practice, since REINDEX CONCURRENTLY locks out index scans till it's done, it doesn't matter whether it inserts REDIRECTs or PLACEHOLDERs; and likewise it doesn't matter how soon VACUUM reduces such a REDIRECT to a PLACEHOLDER. So in non-assert builds there's no observable problem here, other than perhaps a little index bloat. But it's not behaving as intended. To fix, remove the failing Assert in spgFormDeadTuple, acknowledging that we might sometimes insert a zero XID; and guard VACUUM's GlobalVisTestIsRemovableXid() call with a test for valid XID, ensuring that we'll reduce such a REDIRECT the first time VACUUM sees it. (Versions before v14 use TransactionIdPrecedes here, which won't fail on zero xid, so they really have no bug at all in non-assert builds.) Another solution could be to not create REDIRECTs at all during REINDEX CONCURRENTLY, making the relevant code paths treat that case like index build (which likewise knows that no concurrent index scans can be happening). That would allow restoring the Assert in spgFormDeadTuple, but we'd still need the VACUUM change because redirection tuples with zero xid may be out there already. But there doesn't seem to be a nice way for spginsert() to tell that it's being called in REINDEX CONCURRENTLY without some API changes, so we'll leave that as a possible future improvement. In HEAD, also rename the SpGistState.myXid field to redirectXid, which seems less misleading (since it might not in fact be our transaction's XID) and is certainly less uninformatively generic. Per bug #18499 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18499-8a519c280f956480@postgresql.org
* Remove recordExtensionInitPriv[Worker]'s ownerId argument.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | In the wake of the previous commit, we're not doing anything with that argument. Hence, revert the portions of 534287403 that added that argument and taught the callers to pass it. Passing the ownerId requires additional syscache lookups in some code paths, which'd be fine if we were doing anything useful with the info, but it seems inadvisable if we're not. Committed separately since there's some thought that we might want to un-revert this in future, in case it's decided that storing the original owner ID explicitly in pg_init_privs is worth doing. Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
* Improve tracking of role dependencies of pg_init_privs entries.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 534287403 invented SHARED_DEPENDENCY_INITACL entries in pg_shdepend, but installed them only for non-owner roles mentioned in a pg_init_privs entry. This turns out to be the wrong thing, because there is nothing to cue REASSIGN OWNED to go and update pg_init_privs entries when the object's ownership is reassigned. That leads to leaving dangling entries in pg_init_privs, as reported by Hannu Krosing. Instead, install INITACL entries for all roles mentioned in pg_init_privs entries (except pinned roles), and change ALTER OWNER to not touch them, just as it doesn't touch pg_init_privs entries. REASSIGN OWNED will now substitute the new owner OID for the old in pg_init_privs entries. This feels like perhaps not quite the right thing, since pg_init_privs ought to be a historical record of the state of affairs just after CREATE EXTENSION. However, it's hard to see what else to do, if we don't want to disallow dropping the object's original owner. In any case this is better than the previous do-nothing behavior, and we're unlikely to come up with a superior solution in time for v17. While here, tighten up some coding rules about how ACLs in pg_init_privs should never be null or empty. There's not any obvious reason to allow that, and perhaps asserting that it's not so will catch some bugs. (We were previously inconsistent on the point, with some code paths taking care not to store empty ACLs and others not.) This leaves recordExtensionInitPrivWorker not doing anything with its ownerId argument, but we'll deal with that separately. catversion bump forced because of change of expected contents of pg_shdepend when pg_init_privs entries exist. Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
* Teach jsonpath string() to unwrap in lax modeAndrew Dunstan2024-06-17
| | | | | | | | | | | | This was an ommission in commit 66ea94e, and brings it into compliance with both other methods and the standard. Per complaint from David Wheeler. Author: David Wheeler, Jeevan Chalke Reviewed-by: Chapman Flack Discussion: https://postgr.es/m/A64AE04F-4410-42B7-A141-7A7349260F4D@justatheory.com
* pg_createsubscriber: Remove failover replication slots on subscriberPeter Eisentraut2024-06-17
| | | | | | | | | After running pg_createsubscriber, these replication slots have no use on subscriber, so drop them. Author: Euler Taveira <euler.taveira@enterprisedb.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://www.postgresql.org/message-id/776c5cac-5ef5-4001-b1bc-5b698bc0c62a%40app.fastmail.com
* pg_createsubscriber: Remove replication slot check on primaryPeter Eisentraut2024-06-17
| | | | | | | | | | | | | | | It used to check if the replication slot exists and is active on primary. This check might fail on slow hosts because the replication slot might not be active at the time of this check. The current code obtains the replication slot name from the primary_slot_name on standby and assumes the replication slot exists and is active on primary. If it doesn't exist, this tool will log an error and continue. Author: Euler Taveira <euler.taveira@enterprisedb.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://www.postgresql.org/message-id/776c5cac-5ef5-4001-b1bc-5b698bc0c62a%40app.fastmail.com
* pg_createsubscriber: Only --recovery-timeout controls the end of recovery ↵Peter Eisentraut2024-06-17
| | | | | | | | | | | | | | | | | | | | process It used to check if the target server is connected to the primary server (send required WAL) to rapidly react when the process won't succeed. This code is not enough to guarantee that the recovery process will complete. There is a window between the walreceiver shutdown and the pg_is_in_recovery() returns false that can reach NUM_CONN_ATTEMPTS attempts and fails. Instead, rely only on the --recovery-timeout option to give up the process after the specified number of seconds. This should help with buildfarm failures on slow machines. Author: Euler Taveira <euler.taveira@enterprisedb.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://www.postgresql.org/message-id/776c5cac-5ef5-4001-b1bc-5b698bc0c62a%40app.fastmail.com
* Make regress function make_tuple_indirect() able to handle plain attributesMichael Paquier2024-06-17
| | | | | | | | | | | The function has been introduced in 368202501539 to test at a low level the new kinds of external toast datums, and would fail on OOM when dealing with a plain storage attribute. The existing tests of indirect_toast do not test this case, still the error generated was confusing. Author: Alexander Lakhin Discussion: https://postgr.es/m/250a21e5-d677-6b2a-2692-cd4233785e37@gmail.com
* doc: Mention modules/injection_points as example for injection pointsMichael Paquier2024-06-17
| | | | | | | This should have been added in 49cd2b93d7db, that introduced the module. Reported-by: Jian He Discussion: https://postgr.es/m/CACJufxF+Vfj2Oz2kBR5v1bjHeZxvs63cLogm70v9Uto1Rqiieg@mail.gmail.com
* Add Windows file version information to test_json_parser programs.Noah Misch2024-06-16
|
* Remove use of %z in sscanf.Noah Misch2024-06-16
| | | | | As in 9d7ded0f4277f5c0063eca8e871a34e2355a8371, it causes warnings on some MinGW compilers.
* Convert confusing macros in multixact.c to static inline functionsHeikki Linnakangas2024-06-16
| | | | | | | | | | | | | The macros were confused about the argument data types. All the arguments were called 'xid', and some of the macros included casts to TransactionId, even though the arguments were actually either MultiXactIds or MultiXactOffsets. It compiles to the same thing, because TransactionId, MultiXactId and MultiXactOffset are all typedefs of uint32, but it was highly misleading. Author: Maxim Orlov <orlovmg@gmail.com> Discussion: https://www.postgresql.org/message-id/CACG%3DezbLUG-OD1osAW3OchOMxZtdxHh2itYR9Zhh-a13wEBEQw%40mail.gmail.com Discussion: https://www.postgresql.org/message-id/ff143b24-a093-40da-9833-d36b83726bdf%40iki.fi
* doc: fix typo in create role manual.Tatsuo Ishii2024-06-16
| | | | | | | | | There was a small mistake in the create role manual. Author: Satoru Koizumi Reviewed-by: David G. Johnston Discussion: https://postgr.es/m/flat/20240616.112523.1208348667552014162.t-ishii%40sranhm.sra.co.jp Backpatch-through: 16
* Clean out column-level pg_init_privs entries when dropping tables.Tom Lane2024-06-14
| | | | | | | | | | | | | | DeleteInitPrivs did not get the memo about how, when dropping a whole object (with subid == 0), you should drop entries relating to its sub-objects too. This is visible in the test_pg_dump test case if one drops the extension at the end: the entry for GRANT SELECT(col1) ON regress_pg_dump_table TO public; was still present in pg_init_privs afterwards, although it was pointing to a dangling table OID. Noted while fooling with a fix for REASSIGN OWNED for pg_init_privs entries. This bug is aboriginal in the pg_init_privs feature though, and there seems no reason not to back-patch the fix.
* Fix misc_sanity test to accept SHARED_DEPENDENCY_INITACL entries.Tom Lane2024-06-14
| | | | | | | | Oversight in 534287403. We missed this up to now because the core regression tests create no such entries (at least up to this test), so the only way to see the failure is to do "make installcheck" in an installation where some other DB has such entries. I happened to do that just now ...
* Reintroduce dead tuple counter in pg_stat_progress_vacuum.Masahiko Sawada2024-06-14
| | | | | | | | | | | | | | | | | | Commit 667e65aac3 changed both num_dead_tuples and max_dead_tuples columns to dead_tuple_bytes and max_dead_tuple_bytes columns, respectively. But as per discussion, the number of dead tuples collected still provides meaningful insights for users. This commit reintroduces the column for the count of dead tuples, renamed as num_dead_item_ids. It avoids confusion with the number of dead tuples removed by VACUUM, which includes dead heap-only tuples but excludes any pre-existing LP_DEAD items left behind by opportunistic pruning. Bump catalog version. Reviewed-by: Peter Geoghegan, Álvaro Herrera, Andrey Borodin Discussion: https://postgr.es/m/CAD21AoBL5sJE9TRWPyv%2Bw7k5Ee5QAJqDJEDJBUdAaCzGWAdvZw%40mail.gmail.com
* Fix parsing of ignored operators in websearch_to_tsquery().Tom Lane2024-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | The manual says clearly that punctuation in the input of websearch_to_tsquery() is ignored, except for the special cases of dashes and quotes. However, this failed for cases like "(foo bar) or something", or in general an ISOPERATOR character in front of the "or". We'd switch back to WAITOPERAND state, then ignore the operator character while remaining in that state, and then reach the "or" in WAITOPERAND state which (intentionally) makes us treat it as data. The fix is simple enough: if we see an ISOPERATOR character while in WAITOPERATOR state, we have to skip it while staying in that state. (We don't need to worry about other punctuation characters: those will be consumed as though they were words, but then rejected by lexizing.) In v14 and up (since commit eb086056f) we can simplify the code a bit more too, because there is no longer a reason for the WAITOPERAND state to distinguish between quoted and unquoted operands. Per bug #18479 from Manos Emmanouilidis. Back-patch to all supported branches. Discussion: https://postgr.es/m/18479-d9b46e2fc242c33e@postgresql.org
* doc: Fix description WAL summarizer in glossaryMichael Paquier2024-06-14
| | | | | | | | | The WAL summarizer is an auxiliary process. Oversight in 7b1dbf0a8d1d. Author: Masahiro Ikeda Discussion: https://postgr.es/m/d3a5a4278fd8d9e7a47c6aa4db9e9a39@oss.nttdata.com
* doc: Fix description WAL writer in glossaryMichael Paquier2024-06-14
| | | | | | | | | | | The WAL writer is an auxiliary process, but its description in the glossary did not match that. This is inexact since d3014fff4cd4. Author: Masahiro Ikeda Discussion: https://postgr.es/m/d3a5a4278fd8d9e7a47c6aa4db9e9a39@oss.nttdata.com Backpatch-through: 15
* Improve the granularity of PQsocketPoll's timeout parameter.Tom Lane2024-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit f5e4dedfa exposed libpq's internal function PQsocketPoll without a lot of thought about whether that was an API we really wanted to chisel in stone. The main problem with it is the use of time_t to specify the timeout. While we do want an absolute time so that a loop around PQsocketPoll doesn't have problems with timeout slippage, time_t has only 1-second resolution. That's already problematic for libpq's own internal usage --- for example, pqConnectDBComplete has long had a kluge to treat "connect_timeout=1" as 2 seconds so that it doesn't accidentally round to nearly zero. And it's even less likely to be satisfactory for external callers. Hence, let's change this while we still can. The best idea seems to be to use an int64 count of microseconds since the epoch --- basically the same thing as the backend's TimestampTz, but let's use the standard Unix epoch (1970-01-01) since that's more likely for clients to be easy to calculate. Millisecond resolution would be plenty for foreseeable uses, but maybe the day will come that we're glad we used microseconds. Also, since time(2) isn't especially helpful for computing timeouts defined this way, introduce a new function PQgetCurrentTimeUSec to get the current time in this form. Remove the hack in pqConnectDBComplete, so that "connect_timeout=1" now means what you'd expect. We can also remove the "#include <time.h>" that f5e4dedfa added to libpq-fe.h, since there's no longer a need for time_t in that header. It seems better for v17 not to enlarge libpq-fe.h's include footprint from what it's historically been, anyway. I also failed to resist the temptation to do some wordsmithing on PQsocketPoll's documentation. Patch by me, per complaint from Dominique Devienne. Discussion: https://postgr.es/m/913559.1718055575@sss.pgh.pa.us
* When replanning a plpgsql "simple expression", check it's still simple.Tom Lane2024-06-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org
* Clamp result of MultiXactMemberFreezeThresholdHeikki Linnakangas2024-06-13
| | | | | | | | | | | | The purpose of the function is to reduce the effective autovacuum_multixact_freeze_max_age if the multixact members SLRU is approaching wraparound, to make multixid freezing more aggressive. The returned value should therefore never be greater than plain autovacuum_multixact_freeze_max_age. Reviewed-by: Robert Haas Discussion: https://www.postgresql.org/message-id/85fb354c-f89f-4d47-b3a2-3cbd461c90a3@iki.fi Backpatch-through: 12, all supported versions
* Skip some permissions checks on CygwinAndrew Dunstan2024-06-13
| | | | | | These are checks that are already skipped on other Windows systems. Backpatch to all live branches, as appropriate.
* Add postgres_inc to meson check for Python.hAndrew Dunstan2024-06-13
| | | | | | Required for Cygwin. Backpatch to release 16.
* Fix documentation of initdb --show optionPeter Eisentraut2024-06-13
| | | | | | It wasn't in the documentation at all (even though we document all the other debugging-like options). Also, change the --help output to show that it exits after showing, similar to other options.
* Add missing source files to nls.mkPeter Eisentraut2024-06-13
| | | | | | | | | | Files in common/ and fe_utils/ that contain translatable strings need to be listed in the nls.mk files of the programs that use them. (Not great, but that's the way it works for now.) This usually requires some manual analysis which is done about once during each major release beta period. This time, I wrote a hackish script that figures some of this out more automatically, so this update is a bit larger as it also includes some files that were missed in the past.
* libpq: Some message style normalizationPeter Eisentraut2024-06-13
|
* Harmonize pg_bsd_indent parameter names.Peter Geoghegan2024-06-12
| | | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions in pg_bsd_indent. This commit was written with help from clang-tidy, by mechanically applying the same rules as similar clean-up commits. Discussion: https://postgr.es/m/CAH2-WzkaBS8w-vCbG5M5Bx7XikC0WhNLJV_+Z_YAWW9Kef6OBQ@mail.gmail.com
* Harmonize function parameter names for Postgres 17.Peter Geoghegan2024-06-12
| | | | | | | | | | | | | Make sure that function declarations use names that exactly match the corresponding names from function definitions in a few places. These inconsistencies were all introduced during Postgres 17 development. pg_bsd_indent still has a couple of similar inconsistencies, which I (pgeoghegan) have left untouched for now. This commit was written with help from clang-tidy, by mechanically applying the same rules as similar clean-up commits (the earliest such commit was commit 035ce1fe).
* libpq: Add missing gettext markersPeter Eisentraut2024-06-12
| | | | | | | | Follow-up to 87d2801d4b: That commit restored some lost error messages, but they ended up in a place where xgettext wouldn't find them. Rather than elevating ENCRYPTION_NEGOTIATION_FAILED() to a gettext trigger, it's easiest for now to put in some explicit libpq_gettext() calls in the couple of call sites.
* libpq: Remove a gettext markerPeter Eisentraut2024-06-12
| | | | | | | | This one error message is just a workaround for a missing OpenSSL error string. But OpenSSL does not have gettext support, so we don't need to provide it in our workaround either. That way, the user-facing behavior is consistent whether the user has a fixed OpenSSL or not.
* Fix typo in error messagePeter Eisentraut2024-06-12
|
* Fix segmentation fault in test_tidstore.Masahiko Sawada2024-06-12
| | | | | | | | | | | | | | | The do_set_block_offsets() and other functions accessing the tidstore did not check if the tidstore was NULL. This led to a segmentation fault when these functions are called without calling the test_create(). This commit adds NULL checks in relevant functions of test_tidstore to raise an error instead if the tidstore is not initialized. Bug: #18483 Reported-by: Alexander Kozhemyakin Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/18483-30bfff42de238000%40postgresql.org
* Fix infer_arbiter_indexes() to not assume resultRelation is 1.Tom Lane2024-06-11
| | | | | | | | | | | | | | | | | | infer_arbiter_indexes failed to renumber varnos in index expressions or predicates that it got from the catalogs. This escaped detection up to now because the stored varnos in such trees will be 1, and an INSERT's result relation is usually the first rangetable entry, so that that was fine. However, in cases such as inserting through an updatable view, it's not fine, leading to failure to match the expressions to the query with ensuing "there is no unique or exclusion constraint matching the ON CONFLICT specification" errors. Fix by copy-and-paste from get_relation_info(). Per bug #18502 from Michael Wang. Back-patch to all supported versions. Discussion: https://postgr.es/m/18502-545b53f5b81e54e0@postgresql.org
* Fix creation of partition descriptor during concurrent detachAlvaro Herrera2024-06-11
| | | | | | | | | | | | | | | | | | | | When a partition is being detached in concurrent mode, it is possible for find_inheritance_children_extended() to return that partition in the list, and immediately after that receive an invalidation message that sets its relpartbound to NULL just before we read it. (This can happen because table_open() reads invalidation messages.) Currently we raise an error ERROR: missing relpartbound for relation %u about the situation, but that's bogus because the table is no longer a partition, so we shouldn't be complaining about it. A better reaction is to retry the find_inheritance_children_extended call to get a new list, which will no longer have the partition being detached. Noticed while investigating bug #18377. Backpatch to 14, where DETACH CONCURRENTLY appeared. Discussion: https://postgr.es/m/202405201616.y4ht2qe5ihoy@alvherre.pgsql
* Fix an assert in CheckPointReplicationSlots().Amit Kapila2024-06-11
| | | | | | | | | | | | | | | | | | Commit e0b2eed047 assumed that the confirmed_flush LSN can't go backward. However, it is possible that confirmed_flush LSN can go backward temporarily when the client acknowledges a prior value of flush location. This can happen when the client (subscriber in this case) acknowledges an LSN it doesn't have to do anything for (say for DDLs) and thus didn't store persistently. After restart, the client sends the prior value of flush LSN which it had stored persistently and the server updates the confirmed_flush LSN with that value. The fix is to remove the assumption and not allow the prior value of confirmed_flush LSN to be flushed to the disk. Author: Vignesh C Reviewed-by: Amit Kapila, Shlok Kyal Discussion: https://postgr.es/m/CALDaNm3hgow2+oEov5jBk4iYP5eQrUCF1yZtW7+dV3J__p4KLQ@mail.gmail.com
* Doc: Fix ambuiguity in column lists.Amit Kapila2024-06-11
| | | | | | | | | | | The behavior for columns added later to the table for publications with no specified column lists was not clear. Reported-by: Koen De Groote Author: Peter Smith Reviewed-by: Vignesh C, Laurenz Albe Backpatch-through: 15 Discussion: https://postgr.es/m/171621878740.686.11325940592820985181@wrigleys.postgresql.org
* doc: Mention all options equivalent to pg_dump --filter patterns.Dean Rasheed2024-06-10
| | | | | | | | | | In the documentation for pg_dump's new --filter option, added by commit a5cf808be5, each object pattern should match some other existing pg_dump option, but some had been omitted, so add them. Noted by Daniel Gustafsson, reviewed by Ayush Vatsa. Discussion: https://postgr.es/m/CAEZATCWtVUt51B6BjTUQoS4dcNyOBj%2B04ngL7HSH3ehBXTUt%3Dw%40mail.gmail.com
* Fix comment about cross-checking the varnullingrelsRichard Guo2024-06-10
| | | | | | | | | The nullingrels match checks are not limited to debugging builds. Oversight in commit 867be9c07. Author: Richard Guo Reviewed-by: Alvaro Herrera, Tom Lane, Robert Haas Discussion: https://postgr.es/m/CAMbWs4_SDsdYD7DdQw7RXc3jv3axbg+RGZ7aSi9GaqX=F8hNVw@mail.gmail.com
* Fix RBM_ZERO_AND_LOCK.Thomas Munro2024-06-10
| | | | | | | | | | | | | | | | | | | | Commit 210622c6 accidentally zeroed out pages even if they were found in the buffer pool. It should always lock the page, but it should only zero pages that were not already valid. Otherwise, concurrent readers that hold only a pin could see corrupted page contents changing under their feet. While here, rename ZeroAndLockBuffer() to match the RBM_ flag name. Also restore a some useful comments lost by 210622c6's refactoring, and add some new ones to clarify why we need to use the BM_IO_IN_PROGRESS infrastructure despite not doing I/O. Reported-by: Noah Misch <noah@leadboat.com> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> (earlier version) Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier version) Discussion: https://postgr.es/m/20240512171658.7e.nmisch@google.com Discussion: https://postgr.es/m/7ed10231-ce47-03d5-d3f9-4aea0dc7d5a4%40gmail.com