aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Remove uses of AssertVariableIsOfType() obsoleted by f2b73c8Andres Freund2023-02-08
| | | | | Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/20230208172705.GA451849@nathanxps13
* pgstat: Infrastructure for more detailed IO statisticsAndres Freund2023-02-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds the infrastructure for more detailed IO statistics. The calls to actually count IOs, a system view to access the new statistics, documentation and tests will be added in subsequent commits, to make review easier. While we already had some IO statistics, e.g. in pg_stat_bgwriter and pg_stat_database, they did not provide sufficient detail to understand what the main sources of IO are, or whether configuration changes could avoid IO. E.g., pg_stat_bgwriter.buffers_backend does contain the number of buffers written out by a backend, but as that includes extending relations (always done by backends) and writes triggered by the use of buffer access strategies, it cannot easily be used to tune background writer or checkpointer. Similarly, pg_stat_database.blks_read cannot easily be used to tune shared_buffers / compute a cache hit ratio, as the use of buffer access strategies will often prevent a large fraction of the read blocks to end up in shared_buffers. The new IO statistics count IO operations (evict, extend, fsync, read, reuse, and write), and are aggregated for each combination of backend type (backend, autovacuum worker, bgwriter, etc), target object of the IO (relations, temp relations) and context of the IO (normal, vacuum, bulkread, bulkwrite). What is tracked in this series of patches, is sufficient to perform the aforementioned analyses. Further details, e.g. tracking the number of buffer hits, would make that even easier, but was left out for now, to keep the scope of the already large patchset manageable. Bumps PGSTAT_FILE_FORMAT_ID. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de
* Fix bugs in GetSafeSnapshotBlockingPids(), introduced in 96003717645Andres Freund2023-02-08
| | | | | | | | | | | | | | | | | | While removing the use of SHM_QUEUE from predicate.c, in 96003717645, I made two mistakes in GetSafeSnapshotBlockingPids(): - Removed the check for output_size - Previously, when the first loop didn't find a matching proc, sxact would be NULL. But with naive use of dlist_foreach() it ends up as the value of the last iteration. The second issue is the cause of occasional failures in the deadlock-hard and deadlock-soft isolation tests that we have been observing on CI. The issue was very hard to reproduce, as it requires the transactions.sql regression test to run at the same time as the deadlock-{hard,soft} isolation test. I did not find other similar mistakes in 96003717645. Discussion: https://postgr.es/m/20230208221145.bwzhancellclrgia@awork3.anarazel.de
* pgindent: more ways to find files to indentAndrew Dunstan2023-02-08
| | | | | | | | | | | | | | A new --commit option will add all the files in a commit to the file list. The option can be specified more than once. Also, if a directory is given on the command line, all the files in that directory tree will be added to the file list. Per suggestions from Robert Haas Reviewed by Jelte Fennema Discussion: https://postgr.es/m/CA+TgmoY59Ksso81RNLArNxj0a7xaqV_F_u7gSMHbgdc2kG5Vpw@mail.gmail.com
* Further tighten nullingrel marking rules in build_joinrel_tlist().Tom Lane2023-02-08
| | | | | | | | | | The code I added in fee7b77b9 could misbehave if commute_above_r contains multiple relids. While adding too many relids here is probably harmless (pre-fee7b77b9, we did it all the time), it's not very expensive to be accurate: we just have to intersect commute_above_r with the join's relids. Discussion: https://postgr.es/m/17781-c0405c8b3cd5e072@postgresql.org
* remove_rel_from_query() must clean up PlaceHolderVar.phrels fields.Tom Lane2023-02-08
| | | | | | | | | | While we got away with this sloppiness before, it's not okay now that fee7b77b9 caused build_joinrel_tlist() to make use of phrels. Per report from Robins Tharakan. Richard Guo (some cosmetic tweaks by me) Discussion: https://postgr.es/m/CAMbWs4_ngw9sKxpTE8hqk=-ooVX_CQP3DarA4HzkRMz_JKpTrA@mail.gmail.com
* Remove SQL regression tests for GUCs related to NO_SHOW_ALLMichael Paquier2023-02-08
| | | | | | | | | | | | | | No GUCs that use NO_SHOW_ALL are reported in pg_show_all_settings(), hence trying to check combinations of flags related to it is pointless. These queries have been introduced by d10e41d, so backpatch down to 15 to keep all the branches consistent. Equivalent checks based on NO_SHOW_ALL could be added in check_GUC_init() when a GUC is initially loaded, but this can be done only on HEAD. Author: Nitin Jadhav Discussion: https://postgr.es/m/CAMm1aWaYe0muu3ABo7iSAgK+OWDS9yNe8GGRYnCyeEpScYKa+g@mail.gmail.com Backpatch-through: 15
* Remove stray duplicated comment in heapam.hDavid Rowley2023-02-08
| | | | | | | This is just the same as what's written under the rs_numblocks field. Reported-by: Melanie Plageman Discussion: https://postgr.es/m/20230207204127.7vs6krqjqn5farr7@liskov
* Fix the logical replication timeout during large DDLs.Amit Kapila2023-02-08
| | | | | | | | | | | | | | | | | | | The DDLs like Refresh Materialized views that generate lots of temporary data due to rewrite rules may not be processed by output plugins (for example pgoutput). So, we won't send keep-alive messages for a long time while processing such commands and that can lead the subscriber side to timeout. We have previously fixed a similar case for large transactions in commit f95d53eded where the output plugin filters all or most of the changes but missed to handle the DDLs. We decided not to backpatch this as this adds a new callback in the existing exposed structure and moreover, users can increase the wal_sender_timeout and wal_receiver_timeout to avoid this problem. Author: Wang wei, Hou Zhijie Reviewed-by: Peter Smith, Ashutosh Bapat, Shi yu, Amit Kapila Discussion: https://postgr.es/m/OS3PR01MB6275478E5D29E4A563302D3D9E2B9@OS3PR01MB6275.jpnprd01.prod.outlook.com Discussion: https://postgr.es/m/CAA5-nLARN7-3SLU_QUxfy510pmrYK6JJb=bk3hcgemAM_pAv+w@mail.gmail.com
* Rethink nullingrel marking rules in build_joinrel_tlist().Tom Lane2023-02-07
| | | | | | | | | | | | | | | | | | The logic for when to add the current outer join's own relid to the nullingrels sets of output Vars and PHVs was overly complicated and underly correct. Not sure why I didn't think of this before, but since what we want is marking per the syntactic structure, we can just consult our records about the syntactic structure, ie syn_righthand/syn_lefthand. Also, tighten the rule about when to add the commute_above_r bits, in hopes of eliminating some squishy reasoning. I do not know of a reason to think that that's broken as-is, but this way seems better. Per bug #17781 from Robins Tharakan. Discussion: https://postgr.es/m/17781-c0405c8b3cd5e072@postgresql.org
* Doc: make src/test/*/README match current reality.Tom Lane2023-02-07
| | | | | | | | | Commit c3382a3c3, which moved the implementation of PG_TEST_EXTRA from src/test/Makefile into individual test scripts, broke the directions given in the subdirectory README files about how to run these tests by hand. Update. Also mention wal_consistency_checking in recovery/README --- that omission isn't the fault of c3382a3c3, but it's still an omission.
* Remove leftover code in deconstruct_distribute_oj_quals().Tom Lane2023-02-07
| | | | | | | | | | The initial "put back OJ relids" adjustment of ojscope was incorrect and unnecessary; it seems to be a leftover from when I (tgl) was trying to get this function to work at all. Richard Guo Discussion: https://postgr.es/m/CAMbWs4-L2C47ZGZPabBAi5oDZsKmsbvhYcGCy5o=gCjsaG_ZQA@mail.gmail.com
* Remove useless casts to (void *) in arguments of some system functionsPeter Eisentraut2023-02-07
| | | | | | | | The affected functions are: bsearch, memcmp, memcpy, memset, memmove, qsort, repalloc Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/fd9adf5d-b1aa-e82f-e4c7-263c30145807%40enterprisedb.com
* Use appropriate wait event when sending data in the apply worker.Amit Kapila2023-02-07
| | | | | | | | | | | | | | | Currently, we reuse WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE in the apply worker while sending data to the parallel apply worker via a shared memory queue. This is not appropriate as one won't be able to distinguish whether the worker is waiting for sending data or for the state change. To patch instead uses the wait event WAIT_EVENT_MQ_SEND which has been already used in blocking mode while sending data via a shared memory queue. Author: Hou Zhijie Reviewed-by: Kuroda Hayato, Amit Kapila Discussion: https://postgr.es/m/OS0PR01MB57161C680B22E4C591628EE994DA9@OS0PR01MB5716.jpnprd01.prod.outlook.com
* More refactoring of heapgettup() and heapgettup_pagemode()David Rowley2023-02-07
| | | | | | | | | | | | Here we further simplify the code in heapgettup() and heapgettup_pagemode() to make better use of the helper functions added in the previous recent refactors in this area. In passing, remove an unneeded cast added in 8ca6d49f6. Author: Melanie Plageman Reviewed-by: Andres Freund, David Rowley Discussion: https://postgr.es/m/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com
* Include values of A_Const nodes in query jumblingMichael Paquier2023-02-07
| | | | | | | | | | | | | | | | | | | | | | | Like the implementation for node copy, write and read, this node requires a custom implementation so as the query jumbling is able to consider the correct value assigned to it, depending on its type (int, float, bool, string, bitstring). Based on a dump of pg_stat_statements from the regression database, this would confuse the query jumbling of the following queries: - SET. - COPY TO with SELECT queries. - START TRANSACTION with different isolation levels. - ALTER TABLE with default expressions. - CREATE TABLE with partition bounds. Note that there may be a long-term argument in tracking the location of such nodes so as query strings holding such nodes could be normalized, but this is left as a separate discussion. Oversight in 3db72eb. Discussion: https://postgr.es/m/Y9+HuYslMAP6yyPb@paquier.xyz
* Fix more outdated commentsPeter Eisentraut2023-02-06
| | | | | Same as in f5da3d8 but for write_relcache_init_file(), the comments had gotten a bit wrong due to code added over time.
* Fix up outdated commentsPeter Eisentraut2023-02-06
| | | | | | The existing comments in load_relcache_init_file() were not flexible when new entries were added at the end, so they ended up a bit wrong. Simplify the comments to avoid this issue.
* Fix up join removal's interaction with PlaceHolderVars.Tom Lane2023-02-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The portion of join_is_removable() that checks PlaceHolderVars can be made a little more accurate and intelligible than it was. The key point is that we can allow join removal even if a PHV mentions the target rel in ph_eval_at, if that mention was only added as a consequence of forcing the PHV up to a join level that's at/above the outer join we're trying to get rid of. We can check that by testing for the OJ's relid appearing in ph_eval_at, indicating that it's supposed to be evaluated after the outer join, plus the existing test that the contained expression doesn't actually mention the target rel. While here, add an explicit check that there'll be something left in ph_eval_at after we remove the target rel and OJ relid. There is an Assert later on about that, and I'm not too sure that the case could happen for a PHV satisfying the other constraints, but let's just check. (There was previously a bms_is_subset test that meant to cover this risk, but it's broken now because it doesn't account for the fact that we'll also remove the OJ relid.) The real reason for revisiting this code though is that the Assert I left behind in 8538519db turns out to be easily reachable, because if a PHV of this sort appears in an upper-level qual clause then that clause's clause_relids will include the PHV's ph_eval_at relids. This is a mirage though: we have or soon will remove these relids from the PHV's ph_eval_at, and therefore they no longer belong in qual clauses' clause_relids either. Remove that Assert in join_is_removable, and replace the similar one in remove_rel_from_query with code to remove the deleted relids from clause_relids. Per bug #17773 from Robins Tharakan. Discussion: https://postgr.es/m/17773-a592e6cedbc7bac5@postgresql.org
* Disable STARTUP_PROGRESS_TIMEOUT in standby mode.Robert Haas2023-02-06
| | | | | | | | | | | | | In standby mode, we don't actually report progress of recovery, but up until now, startup_progress_timeout_handler() nevertheless got called every log_startup_progress_interval seconds. That's an unnecessary expense, so avoid it. Report by Thomas Munro. Patch by Bharath Rupireddy, reviewed by Simon Riggs, Thomas Munro, and me. Back-patch to v15, where the problem was introduced. Discussion: https://www.postgresql.org/message-id/CA%2BhUKGKCHSffAj8zZJKJvNX7ygnQFxVD6wm1d-2j3fVw%2BMafPQ%40mail.gmail.com
* Remove useless casts to (void *) in hash_search() callsPeter Eisentraut2023-02-06
| | | | | | | | | | | Some of these appear to be leftovers from when hash_search() took a char * argument (changed in 5999e78fc45dcb91784b64b6e9ae43f4e4f68ca2). Since after this there is some more horizontal space available, do some light reformatting where suitable. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/fd9adf5d-b1aa-e82f-e4c7-263c30145807%40enterprisedb.com
* Extend check_GUC_init() with checks on flag combinations when loading GUCsMichael Paquier2023-02-06
| | | | | | | | | | | | | | | | | | | | | | | | This extends the work begun by a73952b, with the addition of a GUC check for flag combinations in check_GUC_init(), making sure that anything defined with GUC_NO_SHOW_ALL also includes GUC_NOT_IN_SAMPLE, as first step. There has never been any GUCs of this kind in the core code, and this combination makes little sense as a parameter marked as not fit for SHOW ALL should not be hidden in postgresql.conf.sample. Note that GUCs marked with GUC_NO_SHOW_ALL are not listed under pg_settings or SHOW ALL (still they can be queried individually), making them unfit for checks via SQL queries in the regression tests that do a full scan of the parameters available. The SQL tests are still a bit incorrect about that, and will be cleaned up in a separate commit. We have also discussed the possibility to extend the SQL functions for GUCs so as they could show more information about parameters defined with GUC_NO_SHOW_ALL, though it has been concluded that this is not worth the extra complication in the long run, an enforced policy at initialization time being enough to do the same job. Per discussion with Nitin Jadhav and Tom Lane. Discussion: https://postgr.es/m/CAMm1aWaYe0muu3ABo7iSAgK+OWDS9yNe8GGRYnCyeEpScYKa+g@mail.gmail.com
* Add support for progress reporting to pg_verifybackupMichael Paquier2023-02-06
| | | | | | | | | | | | | | | | | | This adds a new option to pg_verifybackup called -P/--progress, showing every second some information about the progress of the checksum verification based on the data of a backup manifest. Similarly to what is done for pg_rewind and pg_basebackup, the information printed in the progress report consists of the current amount of data computed and the total amount of data that will be computed. Note that files found with an incorrect size do not have their checksum verified, hence their size is not appended to the total amount of data estimated during the first scan of the manifest data (such incorrect sizes could be overly high, for one, falsifying the progress report). Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoC5+JOgMd4o3z_oxw0f8JDSsCYY7zSbhe-O9x7f33rw_A@mail.gmail.com
* Properly NULL-terminate GSS receive buffer on error packet receptionMichael Paquier2023-02-06
| | | | | | | | | | | | | | | | | | | | | pqsecure_open_gss() includes a code path handling error messages with v2-style protocol messages coming from the server. The client-side buffer holding the error message does not force a NULL-termination, with the data of the server getting copied to the errorMessage of the connection. Hence, it would be possible for a server to send an unterminated string and copy arbitrary bytes in the buffer receiving the error message in the client, opening the door to a crash or even data exposure. As at this stage of the authentication process the exchange has not been completed yet, this could be abused by an attacker without Kerberos credentials. Clients that have a valid kerberos cache are vulnerable as libpq opportunistically requests for it except if gssencmode is disabled. Author: Jacob Champion Backpatch-through: 12 Security: CVE-2022-41862
* Revert refactoring of restore command code to shell_restore.cMichael Paquier2023-02-06
| | | | | | | | | | | | | | | | | | | | | This reverts commits 24c35ec and 57169ad. PreRestoreCommand() and PostRestoreCommand() need to be put closer to the system() call calling a restore_command, as they enable in_restore_command for the startup process which would in turn trigger an immediate proc_exit() in the SIGTERM handler. Perhaps we could get rid of this behavior entirely, but 24c35ec has made the window where the flag is enabled much larger than it was, and any Postgres-like actions (palloc, etc.) taken by code paths while the flag is enabled could lead to more severe issues in the shutdown processing. Note that curculio has showed that there are much more problems in this area, unrelated to this change, actually, hence the issues related to that had better be addressed first. Keeping the code of HEAD in line with the stable branches should make that a bit easier. Per discussion with Andres Freund and Nathan Bossart. Discussion: https://postgr.es/m/Y979NR3U5VnWrTwB@paquier.xyz
* Fix over-optimistic updating of info about commutable outer joins.Tom Lane2023-02-05
| | | | | | | | | | | | | | | | | | | | | | | | | | make_outerjoininfo was set up to update SpecialJoinInfo's commute_below, commute_above_l, commute_above_r fields as soon as it found a pair of outer joins that look like they can commute. However, this decision could be negated later in the same loop due to finding an intermediate outer join that prevents commutation. That left us with commute_xxx fields that were contradictory to the join order restrictions expressed in min_lefthand/min_righthand. The latter fields would keep us from actually choosing a bad join order; but the inconsistent commute_xxx fields could bollix details such as the varnullingrels values created for intermediate join relation targetlists, ending in an assertion failure in setrefs.c. To fix, wait till the end of make_outerjoininfo where we have accurate values for min_lefthand/min_righthand, and then insert only relids not present in those sets into the commute_xxx fields. Per SQLSmith testing by Robins Tharakan. Note that while Robins bisected the failure to commit b448f1c8d, it's really the fault of 2489d76c4. The outerjoin_delayed logic removed in the later commit was keeping us from deciding that troublesome join pairs commute, at least in the specific example seen here. Discussion: https://postgr.es/m/CAEP4nAyAORgE8K_RHSmvWbE9UaChhjbEL1RrDU3neePwwRUB=A@mail.gmail.com
* Fix thinko in qual distribution.Tom Lane2023-02-04
| | | | | | | | | | | | | | | | deconstruct_distribute tweaks the outer join scope (ojscope) it passes to distribute_qual_to_rels when considering an outer join qual that's above potentially-commutable outer joins. However, if the current join is *not* potentially commutable, we shouldn't do that. The argument that distribute_qual_to_rels will not do something wrong with the bogus ojscope falls flat if we don't pass it non-null postponed_oj_qual_list. Moreover, there's no need to play games in this case since we aren't going to commute anything. Per SQLSmith testing by Robins Tharakan. Discussion: https://postgr.es/m/CAEP4nAw74k4b-=93gmfCNX3MOY3y4uPxqbk_MnCVEpdsqHJVsg@mail.gmail.com
* Fix thinko in outer-join removal.Tom Lane2023-02-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we have a RestrictInfo that mentions both the removal-candidate relation and the outer join's relid, then that is a pushed-down condition not a join condition, so it should be grounds for deciding that we can't remove the outer join. In commit 2489d76c4, I'd blindly included the OJ's relid into "joinrelids" as per the new standard convention, but the checks of attr_needed and ph_needed should only allow the join's input rels to be mentioned. Having done that, the check for references in pushed-down quals a few lines further down should be redundant. I left it in place as an Assert, though. While researching this I happened across a couple of comments that worried about the effects of update_placeholder_eval_levels. That's gone as of b448f1c8d, so we can remove some worry. Per bug #17769 from Robins Tharakan. The submitted test case triggers this more or less accidentally because we flatten out a LATERAL sub-select after we've done join strength reduction; if we did that in the other order, this problem would be masked because the outer join would get simplified to an inner join. To ensure that the committed test case will continue to test what it means to even if we make that happen someday, use a test clause involving COALESCE(), which will prevent us from using it to do join strength reduction. Patch by me, but thanks to Richard Guo for initial investigation. Discussion: https://postgr.es/m/17769-e4f7a5c9d84a80a7@postgresql.org
* Rethink treatment of "postponed" quals in deconstruct_jointree().Tom Lane2023-02-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | After pulling up LATERAL subqueries, we may have qual clauses that refer to relations outside their syntactic scope. Before doing any such pullup, prepjointree.c checks to make sure that it wouldn't create a semantically-invalid situation; but we leave it to deconstruct_jointree() to actually move these quals up the join tree to a place where they can be evaluated. In commit 2489d76c4, I (tgl) refactored deconstruct_jointree() in a way that caused assertion failures while moving such quals, because the new logic failed to distinguish "this jointree node is a parent of the source one" from "this jointree node is processed after the source one in depth-first order". Fix this, and at the same time reduce the overhead a bit, by getting rid of the common PostponedQual list and instead making each JoinTreeItem contain a list of quals that needed to be postponed to its level. We can help distribute_qual_to_rels find the appropriate JoinTreeItem efficiently by adding parent-item links to the JoinTreeItem data structure. This ends up being the same number of relid subset checks as the original (pre-bug) logic, but less list manipulation is required during multi-level postponements. Richard Guo and Tom Lane, per bug #17768 from Robins Tharakan. Discussion: https://postgr.es/m/17768-5ac8730ece54478f@postgresql.org
* Allow underscores in integer and numeric constants.Dean Rasheed2023-02-04
| | | | | | | | | | | | | | | | | | | This allows underscores to be used in integer and numeric literals, and their corresponding type input functions, for visual grouping. For example: 1_500_000_000 3.14159_26535_89793 0xffff_ffff 0b_1001_0001 A single underscore is allowed between any 2 digits, or immediately after the base prefix indicator of non-decimal integers, per SQL:202x draft. Peter Eisentraut and Dean Rasheed Discussion: https://postgr.es/m/84aae844-dc55-a4be-86d9-4f0fa405cc97%40enterprisedb.com
* Remove unused code related to unknown typePeter Eisentraut2023-02-04
| | | | | | | | These are leftovers obsoleted by cfd9be939e9c516243c5b6a49ad1e1a9a38f1052. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/e7887965-9e70-fd01-c2d1-5bc02f9169aa%40enterprisedb.com
* Make int64_div_fast_to_numeric() more robust.Dean Rasheed2023-02-03
| | | | | | | | | | | | | | | | | | The prior coding of int64_div_fast_to_numeric() had a number of bugs that would cause it to fail under different circumstances, such as with log10val2 <= 0, or log10val2 a multiple of 4, or in the "slow" numeric path with log10val2 >= 10. None of those could be triggered by any of our current code, which only uses log10val2 = 3 or 6. However, they made it a hazard for any future code that might use it. Also, since this is exported by numeric.c, users writing their own C code might choose to use it. Therefore fix, and back-patch to v14, where it was introduced. Dean Rasheed, reviewed by Tom Lane. Discussion: https://postgr.es/m/CAEZATCW8gXgW0tgPxPgHDPhVX71%2BSWFRkhnXy%2BTfGDsKLepu2g%40mail.gmail.com
* Reduce code duplication between heapgettup and heapgettup_pagemodeDavid Rowley2023-02-03
| | | | | | | | | | The code to get the next block number was exactly the same between these two functions, so let's just put it into a helper function and call that from both locations. Author: Melanie Plageman Reviewed-by: Andres Freund, David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
* Optimize the origin drop functionality.Amit Kapila2023-02-03
| | | | | | | | | | | | | | To interlock against concurrent drops, we use to hold ExclusiveLock on pg_replication_origin till xact commit. This blocks even concurrent drops of different origins by tablesync workers. So, instead, lock the specific origin to interlock against concurrent drops. This reduces the test time variability in src/test/subscription where multiple tables are being synced. Author: Vignesh C Reviewed-by: Hou Zhijie, Amit Kapila Discussion: https://postgr.es/m/1412708.1674417574@sss.pgh.pa.us
* Add helper functions to simplify heapgettup codeDavid Rowley2023-02-03
| | | | | | | | | Here we add heapgettup_start_page() and heapgettup_continue_page() to simplify the code in the heapgettup() function. Author: Melanie Plageman Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
* Further refactor of heapgettup and heapgettup_pagemodeDavid Rowley2023-02-03
| | | | | | | | | | | | | | | Backward and forward scans share much of the same page acquisition code. Here we consolidate that code to reduce some duplication. Additionally, add a new rs_coffset field to HeapScanDescData to track the offset of the current tuple. The new field fits nicely into the padding between a bool and BlockNumber field and saves having to look at the last returned tuple to figure out which offset we should be looking at for the current tuple. Author: Melanie Plageman Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
* Retire PG_SETMASK() macro.Thomas Munro2023-02-03
| | | | | | | | | | | | | | | | In the 90s we needed to deal with computers that still had the pre-standard signal masking APIs. That hasn't been relevant for a very long time on Unix systems, and c94ae9d8 got rid of a remaining dependency in our Windows porting code. PG_SETMASK didn't expose save/restore functionality, so we'd already started using sigprocmask() directly in places, creating the visual distraction of having two ways to spell it. It's not part of the API that extensions are expected to be using (but if they are, the change will be trivial). It seems like a good time to drop the old macro and just call the standard POSIX function. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BKfQgrhHP2DLTohX1WwubaCBHmTzGnAEDPZ-Gug-Xskg%40mail.gmail.com
* Document installing perltidy with cpanmAndrew Dunstan2023-02-02
| | | | | | | Installing with plain cpan failed for me recently, as the archive it searched has been purged of old releases. However, you can give cpanm a complete URL to the exact version you want to install, so document using that.
* Clarify the choice of rscale in numeric_sqrt().Dean Rasheed2023-02-02
| | | | | | | | | | | | | Improve the comment explaining the choice of rscale in numeric_sqrt(), and ensure that the code works consistently when other values of NBASE/DEC_DIGITS are used. Note that, in practice, we always expect DEC_DIGITS == 4, and this does not change the computation in that case. Joel Jacobson and Dean Rasheed Discussion: https://postgr.es/m/06712c29-98e9-43b3-98da-f234d81c6e49%40app.fastmail.com
* Ensure that numeric.c compiles with other NBASE values.Dean Rasheed2023-02-02
| | | | | | | | | | As noted in the comments, support for different NBASE values is really only of historical interest, but as long as we're keeping it, we might as well make sure that it compiles. Joel Jacobson Discussion: https://postgr.es/m/06712c29-98e9-43b3-98da-f234d81c6e49%40app.fastmail.com
* Allow the logical_replication_mode to be used on the subscriber.Amit Kapila2023-02-02
| | | | | | | | | | | | | | | | | Extend the existing developer option 'logical_replication_mode' to help test the parallel apply of large transactions on the subscriber. When set to 'buffered', the leader sends changes to parallel apply workers via a shared memory queue. When set to 'immediate', the leader serializes all changes to files and notifies the parallel apply workers to read and apply them at the end of the transaction. This helps in adding tests to cover the serialization code path in parallel streaming mode. Author: Hou Zhijie Reviewed-by: Peter Smith, Kuroda Hayato, Sawada Masahiko, Amit Kapila Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com
* Refactor heapam.c adding heapgettup_initial_block functionDavid Rowley2023-02-02
| | | | | | | | | | Here we adjust heapgettup() and heapgettup_pagemode() to move the code that fetches the first block number to scan out into a helper function. This removes some code duplication. Author: Melanie Plageman Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
* meson: Fix typo in pkgconfig generationPeter Eisentraut2023-02-01
| | | | | Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/07b37c70-349a-8fcd-bcc9-6c3ce0f6c2a4%40enterprisedb.com
* Simplify main waiting loop of the archiver processMichael Paquier2023-02-01
| | | | | | | | | | | | | As coded, the timeout given to WaitLatch() was always equal to PGARCH_AUTOWAKE_INTERVAL, as time() was called two times repeatedly. This simplification could have been done in d75288f. While on it, this adjusts a comment in pgarch.c to describe the archiver in a more neutral way. Author: Sravan Kumar, Nathan Bossart Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/CA+=NbjjqYE9-Lnw7H7DAiS5jebmoMikwZQb_sBP7kgBCn9q6Hg@mail.gmail.com
* Try to fix pg_upgrade test on Windows, again.Thomas Munro2023-02-01
| | | | | | | | | | Further to commit 54e72b66e, if rmtree() fails while cleaning up in pg_upgrade, try again. This gives our Windows unlink() wrapper a chance to reach its wait-for-the-other-process-to-go-away logic, if the first go around initiated the unlink of a file that a concurrently exiting program still has open. Discussion: https://postgr.es/m/CA%2BhUKGKCVy2%3Do%3Dd8c2Va6a_3Rpf_KkhUitkWCZ3hzuO2VwLMXA%40mail.gmail.com
* Update time zone data files to tzdata release 2022g.Tom Lane2023-01-31
| | | | | | | DST law changes in Greenland and Mexico. Notably, a new timezone America/Ciudad_Juarez has been split off from America/Ojinaga. Historical corrections for northern Canada, Colombia, and Singapore.
* Remove dead NoMovementScanDirection codeDavid Rowley2023-02-01
| | | | | | | | | | | | | | | | | | | | | | Here remove some dead code from heapgettup() and heapgettup_pagemode() which was trying to support NoMovementScanDirection scans. This code can never be reached as standard_ExecutorRun() never calls ExecutePlan with NoMovementScanDirection. Additionally, plans which were scanning an unordered index would use NoMovementScanDirection rather than ForwardScanDirection. There was no real need for this, so here we adjust this so we use ForwardScanDirection for unordered index scans. A comment in pathnodes.h claimed that NoMovementScanDirection was used for PathKey reasons, but if that was true, it no longer is, per code in build_index_paths(). This does change the non-text format of the EXPLAIN output so that unordered index scans now have a "Forward" scan direction rather than "NoMovement". The text format of EXPLAIN has not changed. Author: Melanie Plageman Reviewed-by: Tom Lane, David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
* Remove over-optimistic Assert.Tom Lane2023-01-31
| | | | | | | | | | | | | | | | | | | In commit 2489d76c4, I'd thought it'd be safe to assert that a PlaceHolderVar appearing in a scan-level expression has empty nullingrels. However this is not so, as when we determine that a join relation is certainly empty we'll put its targetlist into a Result-with-constant-false-qual node, and nothing is done to adjust the nullingrels of the Vars or PHVs therein. (Arguably, a Result used in this way isn't really a scan-level node, but it certainly isn't an upper node either ...) It's not clear this is worth any close analysis, so let's just take out the faulty Assert. Per report from Robins Tharakan. I added a test case based on his example, just in case somebody tries to tighten this up. Discussion: https://postgr.es/m/CAEP4nAz7Enq3+DEthGG7j27DpuwSRZnW0Nh6jtNh75yErQ_nbA@mail.gmail.com
* Generate code for query jumbling through gen_node_support.plMichael Paquier2023-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit changes the query jumbling code in queryjumblefuncs.c to be generated automatically based on the information of the nodes in the headers of src/include/nodes/ by using gen_node_support.pl. This approach offers many advantages: - Support for query jumbling for all the utility statements, based on the state of their parsed Nodes and not only their query string. This will greatly ease the switch to normalize the information of some DDLs, like SET or CALL for example (this is left unchanged and should be part of a separate discussion). With this feature, the number of entries stored for utilities in pg_stat_statements is reduced (for example now "CHECKPOINT" and "checkpoint" mean the same thing with the same query ID). - Documentation of query jumbling directly in the structure definition of the nodes. Since this code has been introduced in pg_stat_statements and then moved to code, the reasons behind the choices of what should be included in the jumble are rather sparse. Note that some explanation is added for the most relevant parts, as a start. - Overall code reduction and more consistency with the other parts generating read, write and copy depending on the nodes. The query jumbling is controlled by a couple of new node attributes, documented in nodes/nodes.h: - custom_query_jumble, to mark a Node as having a custom implementation. - no_query_jumble, to ignore entirely a Node. - query_jumble_ignore, to ignore a field in a Node. - query_jumble_location, to mark a location in a Node, for normalization. This can apply only to int fields, with "location" in their name (only Const as of this commit). There should be no compatibility impact on pg_stat_statements, as the new code applies the jumbling to the same fields for each node (its regression tests have no modification, for one). Some benchmark of the query jumbling between HEAD and this commit for SELECT and DMLs has proved that this new code does not cause a performance regression, with computation times close for both methods. For utility queries, the new method is slower than the previous method of calculating a hash of the query string, though we are talking about extra ns-level changes based on what I measured, which is unnoticeable even for OLTP workloads as a query ID is calculated once per query post-parse analysis. Author: Michael Paquier Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/Y5BHOUhX3zTH/ig6@paquier.xyz
* Remove recovery test 011_crash_recovery.plMichael Paquier2023-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This test has been added as of 857ee8e that has introduced the SQL function txid_status(), with the purpose of checking that a transaction ID still in-progress during a crash is correctly marked as aborted after recovery finishes. This test is unstable, and some configuration scenarios may that easier to reproduce (wal_level=minimal, wal_compression=on) because the WAL holding the information about the in-progress transaction ID may not have made it to disk yet, hence a post-crash recovery may cause the same XID to be reused, triggering a test failure. We have discussed a few approaches, like making this function force a WAL flush to make it reliable across crashes, but we don't want to pay a performance penalty in some scenarios, as well. The test could have been tweaked to enforce a checkpoint but that actually breaks the promise of the test to rely on a stable result of txid_status() after a crash. This issue has been reported a few times across the past years, with an original report from Kyotaro Horiguchi. The buildfarm machines tanager, hachi and gokiburi enable wal_compression, and fail on this test periodically. Discussion: https://postgr.es/m/3163112.1674762209@sss.pgh.pa.us Discussion: https://postgr.es/m/20210305.115011.558061052471425531.horikyota.ntt@gmail.com Backpatch-through: 11