aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix incorrect non-strict join recheck in MERGE WHEN NOT MATCHED BY SOURCE.Dean Rasheed2024-10-03
| | | | | | | | | | | | | | | | | | | | | | If a MERGE command contains WHEN NOT MATCHED BY SOURCE actions, the merge join condition is used by the executor to distinguish MATCHED from NOT MATCHED BY SOURCE cases. However, this qual is executed using the output from the join subplan node, which nulls the output from the source relation in the not matched case, and so the result may be incorrect if the join condition is "non-strict" -- for example, something like "src.col IS NOT DISTINCT FROM tgt.col". Fix this by enhancing the join recheck condition with an additional "src IS NOT NULL" check, so that it does the right thing when evaluated using the output from the join subplan. Noted by Tom Lane while investigating bug #18634 from Alexander Lakhin. Back-patch to v17, where WHEN NOT MATCHED BY SOURCE support was added to MERGE. Discussion: https://postgr.es/m/18634-db5299c937877f2b%40postgresql.org
* Replace Unicode apostrophe with ASCII apostropheAmit Langote2024-10-03
| | | | | | | | | In commit babb3993dbe9, I accidentally introduced a Unicode apostrophe (U+2019). This commit replaces it with the ASCII apostrophe (U+0027) for consistency. Reported-by: Alexander Korotkov <aekorotkov@gmail.com> Discussion: https://postgr.es/m/CAPpHfduNWMBjkJFtqXJremk6b6YQYO2s3_VEpnj-T_CaUNUYYQ@mail.gmail.com
* Refactor CopyFrom() in copyfrom.c.Fujii Masao2024-10-03
| | | | | | | | | | | | | | | | | | This commit simplifies CopyFrom() by removing the unnecessary local variable 'skipped', which tracked the number of rows skipped due to on_error = 'ignore'. That count is already handled by cstate->num_errors, so the 'skipped' variable was redundant. Additionally, the condition on_error != COPY_ON_ERROR_STOP is removed. Since on_error == COPY_ON_ERROR_IGNORE is already checked, and on_error only has two values (ignore and stop), the additional check was redundant and made the logic harder to read. Seemingly this was introduced in preparation for a future patch, but the current checks don’t offer clear value and have been removed to improve readability. Author: Atsushi Torikoshi Reviewed-by: Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com
* Add log_verbosity = 'silent' support to COPY command.Fujii Masao2024-10-03
| | | | | | | | | | | | | | | | | | | | | Previously, when the on_error option was set to ignore, the COPY command would always log NOTICE messages for input rows discarded due to data type incompatibility. Users had no way to suppress these messages. This commit introduces a new log_verbosity setting, 'silent', which prevents the COPY command from emitting NOTICE messages when on_error = 'ignore' is used, even if rows are discarded. This feature is particularly useful when processing malformed files frequently, where a flood of NOTICE messages can be undesirable. For example, when frequently loading malformed files via the COPY command or querying foreign tables using file_fdw (with an upcoming patch to add on_error support for file_fdw), users may prefer to suppress these messages to reduce log noise and improve clarity. Author: Atsushi Torikoshi Reviewed-by: Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/ab59dad10490ea3734cf022b16c24cfd@oss.nttdata.com
* Fix expression list handling in ATExecAttachPartition()Amit Langote2024-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit addresses two issues related to the manipulation of the partition constraint expression list in ATExecAttachPartition(). First, the current use of list_concat() to combine the partition's constraint (retrieved via get_qual_from_partbound()) with the parent table’s partition constraint can lead to memory safety issues. After calling list_concat(), the original constraint (partBoundConstraint) might no longer be safe to access, as list_concat() may free or modify it. Second, there's a logical error in constructing the constraint for validating against the default partition. The current approach incorrectly includes a negated version of the parent table's partition constraint, which is redundant, as it always evaluates to false for rows in the default partition. To resolve these issues, list_concat() is replaced with list_concat_copy(), ensuring that partBoundConstraint remains unchanged and can be safely reused when constructing the validation constraint for the default partition. This fix is not applied to back-branches, as there is no live bug and the issue has not caused any reported problems in practice. Nitin Jadhav posted a patch to address the memory safety issue, but I decided to follow Alvaro Herrera's suggestion from the initial discussion, as it allows us to fix both the memory safety and logical issues. Reported-by: Andres Freund <andres@anarazel.de> Reported-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/20231115165737.zeulb575cgrbqo74@awork3.anarazel.de Discussion: https://postgr.es/m/CAMm1aWbmYHM3bqtjyMQ-a+4Ub=dgsb_2E3_up2cn=UGdHNrGTg@mail.gmail.com
* Remove support for unlogged on partitioned tablesMichael Paquier2024-10-03
| | | | | | | | | | | | | | | | | | | | | | The following commands were allowed on partitioned tables, with different effects: 1) ALTER TABLE SET [UN]LOGGED did not issue an error, and did not update pg_class.relpersistence. 2) CREATE UNLOGGED TABLE was working with pg_class.relpersistence marked as initially defined, but partitions did not inherit the UNLOGGED property, which was confusing. This commit causes the commands mentioned above to fail for partitioned tables, instead. pg_dump is tweaked so as partitioned tables marked as UNLOGGED ignore the option when dumped from older server versions. pgbench needs a tweak for --unlogged and --partitions=N to ignore the UNLOGGED option on the partitioned tables created, its partitions still being unlogged. Author: Michael Paquier Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/ZiiyGFTBNkqcMQi_@paquier.xyz
* Adjust json_manifest_per_file_callback API in one more place.Tom Lane2024-10-02
| | | | | | Oversight in commit d94cf5ca7 (and in my testing of same). Discussion: https://postgr.es/m/9468.1727895630@sss.pgh.pa.us
* Parse libpq's "keepalives" option more like other integer options.Tom Lane2024-10-02
| | | | | | | | | | | | | | | | | | | | | Use pqParseIntParam (nee parse_int_param) instead of using strtol directly. This allows trailing whitespace, which the previous coding didn't, and makes the spelling of the error message consistent with other similar cases. This seems to be an oversight in commit e7a221797, which introduced parse_int_param. That fixed places that were using atoi(), but missed this place which was randomly using strtol() instead. Ordinarily I'd consider this minor cleanup not worth back-patching. However, it seems that ecpg assumes it can add trailing whitespace to URL parameters, so that use of the keepalives option fails in that context. Perhaps that's worth improving as a separate matter. In the meantime, back-patch this to all supported branches. Yuto Sasaki (some further cleanup by me) Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
* File size in a backup manifest should use uint64, not size_t.Robert Haas2024-10-02
| | | | | | | | size_t is the size of an object in memory, not the size of a file on disk. Thanks to Tom Lane for noting the error. Discussion: http://postgr.es/m/1865585.1727803933@sss.pgh.pa.us
* Add fastpaths for when no objects are foundDaniel Gustafsson2024-10-02
| | | | | | | | | | | If there are no objects found, there is no reason to inspect the result columns and mallocing a zero-sized (which will be 1 byte in reality) heap buffer for it. Add a fast-path for immediately returning like how other object inspection functions are already doing it. Reviewed-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se
* Remove superfluous PQExpBuffer resettingDaniel Gustafsson2024-10-02
| | | | | | | | Since the buffer was just created, there is no reason to immediately reset it. Reviewed-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/C2F05B3C-1414-45DD-AE09-6FEE4D0F89BD@yesql.se
* Fix inconsistent reporting of checkpointer stats.Fujii Masao2024-10-02
| | | | | | | | | | | | | | | | | | | | | | | Previously, the pg_stat_checkpointer view and the checkpoint completion log message could show different numbers for buffers written during checkpoints. The view only counted shared buffers, while the log message included both shared and SLRU buffers, causing inconsistencies. This commit resolves the issue by updating both the view and the log message to separately report shared and SLRU buffers written during checkpoints. A new slru_written column is added to the pg_stat_checkpointer view to track SLRU buffers, while the existing buffers_written column now tracks only shared buffers. This change would help users distinguish between the two types of buffers, in the pg_stat_checkpointer view and the checkpoint complete log message, respectively. Bump catalog version. Author: Nitin Jadhav Reviewed-by: Bharath Rupireddy, Michael Paquier, Kyotaro Horiguchi, Robert Haas Reviewed-by: Andres Freund, vignesh C, Fujii Masao Discussion: https://postgr.es/m/CAMm1aWb18EpT0whJrjG+-nyhNouXET6ZUw0pNYYAe+NezpvsAA@mail.gmail.com
* Reject a copy EOF marker that has data ahead of it on the same line.Tom Lane2024-10-01
| | | | | | | | | | | | | | | | | | We have always documented that a copy EOF marker (\.) must appear by itself on a line, and that is how psql interprets the rule. However, the backend's actual COPY FROM logic only insists that there not be data between the \. and the following newline. Any data ahead of the \. is parsed as a final line of input. It's hard to interpret this as anything but an ancient mistake that we've faithfully carried forward. Continuing to allow it is not cost-free, since it could mask client-side bugs that unnecessarily backslash-escape periods (and thereby risk accidentally creating an EOF marker). So, let's remove that provision and throw error if the EOF marker isn't alone on its line, matching what the documentation has said right along. Adjust the relevant error messages to be clearer, too. Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org
* initdb: Add new option "--no-data-checksums"Peter Eisentraut2024-10-01
| | | | | | | | | | Right now this does nothing except override any earlier --data-checksums option. But the idea is that --data-checksums could become the default, and then this option would allow forcing it off instead. Author: Greg Sabino Mullane <greg@turnstep.com> Discussion: https://www.postgresql.org/message-id/flat/CAKAnmmKwiMHik5AHmBEdf5vqzbOBbcwEPHo4-PioWeAbzwcTOQ@mail.gmail.com
* Use macro to define the number of enum valuesPeter Eisentraut2024-10-01
| | | | | | | | | | | | Refactoring in the interest of code consistency, a follow-up to 2e068db56e31. The argument against inserting a special enum value at the end of the enum definition is that a switch statement might generate a compiler warning unless it has a default clause. Aleksander Alekseev, reviewed by Michael Paquier, Dean Rasheed, Peter Eisentraut Discussion: https://postgr.es/m/CAJ7c6TMsiaV5urU_Pq6zJ2tXPDwk69-NKVh4AMN5XrRiM7N%2BGA%40mail.gmail.com
* Fix some pg_verifybackup issues reported by Coverity.Robert Haas2024-10-01
| | | | | | | | | | | | | | Commit 8dfd3129027969fdd2d9d294220c867d2efd84aa introduced a few problems. verify_tar_file() forgot to free a buffer; the leak can't add up to anything material, but might as well fix it. precheck_tar_backup_file() intended to return after reporting an error but didn't actually do so. member_copy_control_data() could try to copy zero bytes (and maybe Coverity thinks it can even be trying to copy a negative number of bytes). Per discussion with Tom Lane. Discussion: http://postgr.es/m/1240823.1727629418@sss.pgh.pa.us
* Simplify checking for xlocale.hPeter Eisentraut2024-10-01
| | | | | | | | | | | Instead of XXX_IN_XLOCALE_H for several features XXX, let's just include <xlocale.h> if HAVE_XLOCALE_H. The reason for the extra complication was apparently that some old glibc systems also had an <xlocale.h>, and you weren't supposed to include it directly, but it's gone now (as far as I can tell it was harmless to do so anyway). Author: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
* jit: Use opaque pointers in all supported LLVM versions.Peter Eisentraut2024-10-01
| | | | | | | | | | | | | | | | | | LLVM's opaque pointer change began in LLVM 14, but remained optional until LLVM 16. When commit 37d5babb added opaque pointer support, we didn't turn it on for LLVM 14 and 15 yet because we didn't want to risk weird bitcode incompatibility problems in released branches of PostgreSQL. (That might have been overly cautious, I don't know.) Now that PostgreSQL 18 has dropped support for LLVM versions < 14, and since it hasn't been released yet and no extensions or bitcode have been built against it in the wild yet, we can be more aggressive. We can rip out the support code and build system clutter that made opaque pointer use optional. Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussions: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com
* jit: Require at least LLVM 14, if enabled.Peter Eisentraut2024-10-01
| | | | | | | | | | Remove support for LLVM versions 10-13. The default on all non-EOL'd OSes represented in our build farm will be at least LLVM 14 when PostgreSQL 18 ships. Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/CA%2BhUKGLhNs5geZaVNj2EJ79Dx9W8fyWUU3HxcpZy55sMGcY%3DiA%40mail.gmail.com
* Fix race condition in COMMIT PREPARED causing orphaned 2PC filesMichael Paquier2024-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | COMMIT PREPARED removes on-disk 2PC files near its end, but the state checked if a file is on-disk or not gets read from shared memory while not holding the two-phase state lock. Because of that, there was a small window where a second backend doing a PREPARE TRANSACTION could reuse the GlobalTransaction put back into the 2PC free list by the COMMIT PREPARED, overwriting the "ondisk" flag read afterwards by the COMMIT PREPARED to decide if its on-disk two-phase state file should be removed, preventing the file deletion. This commit fixes this issue so as the "ondisk" flag in the GlobalTransaction is read while holding the two-phase state lock, not from shared memory after its entry has been added to the free list. Orphaned two-phase state files flushed to disk after a checkpoint are discarded at the beginning of recovery. However, a truncation of pg_xact/ would make the startup process issue a FATAL when it cannot read the SLRU page holding the state of the transaction whose 2PC file was orphaned, which is a necessary step to decide if the 2PC file should be removed or not. Removing manually the file would be necessary in this case. Issue introduced by effe7d9552dd, so backpatch all the way down. Mea culpa. Author: wuchengwen Discussion: https://postgr.es/m/tencent_A7F059B5136A359625C7B2E4A386B3C3F007@qq.com Backpatch-through: 12
* Expand assertion check for query ID reporting in executorMichael Paquier2024-10-01
| | | | | | | | | | | | | | | | | | As formulated, the assertion added in the executor by 24f520594809 to check that a query ID is set had two problems: - track_activities may be disabled while compute_query_id is enabled, causing the query ID to not be reported to pg_stat_activity. - debug_query_string may not be set in some context. The only path where this would matter is visibly autovacuum, should parallel workers be enabled there at some point. This is not the case currently. There was no test showing the interactions between the query ID and track_activities, so let's add one based on a scan of pg_stat_activity. This assertion is still an experimentation at this stage, but let's see if this shows more paths where query IDs are not properly set while they should. Discussion: https://postgr.es/m/Zvn5616oYXmpXyHI@paquier.xyz
* Add missing command for pg_maintain in commentDaniel Gustafsson2024-10-01
| | | | | | | | | The comment in pg_class_aclmask_ext() which lists the allowed commands for the pg_maintain role lacked LOCK TABLE. Reported-by: Yusuke Sugie <btsugieyuusuke@oss.nttdata.com> Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp> Discussion: https://postgr.es/m/034d3c60f5daba1919cd90f236b2e22d@oss.nttdata.com
* Do not treat \. as an EOF marker in CSV mode for COPY IN.Tom Lane2024-09-30
| | | | | | | | | | | | | | | | | | | | | | | | Since backslash is (typically) not special in CSV data, we should not be treating \. as special either. The server historically did this to keep CSV and TEXT modes more alike and to support V2 protocol; but V2 protocol is long dead, and the inconsistency with CSV standards is annoying. Remove that behavior in CopyReadLineText, and make some minor consequent code simplifications. On the client side, we need to fix psql so that it does not check for \. except when reading data from STDIN (that is, the script source). We must do that regardless of TEXT/CSV mode or there is no way to end the COPY short of script EOF. Also, be careful not to send the \. to the server in that case. This is a small compatibility break in that other applications beside psql may need similar adjustment. Also, using an older version of psql with a v18 server may result in misbehavior during CSV-mode COPY IN. Daniel Vérité, reviewed by vignesh C, Robert Haas, and myself Discussion: https://postgr.es/m/ed659f37-a9dd-42a7-82b9-0da562cc4006@manitou-mail.org
* Remove incorrect entries in pg_walsummary's getopt_long call.Tom Lane2024-09-30
| | | | | | | | | | | For some reason this listed "-f" and "-w" as valid switches, though the code doesn't implement any such thing nor do the docs mention them. The effect of this was that if you tried to use one of these switches, you'd get an unhelpful error message. Yusuke Sugie Discussion: https://postgr.es/m/68e72a2a70f4d84c1c7847b13bcdaef8@oss.nttdata.com
* Don't disallow DROP of constraints ONLY on partitioned tablesAlvaro Herrera2024-09-30
| | | | | | | | | | | | | | | | | This restriction seems to have come about due to some fuzzy thinking: in commit 9139aa19423b we were adding a restriction against ADD constraint ONLY on partitioned tables (which is sensible) and apparently we thought the DROP case had to be symmetrical. However, it isn't, and the comments about it are mistaken about the effect it would have. Remove this limitation. There have been no reports of users bothered by this limitation, so I'm not backpatching it just yet. We can revisit this decision later, as needed. Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/202409261752.nbvlawkxsttf@alvherre.pgsql Discussion: https://postgr.es/m/7682253a-6f79-6a92-00aa-267c4c412870@lab.ntt.co.jp (about commit 9139aa19423b, previously not registered)
* Bump catalog version for change in VariableSetStmtMichael Paquier2024-09-30
| | | | | | | | Oversight in dc68515968e8, as this breaks SQL functions with a SET command. Reported-by: Tom Lane Discussion: https://postgr.es/m/1364409.1727673407@sss.pgh.pa.us
* Show values of SET statements as constants in pg_stat_statementsMichael Paquier2024-09-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a continuation of work like 11c34b342bd7, done to reduce the bloat of pg_stat_statements by applying more normalization to query entries. This commit is able to detect and normalize values in VariableSetStmt, resulting in: SET conf_param = $1 Compared to other parse nodes, VariableSetStmt is embedded in much more places in the parser, impacting many query patterns in pg_stat_statements. A custom jumble function is used, with an extra field in the node to decide if arguments should be included in the jumbling or not, a location field being not enough for this purpose. This approach allows for a finer tuning. Clauses relying on one or more keywords are not normalized, for example: * DEFAULT * FROM CURRENT * List of keywords. SET SESSION CHARACTERISTICS AS TRANSACTION, where it is critical to differentiate different sets of options, is a good example of why normalization should not happen. Some queries use VariableSetStmt for some subclauses with SET, that also have their values normalized: - ALTER DATABASE - ALTER ROLE - ALTER SYSTEM - CREATE/ALTER FUNCTION ba90eac7a995 has added test coverage for most of the existing SET patterns. The expected output of these tests shows the difference this commit creates. Normalization could be perhaps applied to more portions of the grammar but what is done here is conservative, and good enough as a starting point. Author: Greg Sabino Mullane, Michael Paquier Discussion: https://postgr.es/m/36e5bffe-e989-194f-85c8-06e7bc88e6f7@amazon.com Discussion: https://postgr.es/m/B44FA29D-EBD0-4DD9-ABC2-16F1CB087074@amazon.com Discussion: https://postgr.es/m/CAKAnmmJtJY2jzQN91=2QAD2eAJAA-Per61eyO48-TyxEg-q0Rg@mail.gmail.com
* Add num_done counter to the pg_stat_checkpointer view.Fujii Masao2024-09-30
| | | | | | | | | | | | | | | Checkpoints can be skipped when the server is idle. The existing num_timed and num_requested counters in pg_stat_checkpointer track both completed and skipped checkpoints, but there was no way to count only the completed ones. This commit introduces the num_done counter, which tracks only completed checkpoints, making it easier to see how many were actually performed. Bump catalog version. Author: Anton A. Melnikov Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/9ea77f40-818d-4841-9dee-158ac8f6e690@oss.nttdata.com
* reindexdb: Skip reindexing temporary tables and indexes.Fujii Masao2024-09-30
| | | | | | | | | | | | | | | | | | | | Reindexing temp tables or indexes of other sessions is not allowed. However, reindexdb in parallel mode previously listed them as the objects to process, leading to failures. This commit ensures reindexdb in parallel mode skips temporary tables and indexes by adding a condition based on the relpersistence column in pg_class to the object listing queries, preventing these issues. Note that this commit does not affect reindexdb when temporary tables or indexes are explicitly specified using the -t or -j options; reindexdb in that case still does not skip them and can cause an error. Back-patch to v13 where parallel mode was introduced in reindexdb. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/5f37ee56-14fb-44fe-9150-9eb97e10538b@oss.nttdata.com
* Set query ID in parallel workers for vacuum, BRIN and btreeMichael Paquier2024-09-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | All these code paths use their own entry point when starting parallel workers, but failed to set a query ID, even if they set a text query. Hence, this data would be missed in pg_stat_activity for the worker processes. The main entry point for parallel query processing, ParallelQueryMain(), is already doing that by saving its query ID in a dummy PlannedStmt, but not the others. The code is changed so as the query ID of these queries is set in their shared state, and reported back once the parallel workers start. Some tests are added to show how the failures can happen for btree and BRIN with a parallel build enforced, which are able to trigger a failure in an assertion added by 24f520594809 in the recovery TAP test 027_stream_regress.pl where pg_stat_statements is always loaded. In this case, the executor path was taken because the index expression needs to be flattened when building its IndexInfo. Alexander Lakhin has noticed the problem in btree, and I have noticed that the issue was more spread. This is arguably a bug, but nobody has complained about that until now, so no backpatch is done out of caution. If folks would like to see a backpatch, well, let me know. Reported-by: Alexander Lakhin Reviewed-by: Sami Imseih Discussion: https://postgr.es/m/cf3547c1-498a-6a61-7b01-819f902a251f@gmail.com
* Remove NULL dereference from RenameRelationInternal().Noah Misch2024-09-29
| | | | | | Defect in last week's commit aac2c9b4fde889d13f859c233c2523345e72d32b, per Coverity. Reaching this would need catalog corruption. Back-patch to v12, like that commit.
* In passwordFromFile, don't leak the open file after stat failures.Tom Lane2024-09-29
| | | | Oversight in e882bcae0. Per Coverity.
* Avoid 037_invalid_database.pl hang under debug_discard_caches.Noah Misch2024-09-27
| | | | Back-patch to v12 (all supported versions).
* Recalculate where-needed data accurately after a join removal.Tom Lane2024-09-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, remove_rel_from_query() has done a pretty shoddy job of updating our where-needed bitmaps (per-Var attr_needed and per-PlaceHolderVar ph_needed relid sets). It removed direct mentions of the to-be-removed baserel and outer join, which is the minimum amount of effort needed to keep the data structures self-consistent. But it didn't account for the fact that the removed join ON clause probably mentioned Vars of other relations, and those Vars might now not be needed as high up in the join tree as before. It's easy to show cases where this results in failing to remove a lower outer join that could also have been removed. To fix, recalculate the where-needed bitmaps from scratch after each successful join removal. This sounds expensive, but it seems to add only negligible planner runtime. (We cheat a little bit by preserving "relation 0" entries in the bitmaps, allowing us to skip re-scanning the targetlist and HAVING qual.) The submitted test case drew attention because we had successfully optimized away the lower join prior to v16. I suspect that that's somewhat accidental and there are related cases that were never optimized before and now can be. I've not tried to come up with one, though. Perhaps we should back-patch this into v16 and v17 to repair the performance regression. However, since it took a year for anyone to notice the problem, it can't be affecting too many people. Let's let the patch bake awhile in HEAD, and see if we get more complaints. Per bug #18627 from Mikaël Gourlaouen. No back-patch for now. Discussion: https://postgr.es/m/18627-44f950eb6a8416c2@postgresql.org
* Reindent pg_verifybackup.c.Robert Haas2024-09-27
|
* pg_verifybackup: Verify tar-format backups.Robert Haas2024-09-27
| | | | | | | | This also works for compressed tar-format backups. However, -n must be used, because we use pg_waldump to verify WAL, and it doesn't yet know how to verify WAL that is stored inside of a tarfile. Amul Sul, reviewed by Sravan Kumar and by me, and revised by me.
* Fix typo in pg_walsummary/nls.mk.Fujii Masao2024-09-27
| | | | | Author: Koki Nakamura Discussion: https://postgr.es/m/485c613d1db8de2e8169d5afd43e7f9e@oss.nttdata.com
* Fix incorrect memory access in VACUUM FULL with invalid toast indexesMichael Paquier2024-09-27
| | | | | | | | | | | | | | | | | | | | | | | | An invalid toast index is skipped in reindex_relation(). These would be remnants of a failed REINDEX CONCURRENTLY and they should never been rebuilt as there can only be one valid toast index at a time. REINDEX_REL_SUPPRESS_INDEX_USE, used by CLUSTER and VACUUM FULL, needs to maintain a list of the indexes being processed. The list of indexes is retrieved from the relation cache, and includes invalid indexes. The code has missed that invalid toast indexes are ignored in reindex_relation() as this leads to a hard failure in reindex_index(), and they were left in the reindex pending list, making the list inconsistent when rechecked. The incorrect memory access was happening when scanning pg_class for the refresh of pg_database.datfrozenxid, when doing a scan of pg_class. This issue exists since REINDEX CONCURRENTLY exists, where invalid toast indexes can exist, so backpatch all the way down. Reported-by: Alexander Lakhin Author: Tender Wang Discussion: https://postgr.es/m/18630-9aed99c38830657d@postgresql.org Backpatch-through: 12
* Fix catalog data of new LO privilege functionsMichael Paquier2024-09-27
| | | | | | | | | | | | | This commit improves the catalog data in pg_proc for the three functions for has_largeobject_privilege(), introduced in 4eada203a5a8: - Fix their descriptions (typos and consistency). - Reallocate OIDs to be within the 8000-9999 range as required by a6417078c414. Bump catalog version. Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/ZvUYR0V0dzWaLnsV@paquier.xyz
* Ensure we have a snapshot when updating pg_index entries.Nathan Bossart2024-09-26
| | | | | | | | | | | | | | | | | | | Creating, reindexing, and dropping an index concurrently could entail accessing pg_index's TOAST table, which was recently added in commit b52c4fc3c0. These code paths start and commit their own transactions, but they do not always set an active snapshot. This rightfully leads to assertion failures and ERRORs when trying to access pg_index's TOAST table, such as the following: ERROR: cannot fetch toast data without an active snapshot To fix, push an active snapshot just before each section of code that might require accessing pg_index's TOAST table, and pop it shortly afterwards. Reported-by: Alexander Lakhin Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/a97d7401-e7c9-f771-6a00-037379f0a8bb%40gmail.com
* Improve style of pg_upgrade task callback functions.Nathan Bossart2024-09-26
| | | | | | | | | | | | | | | | I wanted to avoid adjusting this code too much when converting these tasks to use the new parallelization framework (see commit 40e2e5e92b), which is why this is being done as a follow-up commit. These stylistic adjustments result in fewer lines of code and fewer levels of indentation in some places. While at it, add names to the UpgradeTaskSlotState enum and the UpgradeTaskSlot struct. I'm not aware of any established project policy in this area, but let's at least be consistent within the same file. Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/ZunW7XHLd2uTts4f%40nathan
* Modernize to_char's Roman-numeral code, fixing overflow problems.Tom Lane2024-09-26
| | | | | | | | | | | | | | | int_to_roman() only accepts plain "int" input, which is fine since we're going to produce '###############' for any value above 3999 anyway. However, the numeric and int8 variants of to_char() would throw an error if the given input exceeded the integer range, while the float-input variants invoked undefined-per-C-standard behavior. Fix things so that you uniformly get '###############' for out of range input. Also add test cases covering this code, plus the equally-untested EEEE, V, and PL format codes. Discussion: https://postgr.es/m/2956175.1725831136@sss.pgh.pa.us
* Update oid for pg_wal_replay_wait() procedureAlexander Korotkov2024-09-26
| | | | | | | Use an oid from 8000-9999 range, as required by 98eab30b93d5. Reported-by: Michael Paquier Discussion: https://postgr.es/m/ZvUY6bfTwB0GsyzP%40paquier.xyz
* Remove extra whitespace in pg_upgrade status message.Nathan Bossart2024-09-25
| | | | | | | | | | | There's no need to add another level of indentation to this status message. pg_log() will put it in the right place. Oversight in commit 347758b120. Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/ZunW7XHLd2uTts4f%40nathan Backpatch-through: 17
* Turn 'if' condition around to avoid Svace complaintAlvaro Herrera2024-09-25
| | | | | | | | | | | | | | | | | The unwritten assumption of this code is that both events->head and events->tail are NULL together (an empty list) or they aren't. So the code was testing events->head for nullness and using that as a cue to deference events->tail, which annoys the Svace static code analyzer. We can silence it by testing events->tail member instead, and add an assertion about events->head to ensure it's all consistent. This code is very old and as far as we know, there's never been a bug report related to this, so there's no need to backpatch. This was found by the ALT Linux Team using Svace. Author: Alexander Kuznetsov <kuznetsovam@altlinux.org> Discussion: https://postgr.es/m/6d0323c3-3f5d-4137-af73-98a5ab90e77c@altlinux.org
* vacuumdb: Skip temporary tables in query to build list of relationsMichael Paquier2024-09-25
| | | | | | | | | | | | | | | | | Running vacuumdb with a non-superuser while another user has created a temporary table would lead to a mid-flight permission failure, interrupting the operation. vacuum_rel() skips temporary relations of other backends, and it makes no sense for vacuumdb to know about these relations, so let's switch it to ignore temporary relations entirely. Adding a qual in the query based on relpersistence simplifies the generation of its WHERE clause in vacuum_one_database(), per se the removal of "has_where". Author: VaibhaveS, Michael Paquier Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAM_eQjwfAR=y3G1fGyS1U9FTmc+FyJm9amNfY2QCZBnDDbNPZg@mail.gmail.com Backpatch-through: 12
* For inplace update durability, make heap_update() callers wait.Noah Misch2024-09-24
| | | | | | | | | | | | | | | | | | | The previous commit fixed some ways of losing an inplace update. It remained possible to lose one when a backend working toward a heap_update() copied a tuple into memory just before inplace update of that tuple. In catalogs eligible for inplace update, use LOCKTAG_TUPLE to govern admission to the steps of copying an old tuple, modifying it, and issuing heap_update(). This includes MERGE commands. To avoid changing most of the pg_class DDL, don't require LOCKTAG_TUPLE when holding a relation lock sufficient to exclude inplace updaters. Back-patch to v12 (all supported versions). In v13 and v12, "UPDATE pg_class" or "UPDATE pg_database" can still lose an inplace update. The v14+ UPDATE fix needs commit 86dc90056dfdbd9d1b891718d2e5614e3e432f35, and it wasn't worth reimplementing that fix without such infrastructure. Reviewed by Nitin Motiani and (in earlier versions) Heikki Linnakangas. Discussion: https://postgr.es/m/20231027214946.79.nmisch@google.com
* Fix data loss at inplace update after heap_update().Noah Misch2024-09-24
| | | | | | | | | | | | | | | | | | | | | | As previously-added tests demonstrated, heap_inplace_update() could instead update an unrelated tuple of the same catalog. It could lose the update. Losing relhasindex=t was a source of index corruption. Inplace-updating commands like VACUUM will now wait for heap_update() commands like GRANT TABLE and GRANT DATABASE. That isn't ideal, but a long-running GRANT already hurts VACUUM progress more just by keeping an XID running. The VACUUM will behave like a DELETE or UPDATE waiting for the uncommitted change. For implementation details, start at the systable_inplace_update_begin() header comment and README.tuplock. Back-patch to v12 (all supported versions). In back branches, retain a deprecated heap_inplace_update(), for extensions. Reported by Smolkin Grigory. Reviewed by Nitin Motiani, (in earlier versions) Heikki Linnakangas, and (in earlier versions) Alexander Lakhin. Discussion: https://postgr.es/m/CAMp+ueZQz3yDk7qg42hk6-9gxniYbp-=bG2mgqecErqR5gGGOA@mail.gmail.com
* Warn if LOCKTAG_TUPLE is held at commit, under debug_assertions.Noah Misch2024-09-24
| | | | | | | | | | | | The current use always releases this locktag. A planned use will continue that intent. It will involve more areas of code, making unlock omissions easier. Warn under debug_assertions, like we do for various resource leaks. Back-patch to v12 (all supported versions), the plan for the commit of the new use. Reviewed by Heikki Linnakangas. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Allow length=-1 for NUL-terminated input to pg_strncoll(), etc.Jeff Davis2024-09-24
| | | | | | | | | Like ICU, allow a length of -1 to be specified for NUL-terminated arguments to pg_strncoll(), pg_strnxfrm(), and pg_strnxfrm_prefix(). Simplifies the code and comments. Discussion: https://postgr.es/m/2d758e07dff26bcc7cbe2aec57431329bfe3679a.camel@j-davis.com