aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Allow the use of indexes other than PK and REPLICA IDENTITY on the subscriber.Amit Kapila2023-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Using REPLICA IDENTITY FULL on the publisher can lead to a full table scan per tuple change on the subscription when REPLICA IDENTITY or PK index is not available. This makes REPLICA IDENTITY FULL impractical to use apart from some small number of use cases. This patch allows using indexes other than PRIMARY KEY or REPLICA IDENTITY on the subscriber during apply of update/delete. The index that can be used must be a btree index, not a partial index, and it must have at least one column reference (i.e. cannot consist of only expressions). We can uplift these restrictions in the future. There is no smart mechanism to pick the index. If there is more than one index that satisfies these requirements, we just pick the first one. We discussed using some of the optimizer's low-level APIs for this but ruled it out as that can be a maintenance burden in the long run. This patch improves the performance in the vast majority of cases and the improvement is proportional to the amount of data in the table. However, there could be some regression in a small number of cases where the indexes have a lot of duplicate and dead rows. It was discussed that those are mostly impractical cases but we can provide a table or subscription level option to disable this feature if required. Author: Onder Kalaci, Amit Kapila Reviewed-by: Peter Smith, Shi yu, Hou Zhijie, Vignesh C, Kuroda Hayato, Amit Kapila Discussion: https://postgr.es/m/CACawEhVLqmAAyPXdHEPv1ssU2c=dqOniiGz7G73HfyS7+nGV4w@mail.gmail.com
* Fix fractional vacuum_cost_delay.Thomas Munro2023-03-15
| | | | | | | | | | | | | | | | | | | | | | | Commit 4753ef37 changed vacuum_delay_point() to use the WaitLatch() API, to fix the problem that vacuum could keep running for a very long time after the postmaster died. Unfortunately, that broke commit caf626b2's support for fractional vacuum_cost_delay, which shipped in PostgreSQL 12. WaitLatch() works in whole milliseconds. For now, revert the change from commit 4753ef37, but add an explicit check for postmaster death. That's an extra system call on systems other than Linux and FreeBSD, but that overhead doesn't matter much considering that we willingly went to sleep and woke up again. (In later work, we might add higher resolution timeouts to the latch API so that we could do this with our standard programming pattern, but that wouldn't be back-patched.) Back-patch to 14, where commit 4753ef37 arrived. Reported-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/CAAKRu_b-q0hXCBUCAATh0Z4Zi6UkiC0k2DFgoD3nC-r3SkR3tg%40mail.gmail.com
* Fix waitpid() emulation on Windows.Thomas Munro2023-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | Our waitpid() emulation didn't prevent a PID from being recycled by the OS before the call to waitpid(). The postmaster could finish up tracking more than one child process with the same PID, and confuse them. Fix, by moving the guts of pgwin32_deadchild_callback() into waitpid(), so that resources are released synchronously. The process and PID continue to exist until we close the process handle, which only happens once we're ready to adjust our book-keeping of running children. This seems to explain a couple of failures on CI. It had never been reported before, despite the code being as old as the Windows port. Perhaps Windows started recycling PIDs more rapidly, or perhaps timing changes due to commit 7389aad6 made it more likely to break. Thanks to Alexander Lakhin for analysis and Andres Freund for tracking down the root cause. Back-patch to all supported branches. Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20230208012852.bvkn2am4h4iqjogq%40awork3.anarazel.de
* Fix corner case bug in numeric to_char() some more.Tom Lane2023-03-14
| | | | | | | | | | | | | | | The band-aid applied in commit f0bedf3e4 turns out to still need some work: it made sure we didn't set Np->last_relevant too small (to the left of the decimal point), but it didn't prevent setting it too large (off the end of the partially-converted string). This could result in fetching data beyond the end of the allocated space, which with very bad luck could cause a SIGSEGV, though I don't see any hazard of interesting memory disclosure. Per bug #17839 from Thiago Nunes. The bug's pretty ancient, so back-patch to all supported versions. Discussion: https://postgr.es/m/17839-aada50db24d7b0da@postgresql.org
* Remove unnecessary code in dependency_is_compatible_expression().Tom Lane2023-03-14
| | | | | | | | | | | | | | | | Scanning the expression for compatible Vars isn't really necessary, because the subsequent match against StatisticExtInfo entries will eliminate expressions containing other Vars just fine. Moreover, this code hadn't stopped to think about what to do with PlaceHolderVars or Aggrefs in the clause; and at least for the PHV case, that demonstrably leads to failures. Rather than work out whether it's reasonable to ignore those, let's just remove the whole stanza. Per report from Richard Guo. Back-patch to v14 where this code was added. Discussion: https://postgr.es/m/CAMbWs48Mmvm-acGevXuwpB=g5JMqVSL6i9z5UaJyLGJqa-XPAA@mail.gmail.com
* Add support for the error functions erf() and erfc().Dean Rasheed2023-03-14
| | | | | | | | | | | | | | | | | | | | | Expose the standard error functions as SQL-callable functions. These are expected to be useful to people working with normal distributions, and we use them here to test the distribution from random_normal(). Since these functions are defined in the POSIX and C99 standards, they should in theory be available on all supported platforms. If that turns out not to be the case, more work will be needed. On all platforms tested so far, using extra_float_digits = -1 in the regression tests is sufficient to allow for variations between implementations. However, past experience has shown that there are almost certainly going to be additional unexpected portability issues, so these tests may well need further adjustments, based on the buildfarm results. Dean Rasheed, reviewed by Nathan Bossart and Thomas Munro. Discussion: https://postgr.es/m/CAEZATCXv5fi7+Vu-POiyai+ucF95+YMcCMafxV+eZuN1B-=MkQ@mail.gmail.com
* Fix JSON error reporting for many cases of erroneous string values.Tom Lane2023-03-13
| | | | | | | | | | | | | | | | | | | | | | | The majority of error exit cases in json_lex_string() failed to set lex->token_terminator, causing problems for the error context reporting code: it would see token_terminator less than token_start and do something more or less nuts. In v14 and up the end result could be as bad as a crash in report_json_context(). Older versions accidentally avoided that fate; but all versions produce error context lines that are far less useful than intended, because they'd stop at the end of the prior token instead of continuing to where the actually-bad input is. To fix, invent some macros that make it less notationally painful to do the right thing. Also add documentation about what the function is actually required to do; and in >= v14, add an assertion in report_json_context about token_terminator being sufficiently far advanced. Per report from Nikolay Shaplov. Back-patch to all supported versions. Discussion: https://postgr.es/m/7332649.x5DLKWyVIX@thinkpad-pgpro
* Fix failure to detect some cases of improperly-nested aggregates.Tom Lane2023-03-13
| | | | | | | | | | | | | | check_agg_arguments_walker() supposed that it needn't descend into the arguments of a lower-level aggregate function, but this is just wrong in the presence of multiple levels of sub-select. The oversight would lead to executor failures on queries that should be rejected. (Prior to v11, they actually were rejected, thanks to a "redundant" execution-time check.) Per bug #17835 from Anban Company. Back-patch to all supported branches. Discussion: https://postgr.es/m/17835-4f29f3098b2d0ba4@postgresql.org
* Add a DEFAULT option to COPY FROMAndrew Dunstan2023-03-13
| | | | | | | | | | | | | | This allows for a string which if an input field matches causes the column's default value to be inserted. The advantage of this is that the default can be inserted in some rows and not others, for which non-default data is available. The file_fdw extension is also modified to take allow use of this option. Israel Barth Rubio Discussion: https://postgr.es/m/CAO_rXXAcqesk6DsvioOZ5zmeEmpUN5ktZf-9=9yu+DTr0Xr8Uw@mail.gmail.com
* Fix MERGE command tag for actions blocked by BEFORE ROW triggers.Dean Rasheed2023-03-13
| | | | | | | | | | | | This ensures that the row count in the command tag for a MERGE is correctly computed in the case where UPDATEs or DELETEs are skipped due to a BEFORE ROW trigger returning NULL (the INSERT case was already handled correctly by ExecMergeNotMatched() calling ExecInsert()). Back-patch to v15, where MERGE was introduced. Discussion: https://postgr.es/m/CAEZATCU8XEmR0JWKDtyb7iZ%3DqCffxS9uyJt0iOZ4TV4RT%2Bow1w%40mail.gmail.com
* Fix concurrent update issues with MERGE.Dean Rasheed2023-03-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If MERGE attempts an UPDATE or DELETE on a table with BEFORE ROW triggers, or a cross-partition UPDATE (with or without triggers), and a concurrent UPDATE or DELETE happens, the merge code would fail. In some cases this would lead to a crash, while in others it would cause the wrong merge action to be executed, or no action at all. The immediate cause of the crash was the trigger code calling ExecGetUpdateNewTuple() as part of the EPQ mechanism, which fails because during a merge ri_projectNew is NULL, since merge has its own per-action projection information, which ExecGetUpdateNewTuple() knows nothing about. Fix by arranging for the trigger code to exit early, returning the TM_Result and TM_FailureData information, if a concurrent modification is detected, allowing the merge code to do the necessary EPQ handling in its own way. Similarly, prevent the cross-partition update code from doing any EPQ processing for a merge, allowing the merge code to work out what it needs to do. This leads to a number of simplifications in nodeModifyTable.c. Most notably, the ModifyTableContext->GetUpdateNewTuple() callback is no longer needed, and mergeGetUpdateNewTuple() can be deleted, since there is no longer any requirement for get-update-new-tuple during a merge. Similarly, ModifyTableContext->cpUpdateRetrySlot is no longer needed. Thus ExecGetUpdateNewTuple() and the retry_slot handling of ExecCrossPartitionUpdate() can be restored to how they were in v14, before the merge code was added, and ExecMergeMatched() no longer needs any special-case handling for cross-partition updates. While at it, tidy up ExecUpdateEpilogue() a bit, making it handle recheckIndexes locally, rather than passing it in as a parameter, ensuring that it is freed properly. This dates back to when it was split off from ExecUpdate() to support merge. Per bug #17809 from Alexander Lakhin, and follow-up investigation of bug #17792, also from Alexander Lakhin. Back-patch to v15, where MERGE was introduced, taking care to preserve backwards-compatibility of the trigger API in v15 for any extensions that might use it. Discussion: https://postgr.es/m/17809-9e6650bef133f0fe%40postgresql.org https://postgr.es/m/17792-0f89452029662c36%40postgresql.org
* Work around implementation restriction in adjust_appendrel_attrs.Tom Lane2023-03-12
| | | | | | | | | | | | | | | | | | | | | | | adjust_appendrel_attrs can't transfer nullingrel labeling to a non-Var translation expression (mainly because it's too late to wrap such an expression in a PlaceHolderVar). I'd supposed in commit 2489d76c4 that that restriction was unreachable because we'd not attempt to push problematic clauses down to an appendrel child relation. I forgot that set_append_rel_size blindly converts all the parent rel's joininfo clauses to child clauses, and that list could well contain clauses from above a nulling outer join. We might eventually have to devise a direct fix for this implementation restriction, but for now it seems enough to filter out troublesome clauses while constructing the child's joininfo list. Such clauses are certainly not useful while constructing paths for the child rel; they'll have to be applied later when we join the completed appendrel to something else. So we don't need them here, and omitting them from the list should save a few cycles while processing the child rel. Per bug #17832 from Marko Tiikkaja. Discussion: https://postgr.es/m/17832-d0a8106cdf1b722e@postgresql.org
* Ensure COPY TO on an RLS-enabled table copies no more than it should.Tom Lane2023-03-10
| | | | | | | | | | | | | | The COPY documentation is quite clear that "COPY relation TO" copies rows from only the named table, not any inheritance children it may have. However, if you enabled row-level security on the table then this stopped being true, because the code forgot to apply the ONLY modifier in the "SELECT ... FROM relation" query that it constructs in order to allow RLS predicates to be attached. Fix that. Report and patch by Antonin Houska (comment adjustments and test case by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/3472.1675251957@antos
* Fix incorrect format placeholdersPeter Eisentraut2023-03-10
|
* Reject combining "epoch" and "infinity" with other datetime fields.Tom Lane2023-03-09
| | | | | | | | | | | Datetime input formerly accepted combinations such as '1995-08-06 infinity', but this seems like a clear error. Reject any combination of regular y/m/d/h/m/s fields with these special tokens. Joseph Koshakow, reviewed by Keisuke Kuroda and myself Discussion: https://postgr.es/m/CAAvxfHdm8wwXwG_FFRaJ1nTHiMWb7YXS2YKCzCt8Q0a2ZoMcHg@mail.gmail.com
* Disallow specifying ICU rules unless locale provider is ICUPeter Eisentraut2023-03-09
| | | | | | Follow-up for 30a53b7929; this was not checked in all cases. Reported-by: Jeff Davis <pgsql@j-davis.com>
* Fix race in SERIALIZABLE READ ONLY.Thomas Munro2023-03-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit bdaabb9b started skipping doomed transactions when building the list of possible conflicts for SERIALIZABLE READ ONLY. That makes sense, because doomed transactions won't commit, but a couple of subtle things broke: 1. If all uncommitted r/w transactions are doomed, a READ ONLY transaction would arbitrarily not benefit from the safe snapshot optimization. It would not be taken immediately, and yet no other transaction would set SXACT_FLAG_RO_SAFE later. 2. In the same circumstances but with DEFERRABLE, GetSafeSnapshot() would correctly exit its wait loop without sleeping and then take the optimization in non-assert builds, but assert builds would fail a sanity check that SXACT_FLAG_RO_SAFE had been set by another transaction. This is similar to the case for PredXact->WritableSxactCount == 0. We should opt out immediately if our possibleUnsafeConflicts list is empty after filtering. The code to maintain the serializable global xmin is moved down below the new opt out site, because otherwise we'd have to reverse its effects before returning. Back-patch to all supported releases. Bug #17368. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org Discussion: https://postgr.es/m/20110707212159.GF76634%40csail.mit.edu
* Allow tailoring of ICU locales with custom rulesPeter Eisentraut2023-03-08
| | | | | | | | | | | | This exposes the ICU facility to add custom collation rules to a standard collation. New options are added to CREATE COLLATION, CREATE DATABASE, createdb, and initdb to set the rules. Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Reviewed-by: Daniel Verite <daniel@manitou-mail.org> Discussion: https://www.postgresql.org/message-id/flat/821c71a4-6ef0-d366-9acf-bb8e367f739f@enterprisedb.com
* Clean up commentsPeter Eisentraut2023-03-08
| | | | | | Reformat some of the comments in MergeAttributes(). A lot of code has been added here over time, and the comments could use a bit of editing to make the code flow read better.
* Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xidsAndres Freund2023-03-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When vacuum_defer_cleanup_age is bigger than the current xid, including the epoch, the subtraction of vacuum_defer_cleanup_age would lead to a wrapped around xid. While that normally is not a problem, the subsequent conversion to a 64bit xid results in a 64bit-xid very far into the future. As that xid is used as a horizon to detect whether rows versions are old enough to be removed, that allows removal of rows that are still visible (i.e. corruption). If vacuum_defer_cleanup_age was never changed from the default, there is no chance of this bug occurring. This bug was introduced in dc7420c2c92. A lesser version of it exists in 12-13, introduced by fb5344c969a, affecting only GiST. The 12-13 version of the issue can, in rare cases, lead to pages in a gist index getting recycled too early, potentially causing index entries to be found multiple times. The fix is fairly simple - don't allow vacuum_defer_cleanup_age to retreat further than FirstNormalTransactionId. Patches to make similar bugs easier to find, by adding asserts to the 64bit xid infrastructure, have been proposed, but are not suitable for backpatching. Currently there are no tests for vacuum_defer_cleanup_age. A patch introducing infrastructure to make writing a test easier has been posted to the list. Reported-by: Michail Nikolaev <michail.nikolaev@gmail.com> Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de Backpatch: 12-, but impact/fix is smaller for 12-13
* Improve readability of code PROCESS_MAIN in vacuum_rel()Michael Paquier2023-03-08
| | | | | | | | | | | | | | 4211fbd has been handling PROCESS_MAIN in vacuum_rel() with an "if/else if" structure to avoid an extra level of indentation, but this has been found as being rather parse to read. This commit updates the code so as we check for PROCESS_MAIN in a single place and then handle its subpaths, FULL or non-FULL vacuums. Some comments are added to make that clearer for the reader. Reported-by: Melanie Plageman Author: Nathan Bossart Reviewed-by: Michael Paquier, Melanie Plageman Discussion: https://postgr.es/m/20230306194009.5cn6sp3wjotd36nu@liskov
* Fix more bugs caused by adding columns to the end of a view.Tom Lane2023-03-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If a view is defined atop another view, and then CREATE OR REPLACE VIEW is used to add columns to the lower view, then when the upper view's referencing RTE is expanded by ApplyRetrieveRule we will have a subquery RTE with fewer eref->colnames than output columns. This confuses various code that assumes those lists are always in sync, as they are in plain parser output. We have seen such problems before (cf commit d5b760ecb), and now I think the time has come to do what was speculated about in that commit: let's make ApplyRetrieveRule synthesize some column names to preserve the invariant that holds in parser output. Otherwise we'll be chasing this class of bugs indefinitely. Moreover, it appears from testing that this actually gives us better results in the test case d5b760ecb added, and likely in other corner cases that we lack coverage for. In HEAD, I replaced d5b760ecb's hack to make expandRTE exit early with an elog(ERROR) call, since the case is now presumably unreachable. But it seems like changing that in back branches would bring more risk than benefit, so there I just updated the comment. Per bug #17811 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17811-d31686b78f0dffc9@postgresql.org
* Add support for unit "B" to pg_size_bytes()Peter Eisentraut2023-03-07
| | | | | | | | This makes it consistent with the units support in GUC. Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/0106914a-9eb5-22be-40d8-652cc88c827d%40enterprisedb.com
* Make get_extension_schema() availableMichael Paquier2023-03-07
| | | | | | | | | | | This routine is able to retrieve the OID of the schema used with an extension (pg_extension.extnamespace), or InvalidOid if this information is not available. plpgsql_check embeds a copy of this code when performing checks on functions, as one out-of-core example. Author: Pavel Stehule Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/CAFj8pRD+9x55hjDoi285jCcjPc8uuY_D+FLn5RpXggdz+4O2sQ@mail.gmail.com
* Fix incorrect comment in pg_get_partkeydef()David Rowley2023-03-07
| | | | | | | | | The comment claimed the output of the function was prefixed by "PARTITION BY". This is incorrect. Author: Japin Li Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/MEYP282MB166923B446FF5FE55B9DACB7B6B69@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Fix some more cases of missed GENERATED-column updates.Tom Lane2023-03-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If UPDATE is forced to retry after an EvalPlanQual check, it neglected to repeat GENERATED-column computations, even though those might well have changed since we're dealing with a different tuple than before. Fixing this is mostly a matter of looping back a bit further when we retry. In v15 and HEAD that's most easily done by altering the API of ExecUpdateAct so that it includes computing GENERATED expressions. Also, if an UPDATE in a partitioned table turns into a cross-partition INSERT operation, we failed to recompute GENERATED columns. That's a bug since 8bf6ec3ba allowed partitions to have different generation expressions; although it seems to have no ill effects before that. Fixing this is messier because we can now have situations where the same query needs both the UPDATE-aligned set of GENERATED columns and the INSERT-aligned set, and it's unclear which set will be generated first (else we could hack things by forcing the INSERT-aligned set to be generated, which is indeed how fe9e658f4 made it work for MERGE). The best fix seems to be to build and store separate sets of expressions for the INSERT and UPDATE cases. That would create ABI issues in the back branches, but so far it seems we can leave this alone in the back branches. Per bug #17823 from Hisahiro Kauchi. The first part of this affects all branches back to v12 where GENERATED columns were added. Discussion: https://postgr.es/m/17823-b64909cf7d63de84@postgresql.org
* Fill EState.es_rteperminfos more systematically.Tom Lane2023-03-06
| | | | | | | | | | | | | | | | | | While testing a fix for bug #17823, I discovered that EvalPlanQualStart failed to copy es_rteperminfos from the parent EState, resulting in failure if anything in EPQ execution wanted to consult that information. This led me to conclude that commit a61b1f748 had been too haphazard about where to fill es_rteperminfos, and that we need to be sure that that happens exactly where es_range_table gets filled. So I changed the signature of ExecInitRangeTable to help ensure that this new requirement doesn't get missed. (Indeed, pgoutput.c was also failing to fill it. Maybe we don't ever need it there, but I wouldn't bet on that.) No test case yet; one will arrive with the fix for #17823. But that needs to be back-patched, while this fix is HEAD-only. Discussion: https://postgr.es/m/17823-b64909cf7d63de84@postgresql.org
* Reword overly-optimistic comment about backup checksum verification.Robert Haas2023-03-06
| | | | | | | | | | The comment implies that a single retry is sufficient to avoid spurious checksum failures, but in fact no number of retries is sufficient for that purpose. Update the comment accordingly. Patch by me, reviewed by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com
* Remove an old comment that doesn't seem especially useful.Robert Haas2023-03-06
| | | | | | | | | | | | The functions that follow are concerned with various things, of which the tar format is only one, so this comment doesn't really seem helpful. The file isn't really divided into sections in the way that this comment seems to contemplate -- or at least, not any more. Patch by me, reviewed by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com
* In basebackup.c, perform end-of-file test after checksum validation.Robert Haas2023-03-06
| | | | | | | | | | | | | | | | | | | | We read blocks of data from files that we're backing up in chunks, some multiple of BLCKSZ for each read. If checksum verification fails, we then try rereading just the one block for which validation failed. If that block happened to be the first block of the chunk, and if the file was concurrently truncated to remove that block, then we'd reach a call to bbsink_archive_contents() with a buffer length of 0. That causes an assertion failure. As far as I can see, there are no particularly bad consequences if this happens in a non-assert build, and it's pretty unlikely to happen in the first place because it requires a series of somewhat unlikely things to happen in very quick succession. However, assertion failures are bad, so rearrange the code to avoid that possibility. Patch by me, reviewed by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com
* Add PROCESS_MAIN to VACUUMMichael Paquier2023-03-06
| | | | | | | | | | | | | | | | | | | | | | | | | Disabling this option is useful to run VACUUM (with or without FULL) on only the toast table of a relation, bypassing the main relation. This option is enabled by default. Running directly VACUUM on a toast table was already possible without this feature, by using the non-deterministic name of a toast relation (as of pg_toast.pg_toast_N, where N would be the OID of the parent relation) in the VACUUM command, and it required a scan of pg_class to know the name of the toast table. So this feature is basically a shortcut to be able to run VACUUM or VACUUM FULL on a toast relation, using only the name of the parent relation. A new switch called --no-process-main is added to vacuumdb, to work as an equivalent of PROCESS_MAIN. Regression tests are added to cover VACUUM and VACUUM FULL, looking at pg_stat_all_tables.vacuum_count to see how many vacuums have run on each table, main or toast. Author: Nathan Bossart Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/20221230000028.GA435655@nathanxps13
* Deduplicate handling of binary and text modes in logicalrep_read_tuple().Amit Kapila2023-03-06
| | | | | | Author: Bharath Rupireddy Reviewed-by: Peter Smith Discussion: https://postgr.es/m/CALj2ACXdbq7kW_+bRrSGMsR6nefCvwbHBJ5J51mr3gFf7QysTA@mail.gmail.com
* Revise pg_pwrite_zeros()Michael Paquier2023-03-06
| | | | | | | | | | | | | | | | | The following changes are made to pg_write_zeros(), the API able to write series of zeros using vectored I/O: - Add of an "offset" parameter, to write the size from this position (the 'p' of "pwrite" seems to mean position, though POSIX does not outline ythat directly), hence the name of the routine is incorrect if it is not able to handle offsets. - Avoid memset() of "zbuffer" on every call. - Avoid initialization of the whole IOV array if not needed. - Group the trailing write() call with the main write() call, simplifying the function logic. Author: Andres Freund Reviewed-by: Michael Paquier, Bharath Rupireddy Discussion: https://postgr.es/m/20230215005525.mrrlmqrxzjzhaipl@awork3.anarazel.de
* Fix assert failures in parallel SERIALIZABLE READ ONLY.Thomas Munro2023-03-06
| | | | | | | | | | | | | | | | | | | | | | 1. Make sure that we don't decrement SxactGlobalXminCount twice when the SXACT_FLAG_RO_SAFE optimization is reached in a parallel query. This could trigger a sanity check failure in assert builds. Non-assert builds recompute the count in SetNewSxactGlobalXmin(), so the problem was hidden, explaining the lack of field reports. Add a new isolation test to exercise that case. 2. Remove an assertion that the DOOMED flag can't be set on a partially released SERIALIZABLEXACT. Instead, ignore the flag (our transaction was already determined to be read-only safe, and DOOMED is in fact set during partial release, and there was already an assertion that it wasn't set sooner). Improve an existing isolation test so that it reaches that case (previously it wasn't quite testing what it was supposed to be testing; see discussion). Back-patch to 12. Bug #17116. Defects in commit 47a338cf. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org
* SQL JSON path enhanced numeric literalsPeter Eisentraut2023-03-05
| | | | | | | | | | | | | Add support for non-decimal integer literals and underscores in numeric literals to SQL JSON path language. This follows the rules of ECMAScript, as referred to by the SQL standard. Internally, all the numeric literal parsing of jsonpath goes through numeric_in, which already supports all this, so this patch is just a bit of lexer work and some tests and documentation. Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/b11b25bb-6ec1-d42f-cedd-311eae59e1fb@enterprisedb.com
* Avoid failure when altering state of partitioned foreign-key triggers.Tom Lane2023-03-04
| | | | | | | | | | | | | | | | | | | | | | Beginning in v15, if you apply ALTER TABLE ENABLE/DISABLE TRIGGER to a partitioned table, it also affects the partitions' cloned versions of the affected trigger(s). The initial implementation of this located the clones by name, but that fails on foreign-key triggers which have names incorporating their own OIDs. We can fix that, and also make the behavior more bulletproof in the face of user-initiated trigger renames, by identifying the cloned triggers by tgparentid. Following the lead of earlier commits in this area, I took care not to break ABI in the v15 branch, even though I rather doubt there are any external callers of EnableDisableTrigger. While here, update the documentation, which was not touched when the semantics were changed. Per bug #17817 from Alan Hodgson. Back-patch to v15; older versions do not have this behavior. Discussion: https://postgr.es/m/17817-31dfb7c2100d9f3d@postgresql.org
* meson: Prevent installation of test files during main installPeter Eisentraut2023-03-03
| | | | | | | | | | | | | | | Previously, meson installed modules under src/test/modules/ as part of a normal installation, even though these files are only meant for use by tests. This is because there is no way to set up up the build system to install extra things only when told. This patch fixes that with a workaround: We don't install these modules as part of meson install, but we create a new "test" that runs before the real tests whose action it is to install these files. The installation is done by manual copies using a small helper script. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/2a039e8e-f31f-31e8-afe7-bab3130ad2de%40enterprisedb.com
* Fix incorrect format placeholdersPeter Eisentraut2023-03-03
|
* Don't leak descriptors into subprograms.Thomas Munro2023-03-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Open long-lived data and WAL file descriptors with O_CLOEXEC. This flag was introduced by SUSv4 (POSIX.1-2008), and by now all of our target Unix systems have it. Our open() implementation for Windows already had that behavior, so provide a dummy O_CLOEXEC flag on that platform. For now, callers of open() and the "thin" wrappers in fd.c that deal in raw descriptors need to pass in O_CLOEXEC explicitly if desired. This commit does that for WAL files, and automatically for everything accessed via VFDs including SMgrRelation and BufFile. (With more discussion we might decide to turn it on automatically for the thin open()-wrappers too to avoid risk of missing places that need it, but these are typically used for short-lived descriptors where we don't expect to fork/exec, and it's remotely possible that extensions could be using these APIs and passing descriptors to subprograms deliberately, so that hasn't been done here.) Do the same for sockets and the postmaster pipe with FD_CLOEXEC. (Later commits might use modern interfaces to remove these extra fcntl() calls and more where possible, but we'll need them as a fallback for a couple of systems, so do it that way in this initial commit.) With this change, subprograms executed for archiving, copying etc will no longer have access to the server's descriptors, other than the ones that we decide to pass down. Reviewed-by: Andres Freund <andres@anarazel.de> (earlier version) Discussion: https://postgr.es/m/CA%2BhUKGKb6FsAdQWcRL35KJsftv%2B9zXqQbzwkfRf1i0J2e57%2BhQ%40mail.gmail.com
* Remove local optimizations of empty Bitmapsets into null pointers.Tom Lane2023-03-02
| | | | | | | | These are all dead code now that it's done centrally. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Require empty Bitmapsets to be represented as NULL.Tom Lane2023-03-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When I designed the Bitmapset module, I set things up so that an empty Bitmapset could be represented either by a NULL pointer, or by an allocated object all of whose bits are zero. I've recently come to the conclusion that that was a bad idea and we should instead have a convention like the longstanding invariant for Lists, whereby an empty list is represented by NIL and nothing else. To do this, we need to fix bms_intersect, bms_difference, and a couple of other functions to check for having produced an empty result; but then we can replace bms_is_empty(a) by a simple "a == NULL" test. This is very likely a (marginal) win performance-wise, because we call bms_is_empty many more times than those other functions put together. However, the real reason to do it is that we have various places that have hand-implemented a rule about "this Bitmapset variable must be exactly NULL if empty", so that they can use checks-for-null in place of bms_is_empty calls in particularly hot code paths. That is a really fragile, mistake-prone way to do things, and I'm surprised that we've seldom been bitten by it. It's not well documented at all which variables have this property, so you can't readily tell which code might be violating those conventions. By making the convention universal, we can eliminate a subtle source of bugs. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Mop up some undue familiarity with the innards of Bitmapsets.Tom Lane2023-03-02
| | | | | | | | | | | | | | | | | | | | | nodeAppend.c used non-nullness of appendstate->as_valid_subplans as a state flag to indicate whether it'd done ExecFindMatchingSubPlans (or some sufficient approximation to that). This was pretty questionable even in the beginning, since it wouldn't really work right if there are no valid subplans. It got more questionable after commit 27e1f1456 added logic that could reduce as_valid_subplans to an empty set: at that point we were depending on unspecified behavior of bms_del_members, namely that it'd not return an empty set as NULL. It's about to start doing that, which breaks this logic entirely. Hence, add a separate boolean flag to signal whether as_valid_subplans has been computed. Also fix a previously-cosmetic bug in nodeAgg.c, wherein it ignored the return value of bms_del_member instead of updating its pointer. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Remove bms_first_member().Tom Lane2023-03-02
| | | | | | | | | | | | | | This function has been semi-deprecated ever since we invented bms_next_member(). Its habit of scribbling on the input bitmapset isn't great, plus for sufficiently large bitmapsets it would take O(N^2) time to complete a loop. Now we have the additional problem that reducing the input to empty while leaving it still accessible would violate a planned invariant. So let's just get rid of it, after updating the few extant callers to use bms_next_member(). Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Mark options as deprecated in usage outputDaniel Gustafsson2023-03-02
| | | | | | | | | Some deprecated options were not marked as such in usage output. This does so across the installed binaries in an attempt to provide consistent markup for this. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/062C6A8A-A4E8-4F52-9E31-45F0C9E9915E@yesql.se
* Fix outdated references to guc.cDaniel Gustafsson2023-03-02
| | | | | | | | | Commit 0a20ff54f split out the GUC variables from guc.c into a new file guc_tables.c. This updates comments referencing guc.c regarding variables which are now in guc_tables.c. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/6B50C70C-8C1F-4F9A-A7C0-EEAFCC032406@yesql.se
* Make some xlogreader messages more accuratePeter Eisentraut2023-03-02
| | | | | | | | | | | | When you have some invalid WAL, you often get a message like "wanted 24, got 0". This is a bit incorrect, since it really wanted *at least* 24, not exactly 24. This updates the messages to that effect, and also adds that detail to one message where it was available but not printed. Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Jeevan Ladhe <jeevanladhe.os@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/726d782b-5e45-0c3e-d775-6686afe9aa83%40enterprisedb.com
* Avoid fetching one past the end of translate()'s "to" parameter.Tom Lane2023-03-01
| | | | | | | | | | | | | | | | | This is usually harmless, but if you were very unlucky it could provoke a segfault due to the "to" string being right up against the end of memory. Found via valgrind testing (so we might've found it earlier, except that our regression tests lacked any exercise of translate()'s deletion feature). Fix by switching the order of the test-for-end-of-string and advance-pointer steps. While here, compute "to_ptr + tolen" just once. (Smarter compilers might figure that out for themselves, but let's just make sure.) Report and fix by Daniil Anisimov, in bug #17816. Discussion: https://postgr.es/m/17816-70f3d2764e88a108@postgresql.org
* Suppress more compiler warnings in new pgstats code.Tom Lane2023-02-28
| | | | | | | | Per buildfarm, we didn't get rid of quite all of the -Wtautological-constant-out-of-range-compare warnings in pgstat_io.c. Discussion: https://postgr.es/m/20520.1677435600@sss.pgh.pa.us
* Rework pg_input_error_message(), now renamed pg_input_error_info()Michael Paquier2023-02-28
| | | | | | | | | | | | | | | | | | | pg_input_error_info() is now a SQL function able to return a row with more than just the error message generated for incorrect data type inputs when these are able to handle soft failures, returning more contents of ErrorData, as of: - The error message (same as before). - The error detail, if set. - The error hint, if set. - SQL error code. All the regression tests that relied on pg_input_error_message() are updated to reflect the effects of the rename. Per discussion with Tom Lane and Andrew Dunstan. Author: Nathan Bossart Discussion: https://postgr.es/m/139a68e1-bd1f-a9a7-b5fe-0be9845c6311@dunslane.net
* Suppress compiler warnings in new pgstats code.Tom Lane2023-02-27
| | | | | | | | | | | | | | | | | | | | | Some clang versions whine about comparing an enum variable to a value outside the range of the enum, on the grounds that the result must be constant. In the cases we fix here, the loops will terminate only if the enum variable can in fact hold a value one beyond its declared range. While that's very likely to always be true for these enum types, it still seems like a poor coding practice to assume it; so use "int" loop variables instead to silence the warnings. (This matches what we've done in other places, for example loops over the range of ForkNumber.) While at it, let's drop the XXX_FIRST macros for these enums and just write zeroes for the loop start values. The apparent flexibility seems rather illusory given that iterating up to one-less-than- the-number-of-values is only correct for a zero-based range. Melanie Plageman Discussion: https://postgr.es/m/20520.1677435600@sss.pgh.pa.us