aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* 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
* 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
* 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
* Invent "join domains" to replace the below_outer_join hack.Tom Lane2023-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | EquivalenceClasses are now understood as applying within a "join domain", which is a set of inner-joined relations (possibly underneath an outer join). We no longer need to treat an EC from below an outer join as a second-class citizen. I have hopes of eventually being able to treat outer-join clauses via EquivalenceClasses, by means of only applying deductions within the EC's join domain. There are still problems in the way of that, though, so for now the reconsider_outer_join_clause logic is still here. I haven't been able to get rid of RestrictInfo.is_pushed_down either, but I wonder if that could be recast using JoinDomains. I had to hack one test case in postgres_fdw.sql to make it still test what it was meant to, because postgres_fdw is inconsistent about how it deals with quals containing non-shippable expressions; see https://postgr.es/m/1691374.1671659838@sss.pgh.pa.us. That should be improved, but I don't think it's within the scope of this patch series. Patch by me; thanks to Richard Guo for review. Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
* Do assorted mop-up in the planner.Tom Lane2023-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove RestrictInfo.nullable_relids, along with a good deal of infrastructure that calculated it. One use-case for it was in join_clause_is_movable_to, but we can now replace that usage with a check to see if the clause's relids include any outer join that can null the target relation. The other use-case was in join_clause_is_movable_into, but that test can just be dropped entirely now that the clause's relids include outer joins. Furthermore, join_clause_is_movable_into should now be accurate enough that it will accept anything returned by generate_join_implied_equalities, so we can restore the Assert that was diked out in commit 95f4e59c3. Remove the outerjoin_delayed mechanism. We needed this before to prevent quals from getting evaluated below outer joins that should null some of their vars. Now that we consider varnullingrels while placing quals, that's taken care of automatically, so throw the whole thing away. Teach remove_useless_result_rtes to also remove useless FromExprs. Having done that, the delay_upper_joins flag serves no purpose any more and we can remove it, largely reverting 11086f2f2. Use constant TRUE for "dummy" clauses when throwing back outer joins. This improves on a hack I introduced in commit 6a6522529. If we have a left-join clause l.x = r.y, and a WHERE clause l.x = constant, we generate r.y = constant and then don't really have a need for the join clause. But we must throw the join clause back anyway after marking it redundant, so that the join search heuristics won't think this is a clauseless join and avoid it. That was a kluge introduced under time pressure, and after looking at it I thought of a better way: let's just introduce constant-TRUE "join clauses" instead, and get rid of them at the end. This improves the generated plans for such cases by not having to test a redundant join clause. We can also get rid of the ugly hack used to mark such clauses as redundant for selectivity estimation. Patch by me; thanks to Richard Guo for review. Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
* Make Vars be outer-join-aware.Tom Lane2023-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Traditionally we used the same Var struct to represent the value of a table column everywhere in parse and plan trees. This choice predates our support for SQL outer joins, and it's really a pretty bad idea with outer joins, because the Var's value can depend on where it is in the tree: it might go to NULL above an outer join. So expression nodes that are equal() per equalfuncs.c might not represent the same value, which is a huge correctness hazard for the planner. To improve this, decorate Var nodes with a bitmapset showing which outer joins (identified by RTE indexes) may have nulled them at the point in the parse tree where the Var appears. This allows us to trust that equal() Vars represent the same value. A certain amount of klugery is still needed to cope with cases where we re-order two outer joins, but it's possible to make it work without sacrificing that core principle. PlaceHolderVars receive similar decoration for the same reason. In the planner, we include these outer join bitmapsets into the relids that an expression is considered to depend on, and in consequence also add outer-join relids to the relids of join RelOptInfos. This allows us to correctly perceive whether an expression can be calculated above or below a particular outer join. This change affects FDWs that want to plan foreign joins. They *must* follow suit when labeling foreign joins in order to match with the core planner, but for many purposes (if postgres_fdw is any guide) they'd prefer to consider only base relations within the join. To support both requirements, redefine ForeignScan.fs_relids as base+OJ relids, and add a new field fs_base_relids that's set up by the core planner. Large though it is, this commit just does the minimum necessary to install the new mechanisms and get check-world passing again. Follow-up patches will perform some cleanup. (The README additions and comments mention some stuff that will appear in the follow-up.) Patch by me; thanks to Richard Guo for review. Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
* Ensure that MERGE recomputes GENERATED expressions properly.Dean Rasheed2023-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This fixes a bug that, under some circumstances, would cause MERGE to fail to properly recompute expressions for GENERATED STORED columns. Formerly, ExecInitModifyTable() did not call ExecInitStoredGenerated() for a MERGE command, which meant that the generated expressions information was not computed until later, when the first merge action was executed. However, if the first merge action to execute was an UPDATE, then ExecInitStoredGenerated() could decide to skip some some generated columns, if the columns on which they depended were not updated, which was a problem if the MERGE also contained an INSERT action, for which no generated columns should be skipped. So fix by having ExecInitModifyTable() call ExecInitStoredGenerated() for MERGE, and assume that it isn't safe to skip any generated columns in a MERGE. Possibly that could be relaxed, by allowing some generated columns to be skipped for a MERGE without an INSERT action, but it's not clear that it's worth the effort. Noticed while investigating bug #17759. Back-patch to v15, where MERGE was added. Dean Rasheed, reviewed by Tom Lane. Discussion: https://postgr.es/m/17759-e76d9bece1b5421c%40postgresql.org https://postgr.es/m/CAEZATCXb_ezoMCcL0tzKwRGA1x0oeE%3DawTaysRfTPq%2B3wNJn8g%40mail.gmail.com
* Rename GUC logical_decoding_mode to logical_replication_mode.Amit Kapila2023-01-30
| | | | | | | | | | | | | | | | | | | | | | | | Rename the developer option 'logical_decoding_mode' to the more flexible name 'logical_replication_mode' because doing so will make it easier to extend this option in the future to help test other areas of logical replication. Currently, it is used on the publisher side to allow streaming or serializing each change in logical decoding. In the upcoming patch, we are planning to use it on the subscriber. On the subscriber, it will allow serializing the changes to file and notifies the parallel apply workers to read and apply them at the end of the transaction. We discussed exposing this parameter as a subscription option but it did not seem advisable since it is primarily used for testing/debugging and there is no other such parameter. We also discussed having separate GUCs for publisher and subscriber but for current testing/debugging requirements, one GUC is sufficient. Author: Hou Zhijie Reviewed-by: Peter Smith, Kuroda Hayato, Sawada Masahiko, Amit Kapila Discussion: https://postgr.es/m/CAD21AoAy2c=Mx=FTCs+EwUsf2kQL5MmU3N18X84k0EmCXntK4g@mail.gmail.com Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com
* Remove unneeded volatile qualifiers from postmaster.c.Thomas Munro2023-01-28
| | | | | | | | | | Several flags were marked volatile and in some cases used sig_atomic_t because they were accessed from signal handlers. After commit 7389aad6, we can just use unqualified bool. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA%2BhUKGLMoeZNZY6gYdLUQmuoW_a8bKyLvtuZkd_zHcGVOfDzBA%40mail.gmail.com
* Minor GUC code refactoring.Tom Lane2023-01-27
| | | | | | | | | | | | | | | | | | | | | Split out "ConfigOptionIsVisible" to perform the privilege check for GUC_SUPERUSER_ONLY GUCs (which these days can also be read by pg_read_all_settings role members), and move the should-we-show-it checks from GetConfigOptionValues to its sole caller. This commit also removes get_explain_guc_options's check of GUC_NO_SHOW_ALL, which seems to have got cargo-culted in there. While there's no obvious use-case for marking a GUC both GUC_EXPLAIN and GUC_NO_SHOW_ALL, if it were set up that way one would expect EXPLAIN to show it --- if that's not what you want, then don't set GUC_EXPLAIN. In passing, simplify the loop logic in show_all_settings. Nitin Jadhav, Bharath Rupireddy, Tom Lane Discussion: https://postgr.es/m/CAMm1aWYgfekpRK-Jz5=pM_bV+Om=ktGq1vxTZ_dr1Z6MV-qokA@mail.gmail.com
* Teach planner about more monotonic window functionsDavid Rowley2023-01-27
| | | | | | | | | | | | | 9d9c02ccd introduced runConditions for window functions to allow monotonic window function evaluation to be made more efficient when the window function value went beyond some value that it would never go back from due to its monotonic nature. That commit added prosupport functions to inform the planner that row_number(), rank(), dense_rank() and some forms of count(*) were monotonic. Here we add support for ntile(), cume_dist() and percent_rank(). Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/CAApHDvqR+VqB8s+xR-24bzJbU8xyFrBszJ17qKgECf7cWxLCaA@mail.gmail.com
* Improve TimestampDifferenceMilliseconds to cope with overflow sanely.Tom Lane2023-01-26
| | | | | | | | | | | | | | | | | | | | | | | | | We'd like to use TimestampDifferenceMilliseconds with the stop_time possibly being TIMESTAMP_INFINITY, but up to now it's disclaimed responsibility for overflow cases. Define it to clamp its output to the range [0, INT_MAX], handling overflow correctly. (INT_MAX rather than LONG_MAX seems appropriate, because the function is already described as being intended for calculating wait times for WaitLatch et al, and that infrastructure only handles waits up to INT_MAX. Also, this choice gets rid of cross-platform behavioral differences.) Having done that, we can replace some ad-hoc code in walreceiver.c with a simple call to TimestampDifferenceMilliseconds. While at it, fix some buglets in existing callers of TimestampDifferenceMilliseconds: basebackup_copy.c had not read the memo about TimestampDifferenceMilliseconds never returning a negative value, and postmaster.c had not read the memo about Min() and Max() being macros with multiple-evaluation hazards. Neither of these quite seem worth back-patching. Patch by me; thanks to Nathan Bossart for review. Discussion: https://postgr.es/m/3126727.1674759248@sss.pgh.pa.us
* Code review for commit 05a7be935.Tom Lane2023-01-26
| | | | | | | | | | | | | | | | | | | | | | | | Avoid having walreceiver code know explicitly about the precision and underlying datatype of TimestampTz. (There is still one calculation that knows that, which should be replaced with use of TimestampDifferenceMilliseconds; but we need to figure out what to do about overflow cases first.) In support of this, provide a TimestampTzPlusSeconds macro, as well as TIMESTAMP_INFINITY and TIMESTAMP_MINUS_INFINITY macros. (We could have used the existing DT_NOEND and DT_NOBEGIN symbols, but I judged those too opaque and confusing.) Move GetCurrentTimestamp calls so that it's more obvious that we are not using stale values of "now" anyplace. This doesn't result in net more calls, and might indeed make for net fewer. Avoid having a dummy value in the WalRcvWakeupReason enum, so that we can hope for the compiler to catch overlooked switch cases. Nathan Bossart and Tom Lane Discussion: https://postgr.es/m/20230125235004.GA1327755@nathanxps13
* Don't install postmaster symlink anymorePeter Eisentraut2023-01-26
| | | | | | | | | | This has long been deprecated. Some of the build systems didn't even install it. Also remove man page. Reviewed-by: Karl O. Pinc <kop@karlpinc.com> Discussion: https://www.postgresql.org/message-id/flat/ece84b69-8f94-8b88-925f-64207cb3a2f0@enterprisedb.com
* Revert "Add eager and lazy freezing strategies to VACUUM."Peter Geoghegan2023-01-25
| | | | | | | | | This reverts commit 4d417992613949af35530b4e8e83670c4e67e1b2. Broad concerns about regressions caused by eager freezing strategy have been raised. Whether or not these concerns can be worked through in any time frame is far from certain. Discussion: https://postgr.es/m/20230126004347.gepcmyenk2csxrri@awork3.anarazel.de
* Make auto_explain print the query identifier in verbose modeMichael Paquier2023-01-26
| | | | | | | | | | | | | | | | | | | | | | | When auto_explain.log_verbose is on, auto_explain should print in the logs plans equivalent to the EXPLAIN (VERBOSE). However, when compute_query_id is on, query identifiers were not showing up, being only handled by EXPLAIN (VERBOSE). This brings auto_explain on par with EXPLAIN regarding that. Note that like EXPLAIN, auto_explain does not show the query identifier when compute_query_id=regress. The change is done so as the choice of printing the query identifier is done in ExplainPrintPlan() rather than in ExplainOnePlan(), to avoid a duplication of the logic dealing with the query ID. auto_explain is the only in-core caller of ExplainPrintPlan(). While looking at the area, I have noticed that more consolidation between EXPLAIN and auto_explain would be in order for the logging of the plan duration and the buffer usage. This refactoring is left as a future change. Author: Atsushi Torikoshi Reviewed-by: Justin Pryzby, Julien Rouhaud Discussion: https://postgr.es/m/1ea21936981f161bccfce05765c03bee@oss.nttdata.com
* Fix rare sharedtuplestore.c corruption.Thomas Munro2023-01-26
| | | | | | | | | | | | | | | | | If the final chunk of an oversized tuple being written out to disk was exactly 32760 bytes, it would be corrupted due to a fencepost bug. Bug #17619. Back-patch to 11 where the code arrived. While testing that (see test module in archives), I (tmunro) noticed that the per-participant page counter was not initialized to zero as it should have been; that wasn't a live bug when it was written since DSM memory was originally always zeroed, but since 14 min_dynamic_shared_memory might be configured and it supplies non-zeroed memory, so that is also fixed here. Author: Dmitry Astapov <dastapov@gmail.com> Discussion: https://postgr.es/m/17619-0de62ceda812b8b5%40postgresql.org
* Add eager and lazy freezing strategies to VACUUM.Peter Geoghegan2023-01-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Eager freezing strategy avoids large build-ups of all-visible pages. It makes VACUUM trigger page-level freezing whenever doing so will enable the page to become all-frozen in the visibility map. This is useful for tables that experience continual growth, particularly strict append-only tables such as pgbench's history table. Eager freezing significantly improves performance stability by spreading out the cost of freezing over time, rather than doing most freezing during aggressive VACUUMs. It complements the insert autovacuum mechanism added by commit b07642db. VACUUM determines its freezing strategy based on the value of the new vacuum_freeze_strategy_threshold GUC (or reloption) with logged tables. Tables that exceed the size threshold use the eager freezing strategy. Unlogged tables and temp tables always use eager freezing strategy, since the added cost is negligible there. Non-permanent relations won't incur any extra overhead in WAL written (for the obvious reason), nor in pages dirtied (since any extra freezing will only take place on pages whose PD_ALL_VISIBLE bit needed to be set either way). VACUUM uses lazy freezing strategy for logged tables that fall under the GUC size threshold. Page-level freezing triggers based on the criteria established in commit 1de58df4, which added basic page-level freezing. Eager freezing is strictly more aggressive than lazy freezing. Settings like vacuum_freeze_min_age still get applied in just the same way in every VACUUM, independent of the strategy in use. The only mechanical difference between eager and lazy freezing strategies is that only the former applies its own additional criteria to trigger freezing pages. Note that even lazy freezing strategy will trigger freezing whenever a page happens to have required that an FPI be written during pruning, provided that the page will thereby become all-frozen in the visibility map afterwards (due to the FPI optimization from commit 1de58df4). The vacuum_freeze_strategy_threshold default setting is 4GB. This is a relatively low setting that prioritizes performance stability. It will be reviewed at the end of the Postgres 16 beta period. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Jeff Davis <pgsql@j-davis.com> Reviewed-By: Andres Freund <andres@anarazel.de> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-WzkFok_6EAHuK39GaW4FjEFQsY=3J0AAd6FXk93u-Xq3Fg@mail.gmail.com
* Avoid type cheats for invalid dsa_handles and dshash_table_handles.Tom Lane2023-01-25
| | | | | | | | | | | | | | Invent separate macros for "invalid" values of these types, so that we needn't embed knowledge of their representations into calling code. These are all zeroes anyway ATM, so this is not fixing any live bug, but it makes the code cleaner and more future-proof. I (tgl) also chose to move DSM_HANDLE_INVALID into dsm_impl.h, since it seems like it should live beside the typedef for dsm_handle. Hou Zhijie, Nathan Bossart, Kyotaro Horiguchi, Tom Lane Discussion: https://postgr.es/m/OS0PR01MB5716860B1454C34E5B179B6694C99@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Process pending postmaster work before connections.Thomas Munro2023-01-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Modify the new event loop code from commit 7389aad6 so that it checks for work requested by signal handlers even if it doesn't see a latch event yet. This gives priority to shutdown and reload requests where the latch will be reported later in the event array, or in a later call to WaitEventSetWait(), due to scheduling details. In particular, this guarantees that a SIGHUP-then-connect sequence (as seen in authentication tests) causes the postmaster to process the reload before accepting the connection. If the WaitEventSetWait() call saw the socket as ready, and the reload signal was generated before the connection, then the latest time the signal handler should be able to run is after poll/epoll_wait/kevent returns but before we check the pending_pm_reload_request flag. While here, also shift the handling of child exit below reload requests, per Tom Lane's observation that that might start new processes, so we should make sure we pick up new settings first. This probably explains the one-off failure of build farm animal malleefowl. Reported-by: Hou Zhijie <houzj.fnst@fujitsu.com> Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/OS0PR01MB57163D3BF2AB42ECAA94E5C394C29%40OS0PR01MB5716.jpnprd01.prod.outlook.com
* Update more obsolete multixact.c comments.Peter Geoghegan2023-01-24
| | | | | | | | | | | | Update some remaining comments in multixact.c that still described SLRU truncation as happening in the checkpointer, rather than during VACUUM. Follow-up to commit 5212d447. Shi yu, with tweaks by me. Author: Shi yu <shiy.fnst@fujitsu.com> Discussion: https://postgr.es/m/OSZPR01MB631066BF246F8F74E83222FCFDC69@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Adjust interaction of CREATEROLE with role properties.Robert Haas2023-01-24
| | | | | | | | | | | | | | | | | | | | | | | | Previously, a CREATEROLE user without SUPERUSER could not alter REPLICATION users in any way, and could not set the BYPASSRLS attribute. However, they could manipulate the CREATEDB property even if they themselves did not possess it. With this change, a CREATEROLE user without SUPERUSER can set or clear the REPLICATION, BYPASSRLS, or CREATEDB property on a new role or a role that they have rights to manage if and only if that property is set for their own role. This implements the standard idea that you can't give permissions you don't have (but you can give the ones you do have). We might in the future want to provide more powerful ways to constrain what a CREATEROLE user can do - for example, to limit whether CONNECTION LIMIT can be set or the values to which it can be set - but that is left as future work. Patch by me, reviewed by Nathan Bossart, Tushar Ahuja, and Neha Sharma. Discussion: http://postgr.es/m/CA+TgmobX=LHg_J5aT=0pi9gJy=JdtrUVGAu0zhr-i5v5nNbJDg@mail.gmail.com
* Fix the Drop Database hang.Amit Kapila2023-01-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The drop database command waits for the logical replication sync worker to accept ProcSignalBarrier and the worker's slot creation waits for the drop database to finish which leads to a deadlock. This happens because the tablesync worker holds interrupts while creating a slot. We prevent cancel/die interrupts while creating a slot in the table sync worker because it is possible that before the server finishes this command, a concurrent drop subscription happens which would complete without removing this slot and that leads to the slot existing until the end of walsender. However, the slot will eventually get dropped at the walsender exit time, so there is no danger of the dangling slot. This patch reallows cancel/die interrupts while creating a slot and modifies the test to wait for slots to become zero to prevent finding an ephemeral slot. The reported hang doesn't happen in PG14 as the drop database starts to wait for ProcSignalBarrier with PG15 (commits 4eb2176318 and e2f65f4255) but it is good to backpatch this till PG14 as it is not a good idea to prevent interrupts during a network call that could block indefinitely. Reported-by: Lakshmi Narayanan Sreethar Diagnosed-by: Andres Freund Author: Hou Zhijie Reviewed-by: Vignesh C, Amit Kapila Backpatch-through: 14, where it was introduced in commit 6b67d72b60 Discussion: https://postgr.es/m/CA+kvmZELXQ4ZD3U=XCXuG3KvFgkuPoN1QrEj8c-rMRodrLOnsg@mail.gmail.com
* libpqwalreceiver: Convert to libpq-be-fe-helpers.hAndres Freund2023-01-23
| | | | | | | | | | | | | | | | In contrast to the changes to dblink and postgres_fdw, this does not fix a bug, as libpqwalreceiver did already process interrupts. Besides reducing code duplication, the conversion leads to libpqwalreceiver now using reserving file descriptors for libpq connections. While not strictly required for the use in walreceiver, we are also using libpqwalreceiver for logical replication, where it does seem more important. Even if we eventually decide to backpatch the prior commits, there'd be no need to backpatch this commit, due to not fixing an active bug. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20220925232237.p6uskba2dw6fnwj2@awork3.anarazel.de
* Fix error handling in libpqrcv_connect()Andres Freund2023-01-23
| | | | | | | | | | | | | | | | | When libpqrcv_connect (also known as walrcv_connect()) failed, it leaked the libpq connection. In most paths that's fairly harmless, as the calling process will exit soon after. But e.g. CREATE SUBSCRIPTION could lead to a somewhat longer lived leak. Fix by releasing resources, including the libpq connection, on error. Add a test exercising the error code path. To make it reliable and safe, the test tries to connect to port=-1, which happens to fail during connection establishment, rather than during connection string parsing. Reviewed-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/20230121011237.q52apbvlarfv6jm6@awork3.anarazel.de Backpatch: 11-
* Fix XLogPageRead() commentPeter Eisentraut2023-01-23
| | | | | | | | | 7fcbf6a and 2ff6555 changed the function signature of XLogPageRead() but did not update the comment. XLogReaderRoutine contains up to date information about the API, so no need to repeat all that at XLogPageRead(), but fix the mentions of the no longer existing function arguments.
* Add non-decimal integer support to type numeric.Dean Rasheed2023-01-23
| | | | | | | | | | | | | | | | | | This enhances the numeric type input function, adding support for hexadecimal, octal, and binary integers of any size, up to the limits of the numeric type. Since 6fcda9aba8, such non-decimal integers have been accepted by the parser as integer literals and passed through to numeric_in(). This commit gives numeric_in() the ability to handle them. While at it, simplify the handling of NaN and infinities, reducing the number of calls to pg_strncasecmp(), and arrange for pg_strncasecmp() to not be called at all for regular numbers. This gives a significant performance improvement for decimal inputs, more than offsetting the small performance hit of checking for non-decimal input. Discussion: https://postgr.es/m/CAEZATCV8XShnmT9HZy25C%2Bo78CVOFmUN5EM9FRAZ5xvYTggPMg%40mail.gmail.com
* Optimise numeric division for 3 and 4 base-NBASE digit divisors.Dean Rasheed2023-01-23
| | | | | | | | | | | | | | | | | On platforms with 128-bit integer support, introduce a new function div_var_int64(), along the same lines as div_var_int() added in d1b307eef2 for divisors with 1 or 2 base-NBASE digits, and use it to speed up div_var() and div_var_fast() in a similar way when the divisor has 3 or 4 base-NBASE digits. This gives significant performance gains for divisors with 9-16 decimal digits. Joel Jacobson. Discussion: https://postgr.es/m/b7a5893d-af18-4c0b-8918-96de5f1bbf39%40app.fastmail.com https://postgr.es/m/CAEZATCXGm%3DDyTq%3DFrcOqC0gPMVveKUYTaD5KRRoajrUTiWxVMw%40mail.gmail.com
* Run pgindent on heapam.cDavid Rowley2023-01-23
| | | | | | | | | | | An upcoming patch by Melanie Plageman does some refactoring work in this area. Run pgindent on that file now before making any changes so that it's easier to maintain/evolve each of the individual patches doing the refactor work. Additionally, add a few new required typedefs to the list to make it easier to do future pgindent runs on this file during the refactor work. Discussion: https://postgr.es/m/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com
* Fix and clarify function comment on LogicalTapeSetCreate.Heikki Linnakangas2023-01-23
| | | | | | | | | | | | | | | | | Commit c4649cce39 removed the "shared" and "ntapes" arguments, but the comment still talked about "shared". It also talked about "a shared file handle", which was technically correct because even before commit c4649cce39, the "shared file handle" referred to the "fileset" argument, not "shared". But it was very confusing. Improve the comment. Also add a comment on what the "preallocate" argument does. Backpatch to v15, just to make backpatching other patches easier in the future. Discussion: https://www.postgresql.org/message-id/af989685-91d5-aad4-8f60-1d066b5ec309@enterprisedb.com Reviewed-by: Peter Eisentraut
* Allow parallel aggregate on string_agg and array_aggDavid Rowley2023-01-23
| | | | | | | | | | | | | | This adds combine, serial and deserial functions for the array_agg() and string_agg() aggregate functions, thus allowing these aggregates to partake in partial aggregations. This allows both parallel aggregation to take place when these aggregates are present and also allows additional partition-wise aggregation plan shapes to include plans that require additional aggregation once the partially aggregated results from the partitions have been combined. Author: David Rowley Reviewed-by: Andres Freund, Tomas Vondra, Stephen Frost, Tom Lane Discussion: https://postgr.es/m/CAKJS1f9sx_6GTcvd6TMuZnNtCh0VhBzhX6FZqw17TgVFH-ga_A@mail.gmail.com
* Track logrep apply workers' last start times to avoid useless waits.Tom Lane2023-01-22
| | | | | | | | | | | | | | Enforce wal_retrieve_retry_interval on a per-subscription basis, rather than globally, and arrange to skip that delay in case of an intentional worker exit. This probably makes little difference in the field, where apply workers wouldn't be restarted often; but it has a significant impact on the runtime of our logical replication regression tests (even though those tests use artificially-small wal_retrieve_retry_interval settings already). Nathan Bossart, with mostly-cosmetic editorialization by me Discussion: https://postgr.es/m/20221122004119.GA132961@nathanxps13
* Allow REPLICA IDENTITY to be set on an index that's not (yet) valid.Tom Lane2023-01-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The motivation for this change is that when pg_dump dumps a partitioned index that's marked REPLICA IDENTITY, it generates a command sequence that applies REPLICA IDENTITY before the partitioned index has been marked valid, causing restore to fail. We could perhaps change pg_dump to not do it like that, but that would be difficult and would not fix existing dump files with the problem. There seems to be very little reason for the backend to disallow this anyway --- the code ignores indisreplident when the index isn't valid --- so instead let's fix it by allowing the case. Commit 9511fb37a previously expressed a concern that allowing indisreplident to be set on invalid indexes might allow us to wind up in a situation where a table could have indisreplident set on multiple indexes. I'm not sure I follow that concern exactly, but in any case the only way that could happen is because relation_mark_replica_identity is too trusting about the existing set of markings being valid. Let's just rip out its early-exit code path (which sure looks like premature optimization anyway; what are we doing expending code to make redundant ALTER TABLE ... REPLICA IDENTITY commands marginally faster and not-redundant ones marginally slower?) and fix it to positively guarantee that no more than one index is marked indisreplident. The pg_dump failure can be demonstrated in all supported branches, so back-patch all the way. I chose to back-patch 9511fb37a as well, just to keep indisreplident handling the same in all branches. Per bug #17756 from Sergey Belyashov. Discussion: https://postgr.es/m/17756-dd50e8e0c8dd4a40@postgresql.org
* Reject CancelRequestPacket having unexpected length.Noah Misch2023-01-21
| | | | | | | | | | | | | When the length was too short, the server read outside the allocation. That yielded the same log noise as sending the correct length with (backendPID,cancelAuthCode) matching nothing. Change to a message about the unexpected length. Given the attacker's lack of control over the memory layout and the general lack of diversity in memory layouts at the code in question, we doubt a would-be attacker could cause a segfault. Hence, while the report arrived via security@postgresql.org, this is not a vulnerability. Back-patch to v11 (all supported versions). Andrey Borodin, reviewed by Tom Lane. Reported by Andrey Borodin.
* Zero initialize uses of instr_time about to trigger compiler warningsAndres Freund2023-01-20
| | | | | | | | | These are all not necessary from a correctness POV. However, in the near future instr_time will be simplified to an int64, at which point gcc would otherwise start to warn about the changed places. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20230116023639.rn36vf6ajqmfciua@awork3.anarazel.de
* Move queryjumble.c code to src/backend/nodes/Michael Paquier2023-01-21
| | | | | | | | | | | | | This will ease a follow-up move that will generate automatically this code. The C file is renamed, for consistency with the node-related files whose code are generated by gen_node_support.pl: - queryjumble.c -> queryjumblefuncs.c - utils/queryjumble.h -> nodes/queryjumble.h Per a suggestion from Peter Eisentraut. Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/Y5BHOUhX3zTH/ig6@paquier.xyz
* Add new GUC reserved_connections.Robert Haas2023-01-20
| | | | | | | | | | | | This provides a way to reserve connection slots for non-superusers. The slots reserved via the new GUC are available only to users who have the new predefined role pg_use_reserved_connections. superuser_reserved_connections remains as a final reserve in case reserved_connections has been exhausted. Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13
* Rename ReservedBackends variable to SuperuserReservedConnections.Robert Haas2023-01-20
| | | | | | | | | | This is in preparation for adding a new reserved_connections GUC, but aligning the GUC name with the variable name is also a good idea on general principle. Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13
* Update docs and error message for superuser_reserved_connections.Robert Haas2023-01-20
| | | | | | | | | | | | | | Commit ea92368cd1da1e290f9ab8efb7f60cb7598fc310 made max_wal_senders a separate pool of backends from max_connections, but the documentation and error message for superuser_reserved_connections weren't updated at the time, and as a result are somewhat misleading. Update. This is arguably a back-patchable bug fix, but because it seems quite minor, no back-patch. Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13
* Remove SHM_QUEUEAndres Freund2023-01-19
| | | | | | | | | | Prior patches got rid of all the uses of SHM_QUEUE. ilist.h style lists are more widely used and have an easier to use interface. As there are no users left, remove SHM_QUEUE. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> (in an older version) Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Use dlists instead of SHM_QUEUE for predicate lockingAndres Freund2023-01-19
| | | | | | | | | Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used and have an easier to use interface. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> (in an older version) Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Support the same patterns for pg-user in pg_ident.conf as in pg_hba.confMichael Paquier2023-01-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While pg_hba.conf has support for non-literal username matches, and this commit extends the capabilities that are supported for the PostgreSQL user listed in an ident entry part of pg_ident.conf, with support for: 1. The "all" keyword, where all the requested users are allowed. 2. Membership checks using the + prefix. 3. Using a regex to match against multiple roles. 1. is a feature that has been requested by Jelte Fennema, 2. something that has been mentioned independently by Andrew Dunstan, and 3. is something I came up with while discussing how to extend the first one, whose implementation is facilitated by 8fea868. This allows matching certain system users against many different postgres users with a single line in pg_ident.conf. Without this, one would need one line for each of the postgres users that a system user can log in as, which can be cumbersome to maintain. Tests are added to the TAP test of peer authentication to provide coverage for all that. Note that this introduces a set of backward-incompatible changes to be able to detect the new patterns, for the following cases: - A role named "all". - A role prefixed with '+' characters, which is something that would not have worked in HBA entries anyway. - A role prefixed by a slash character, similarly to 8fea868. Any of these can be still be handled by using quotes in the Postgres role defined in an ident entry. A huge advantage of this change is that the code applies the same checks for the Postgres roles in HBA and ident entries, via the common routine check_role(). **This compatibility change should be mentioned in the release notes.** Author: Jelte Fennema Discussion: https://postgr.es/m/DBBPR83MB0507FEC2E8965012990A80D0F7FC9@DBBPR83MB0507.EURPRD83.prod.outlook.com
* Use appendStringInfoSpaces in more placesDavid Rowley2023-01-20
| | | | | | | | | | | | | | | | | | | | | | | This adjusts a few places which were appending a string constant containing spaces onto a StringInfo. We have appendStringInfoSpaces for that job, so let's use that instead. For the change to jsonb.c's add_indent() function, appendStringInfoString was being called inside a loop to append 4 spaces on each loop. This meant that enlargeStringInfo would get called once per loop. Here it should be much more efficient to get rid of the loop and just calculate the number of spaces with "level * 4" and just append all the spaces in one go. Here we additionally adjust the appendStringInfoSpaces function so it makes use of memset rather than a while loop to apply the required spaces to the StringInfo. One of the problems with the while loop was that it was incrementing one variable and decrementing another variable once per loop. That's more work than what's required to get the job done. We may as well use memset for this rather than trying to optimize the existing loop. Some testing has shown memset is faster even for very small sizes. Discussion: https://postgr.es/m/CAApHDvp_rKkvwudBKgBHniNRg67bzXVjyvVKfX0G2zS967K43A@mail.gmail.com
* Improve comment about GetWALAvailability's WALAVAIL_REMOVED code.Tom Lane2023-01-19
| | | | | | Sirisha Chamarthi and Kyotaro Horiguchi Discussion: https://postgr.es/m/CAKrAKeXt-=bgm=d+EDmcC9kWoikp8kbVb3LH0K3K+AGGsykpHQ@mail.gmail.com
* Fix ts_headline() to handle ORs and phrase queries more honestly.Tom Lane2023-01-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch largely reverts what I did in commits c9b0c678d and 78e73e875. The maximum cover length limit that I added in 78e73e875 (to band-aid over c9b0c678d's performance issues) creates too many user-visible behavior discrepancies, as complained of for example in bug #17691. The real problem with hlCover() is not what I thought at the time, but more that it seems to have been designed with only AND tsquery semantics in mind. It doesn't work quite right for OR, and even less so for NOT or phrase queries. However, we can improve that situation by building a variant of TS_execute() that returns a list of match locations. We already get an ExecPhraseData struct representing match locations for the primitive case of a simple match, as well as one for a phrase match; we just need to add some logic to combine these for AND and OR operators. The result is a list of ExecPhraseDatas, which hlCover can regard as having simple AND semantics, so that its old algorithm works correctly. There's still a lot not to like about ts_headline's behavior, but I think the remaining issues have to do with the heuristics used in mark_hl_words and mark_hl_fragments (which, likewise, were not revisited when phrase search was added). Improving those is a task for another day. Patch by me; thanks to Alvaro Herrera for review. Discussion: https://postgr.es/m/840.1669405935@sss.pgh.pa.us
* Log the correct ending timestamp in recovery_target_xid mode.Tom Lane2023-01-19
| | | | | | | | | | | | | | | | When ending recovery based on recovery_target_xid matching with recovery_target_inclusive = off, we printed an incorrect timestamp (always 2000-01-01) in the "recovery stopping before ... transaction" log message. This is a consequence of sloppy refactoring in c945af80c: the code to fetch recordXtime out of the commit/abort record used to be executed unconditionally, but it was changed to get called only in the RECOVERY_TARGET_TIME case. We need only flip the order of operations to restore the intended behavior. Per report from Torsten Förtsch. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKkG4_kUevPqbmyOfLajx7opAQk6Cvwkvx0HRcFjSPfRPTXanA@mail.gmail.com
* Remove some dead code in selfuncs.cAlvaro Herrera2023-01-19
| | | | | | | | | | | RelOptInfo.userid is the same for all relations in a given inheritance tree, so the code in examine_variable() and example_simple_variable() that repeats the ACL checks on the root parent rel instead of a given leaf child relations need not recompute userid too. Author: Amit Langote <amitlangote09@gmail.com> Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20221210201753.GA27893@telsasoft.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 14:46:13 +0000