aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* autoconf: Move export_dynamic determination to configureAndres Freund2022-12-06
| | | | | | | | | | | | | Previously export_dynamic was set in src/makefiles/Makefile.$port. For solaris this required exporting with_gnu_ld. The determination of with_gnu_ld would be nontrivial to copy for meson PGXS compatibility. It's also nice to delete libtool.m4. This uses -Wl,--export-dynamic on all platforms, previously all platforms but FreeBSD used -Wl,-E. The likelihood of a name conflict seems lower with the longer spelling. Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de
* Generate pg_stat_get*() functions for databases using macrosMichael Paquier2022-12-07
| | | | | | | | | | | | | | | The same code pattern is repeated 21 times for int64 counters (0 for missing entry) and 5 times for doubles (0 for missing entry) on database entries. This code is switched to use macros for the basic code instead, shaving a few hundred lines of originally-duplicated code patterns. The function names remain the same, but some fields of PgStat_StatDBEntry have to be renamed to cope with the new style. This is in the same spirit as 83a1a1b. Author: Michael Paquier Reviewed-by: Nathan Bossart, Bertrand Drouvot Discussion: https://postgr.es/m/Y46stlxQ2LQE20Na@paquier.xyz
* Rework query relation permission checkingAlvaro Herrera2022-12-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, information about the permissions to be checked on relations mentioned in a query is stored in their range table entries. So the executor must scan the entire range table looking for relations that need to have permissions checked. This can make the permission checking part of the executor initialization needlessly expensive when many inheritance children are present in the range range. While the permissions need not be checked on the individual child relations, the executor still must visit every range table entry to filter them out. This commit moves the permission checking information out of the range table entries into a new plan node called RTEPermissionInfo. Every top-level (inheritance "root") RTE_RELATION entry in the range table gets one and a list of those is maintained alongside the range table. This new list is initialized by the parser when initializing the range table. The rewriter can add more entries to it as rules/views are expanded. Finally, the planner combines the lists of the individual subqueries into one flat list that is passed to the executor for checking. To make it quick to find the RTEPermissionInfo entry belonging to a given relation, RangeTblEntry gets a new Index field 'perminfoindex' that stores the corresponding RTEPermissionInfo's index in the query's list of the latter. ExecutorCheckPerms_hook has gained another List * argument; the signature is now: typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable, List *rtePermInfos, bool ereport_on_violation); The first argument is no longer used by any in-core uses of the hook, but we leave it in place because there may be other implementations that do. Implementations should likely scan the rtePermInfos list to determine which operations to allow or deny. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com
* Fix 32-bit build dangling pointer issue in WindowAggDavid Rowley2022-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 9d9c02ccd added window "run conditions", which allows the evaluation of monotonic window functions to be skipped when the run condition is no longer true. Prior to this commit, once the run condition was no longer true and we stopped evaluating the window functions, we simply just left the ecxt_aggvalues[] and ecxt_aggnulls[] arrays alone to store whatever value was stored there the last time the window function was evaluated. Leaving a stale value in there isn't really a problem on 64-bit builds as all of the window functions which we recognize as monotonic all return int8, which is passed by value on 64-bit builds. However, on 32-bit builds, this was a problem as the value stored in the ecxt_values[] element would be a by-ref value and it would be pointing to some memory which would get reset once the tuple context is destroyed. Since the WindowAgg node will output these values in the resulting tupleslot, this could be problematic for the top-level WindowAgg node which must look at these values to filter out the rows that don't meet its filter condition. Here we fix this by just zeroing the ecxt_aggvalues[] and setting the ecxt_aggnulls[] array to true when the run condition first becomes false. This results in the WindowAgg's output having NULLs for the WindowFunc's columns rather than the stale or pointer pointing to possibly freed memory. These tuples with the NULLs can only make it as far as the top-level WindowAgg node before they're filtered out. To ensure that these tuples *are* always filtered out, we now insist that OpExprs making up the run condition are strict OpExprs. Currently, all the window functions which the planner recognizes as monotonic return INT8 and the operator which is used for the run condition must be a member of a btree opclass. In reality, these restrictions exclude nothing that's built-in to Postgres and are unlikely to exclude anyone's custom operators due to the requirement that the operator is part of a btree opclass. It would be unusual if those were not strict. Reported-by: Sergey Shinderuk, using valgrind Reviewed-by: Richard Guo, Sergey Shinderuk Discussion: https://postgr.es/m/29184c50-429a-ebd7-f1fb-0589c6723a35@postgrespro.ru Backpatch-through: 15, where 9d9c02ccd was added
* Generate pg_stat_get*() functions for tables using macrosMichael Paquier2022-12-06
| | | | | | | | | | | | | The same code pattern is repeated 17 times for int64 counters (0 for missing entry) and 5 times for timestamps (NULL for missing entry) on table entries. This code is switched to use a macro for the basic code instead, shaving a few hundred lines of originally-duplicated code. The function names remain the same, but some fields of PgStat_StatTabEntry have to be renamed to cope with the new style. Author: Bertrand Drouvot Reviewed-by: Nathan Bossart Discussion: https:/postgr.es/m/20221204173207.GA2669116@nathanxps13
* Check the snapshot argument of index_beginscan and familyAlexander Korotkov2022-12-06
| | | | | | | | | | | | Passing a NULL snapshot (InvalidSnapshot) is going to work but only as long as the index can't find any matching rows. This can be confusing for the extension authors, so add an explicit check for this argument. The check is implemented with Assert() in order to avoid overhead in release builds. Reported-by: Sven Klemm Discussion: https://postgr.es/m/CAJ7c6TPxitD4vbKyP-mpmC1XwyHdPPqvjLzm%2BVpB88h8LGgneQ%40mail.gmail.com Author: Aleksander Alekseev Reviewed-by: Pavel Borisov
* Fix Memoize to work with partitionwise joining.Tom Lane2022-12-05
| | | | | | | | | | | | A couple of places weren't up to speed for this. By sheer good luck, we didn't fail but just selected a non-memoized join plan, at least in the test case we have. Nonetheless, it's a bug, and I'm not quite sure that it couldn't have worse consequences in other examples. So back-patch to v14 where Memoize came in. Richard Guo Discussion: https://postgr.es/m/CAMbWs48GkNom272sfp0-WeD6_0HSR19BJ4H1c9ZKSfbVnJsvRg@mail.gmail.com
* Add LSN location in some error messages related to WAL pagesMichael Paquier2022-12-05
| | | | | | | | | | | | | | The error messages reported during any failures while reading or validating the header of a WAL currently includes only the offset of the page but not the compiled LSN referring to the page, requiring an extra step to compile it if looking at the surroundings with pg_waldump or similar. Adding this information costs a bit in translation, but also eases debugging. Author: Bharath Rupireddy Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, Maxim Orlov, Michael Paquier Discussion: https://postgr.es/m/CALj2ACWV=FCddsxcGbVOA=cvPyMr75YCFbSQT6g4KDj=gcJK4g@mail.gmail.com
* Fix thinko introduced in 6b423ec67David Rowley2022-12-05
| | | | | | | | | | | | | | | | | | As pointed out by Dean Rasheed, we really should be using tmp > -(PG_INTNN_MIN / 10) rather than tmp > (PG_INTNN_MAX / 10) for checking for overflows in the accumulation in the pg_strtointNN functions. This does happen to be the same number when dividing by 10, but there is a pending patch which adds other bases and this is not the same number if we were to divide by 2 rather than 10, for example. If the base 2 parsing was to follow this example then we could accidentally think a string containing the value of PG_INT32_MIN was an overflow in pg_strtoint32. Clearly that shouldn't overflow. This does not fix any actual live bugs, only some bad examples of overflow checks for future bases. Reported-by: Dean Rasheed Discussion: https://postgr.es/m/CAEZATCVEtwfhdm-K-etZYFB0=qsR0nT6qXta_W+GQx4RYph1dg@mail.gmail.com
* Re-pgindent a few files.Tom Lane2022-12-04
| | | | | | Just because I'm a neatnik, and I'm currently working on code in this area. It annoys me to not be able to pgindent my patches without working around unrelated changes.
* Fix broken MemoizePath support in reparameterize_path().Tom Lane2022-12-04
| | | | | | | | | | | | | | It neglected to recurse to the subpath, meaning you'd get back a path identical to the input. This could produce wrong query results if the omission meant that the subpath fails to enforce some join clause it should be enforcing. We don't have a test case for this at the moment, but the code is obviously broken and the fix is equally obvious. Back-patch to v14 where Memoize was introduced. Richard Guo Discussion: https://postgr.es/m/CAMbWs4_R=ORpz=Lkn2q3ebPC5EuWyfZF+tmfCPVLBVK5W39mHA@mail.gmail.com
* Add missing MaterialPath support in reparameterize_path[_by_child].Tom Lane2022-12-04
| | | | | | | | | | These two functions failed to cover MaterialPath. That's not a fatal problem, but we can generate better plans in some cases if we support it. Tom Lane and Richard Guo Discussion: https://postgr.es/m/1854233.1669949723@sss.pgh.pa.us
* Fix generate_partitionwise_join_paths() to tolerate failure.Tom Lane2022-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | We might fail to generate a partitionwise join, because reparameterize_path_by_child() does not support all path types. This should not be a hard failure condition: we should just fall back to a non-partitioned join. However, generate_partitionwise_join_paths did not consider this possibility and would emit the (misleading) error "could not devise a query plan for the given query" if we'd failed to make any paths for a child join. Fix it to give up on partitionwise joining if so. (The accepted technique for giving up appears to be to set rel->nparts = 0, which I find pretty bizarre, but there you have it.) I have not added a test case because there'd be little point: any omissions of this sort that we identify would soon get fixed by extending reparameterize_path_by_child(), so the test would stop proving anything. However, right now there is a known test case based on failure to cover MaterialPath, and with that I've found that this is broken in all supported versions. Hence, patch all the way back. Original report and patch by me; thanks to Richard Guo for identifying a test case that works against committed versions. Discussion: https://postgr.es/m/1854233.1669949723@sss.pgh.pa.us
* Improve performance of pg_strtointNN functionsDavid Rowley2022-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Experiments have shown that modern versions of both gcc and clang are unable to fully optimize the multiplication by 10 that we're doing in the pg_strtointNN functions. Both compilers seem to be making use of "imul", which is not the most efficient way to multiply by 10. This seems to be due to the overflow checking that we're doing. Without the overflow checks, both those compilers switch to a more efficient method of multiplying by 10. In absence of overflow concern, integer multiplication by 10 can be done by bit-shifting left 3 places to multiply by 8 and then adding the original value twice. To allow compilers this flexibility, here we adjust the code so that we accumulate the number as an unsigned version of the type and remove the use of pg_mul_sNN_overflow() and pg_sub_sNN_overflow(). The overflow checking can be done simply by checking if the accumulated value has gone beyond a 10th of the maximum *signed* value for the given type. If it has then the accumulation of the next digit will cause an overflow. After this is done, we do a final overflow check before converting the unsigned version of the number back to its signed counterpart. Testing has shown about an 8% speedup of a COPY into a table containing 2 INT columns. Author: David Rowley, Dean Rasheed Discussion: https://postgr.es/m/CAApHDvrL6_+wKgPqRHr7gH_6xy3hXM6a3QCsZ5ForurjDFfenA@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvrdYByjfj-=WbmVNFgmVZg88-dE7heukw8p55aJ+W=qxQ@mail.gmail.com
* Doc: flesh out fmgr/README's description of context-node usage.Tom Lane2022-12-03
| | | | | | | I wrote this to provide a home for a planned discussion of error return conventions for non-error-throwing functions. But it seems useful as documentation of existing code no matter what becomes of that proposal, so commit separately.
* Fix DEFAULT handling for multi-row INSERT rules.Dean Rasheed2022-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | | When updating a relation with a rule whose action performed an INSERT from a multi-row VALUES list, the rewriter might skip processing the VALUES list, and therefore fail to replace any DEFAULTs in it. This would lead to an "unrecognized node type" error. The reason was that RewriteQuery() assumed that a query doing an INSERT from a multi-row VALUES list would necessarily only have one item in its fromlist, pointing to the VALUES RTE to read from. That assumption is correct for the original query, but not for product queries produced for rule actions. In such cases, there may be multiple items in the fromlist, possibly including multiple VALUES RTEs. What is required instead is for RewriteQuery() to skip any RTEs from the product query's originating query, which might include one or more already-processed VALUES RTEs. What's left should then include at most one VALUES RTE (from the rule action) to be processed. Patch by me. Thanks to Tom Lane for reviewing. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAEZATCV39OOW7LAR_Xq4i%2BLc1Byux%3DeK3Q%3DHD_pF1o9LBt%3DphA%40mail.gmail.com
* Prevent pgstats from getting confused when relkind of a relation changesAndres Freund2022-12-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | When the relkind of a relache entry changes, because a table is converted into a view, pgstats can get confused in 15+, leading to crashes or assertion failures. For HEAD, Tom fixed this in b23cd185fd5, by removing support for converting a table to a view, removing the source of the inconsistency. This commit just adds an assertion that a relcache entry's relkind does not change, just in case we end up with another case of that in the future. As there's no cases of changing relkind anymore, we can't add a test that that's handled correctly. For 15, fix the problem by not maintaining the association with the old pgstat entry when the relkind changes during a relcache invalidation processing. In that case the pgstat entry needs to be unlinked first, to avoid PgStat_TableStatus->relation getting out of sync. Also add a test reproducing the issues. No known problem exists in 11-14, so just add the test there. Reported-by: vignesh C <vignesh21@gmail.com> Author: Andres Freund <andres@anarazel.de> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com Discussion: https://postgr.es/m/CALDaNm3oZA-8Wbps2Jd1g5_Gjrr-x3YWrJPek-mF5Asrrvz2Dg@mail.gmail.com Backpatch: 15-
* Fix broken hash function hashbpcharextended().Jeff Davis2022-12-02
| | | | | | | | | | | | | | | Ignore trailing spaces for non-deterministic collations when hashing. The previous behavior could lead to tuples falling into the wrong partitions when hash partitioning is combined with the BPCHAR type and a non-deterministic collation. Fortunately, it did not affect hash indexes, because hash indexes do not use extended hash functions. Decline to backpatch, per discussion. Discussion: https://postgr.es/m/eb83d0ac7b299eb08f9b900dd08a5a0c5d90e517.camel@j-davis.com Reviewed-by: Richard Guo, Tom Lane
* Remove gen_node_support.pl's special treatment of EquivalenceClasses.Tom Lane2022-12-02
| | | | | | | | | | It seems better to deal with this by explicit annotations on the fields in question, instead of magic knowledge embedded in the script. While that creates a risk-of-omission from failing to annotate fields, the preceding commit should catch any such oversights. Discussion: https://postgr.es/m/263413.1669513145@sss.pgh.pa.us
* Add some error cross-checks to gen_node_support.pl.Tom Lane2022-12-02
| | | | | | | | | | | | | | | | | | Check that if we generate a call to copy, compare, write, or read a specific node type, that node type does have the appropriate support function. (This doesn't protect against trying to invoke nonexistent code when considering generic field types such as "Node *", but it seems like a useful check anyway.) Check that array_size() refers to a field appearing earlier in the struct. Aside from catching obvious errors like a misspelled field name, this protects against a more subtle mistake: if the size field appears later in the struct than the array field, then compare and read functions would misbehave. There is actually exactly that situation in PlannerInfo, but it's okay since we do not need compare or read functionality for that (today anyway). Discussion: https://postgr.es/m/263413.1669513145@sss.pgh.pa.us
* Fix psql's \sf and \ef for new-style SQL functions.Tom Lane2022-12-02
| | | | | | | | | | | | | | Some options of these commands need to be able to identify the start of the function body within the output of pg_get_functiondef(). It used to be that that always began with "AS", but since the introduction of new-style SQL functions, it might also start with "BEGIN" or "RETURN". Fix that on the psql side, and add some regression tests. Noted by me awhile ago, but I didn't do anything about it. Thanks to David Johnston for a nag. Discussion: https://postgr.es/m/AM9PR01MB8268D5CDABDF044EE9F42173FE8C9@AM9PR01MB8268.eurprd01.prod.exchangelabs.com
* Remove logic for converting a table to a view.Tom Lane2022-12-02
| | | | | | | | | | | | | | | | | | | | Up to now we have allowed manual creation of an ON SELECT rule on a table to convert it into a view. That was never anything but a horrid, error-prone hack though. pg_dump used to rely on that behavior to deal with cases involving circular dependencies, where a dependency loop could be broken by separating the creation of a view from installation of its ON SELECT rule. However, we changed pg_dump to use CREATE OR REPLACE VIEW for that in commit d8c05aff5 (which was later back-patched as far as 9.4), so there's not a good argument anymore for continuing to support the behavior. The proximate reason for axing it now is that we found that the new statistics code has failure modes associated with the relkind change caused by this behavior. We'll patch around that in v15, but going forward it seems like a better idea to get rid of the need to support relkind changes. Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com
* Generalize ri_RootToPartitionMap to use for non-partition childrenAlvaro Herrera2022-12-02
| | | | | | | | | | | | | | | | | | | | | | | ri_RootToPartitionMap is currently only initialized for tuple routing target partitions, though a future commit will need the ability to use it even for the non-partition child tables, so make adjustments to the decouple it from the partitioning code. Also, make it lazily initialized via ExecGetRootToChildMap(), making that function its preferred access path. Existing third-party code accessing it directly should no longer do so; consequently, it's been renamed to ri_RootToChildMap, which also makes it consistent with ri_ChildToRootMap. ExecGetRootToChildMap() houses the logic of setting the map appropriately depending on whether a given child relation is partition or not. To support this, also add a separate entry point for TupleConversionMap creation that receives an AttrMap. No new code here, just split an existing function in two. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqEYUhDXSK5BTvG_xk=eaAEJCD4GS3C6uH7ybBvv+Z_Tmg@mail.gmail.com
* Fix incorrect output from pgoutput when using column lists.Amit Kapila2022-12-02
| | | | | | | | | | | | | | | | | | | For Updates and Deletes, we were not honoring the columns list for old tuple values while sending tuple data via pgoutput. This results in pgoutput emitting more columns than expected. This is not a problem for built-in logical replication as we simply ignore additional columns based on the relation information sent previously which didn't have those columns. However, some other users of pgoutput plugin may expect the columns as per the column list. Also, sending extra columns unnecessarily consumes network bandwidth defeating the purpose of the column list feature. Reported-by: Gunnar Morling Author: Hou Zhijie Reviewed-by: Amit Kapila Backpatch-through: 15 Discussion: https://postgr.es/m/CADGJaX9kiRZ-OH0EpWF5Fkyh1ZZYofoNRCrhapBfdk02tj5EKg@mail.gmail.com
* autoconf: Don't AC_SUBST() LD in configureAndres Freund2022-12-01
| | | | | | | | | | | | | | The only use of $(LD) in Makefiles is for AIX, to generate the export file for the backend. We only support the system linker on AIX and we already hardcode the path to a number of other binaries. Removing LD substitution will simplify the upcoming meson PGXS compatibility. While at it, add a comment why -r is used. A subsequent commit will remove the determination of LD from configure as well. Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de
* Fix memory leak for hashing with nondeterministic collations.Jeff Davis2022-12-01
| | | | | | | Backpatch through 12, where nondeterministic collations were introduced (5e1963fb76). Backpatch-through: 12
* Fix under-parenthesized display of AT TIME ZONE constructs.Tom Lane2022-12-01
| | | | | | | | | | | | | | | | | | In commit 40c24bfef, I forgot to use get_rule_expr_paren() for the arguments of AT TIME ZONE, resulting in possibly not printing parens for expressions that need it. But get_rule_expr_paren() wouldn't have gotten it right anyway, because isSimpleNode() hadn't been taught that COERCE_SQL_SYNTAX parent nodes don't guarantee sufficient parentheses. Improve all that. Also use this methodology for F_IS_NORMALIZED, so that we don't print useless parens for that. In passing, remove a comment that was obsoleted later. Per report from Duncan Sands. Back-patch to v14 where this code came in. (Before that, we didn't try to print AT TIME ZONE that way, so there was no bug just ugliness.) Discussion: https://postgr.es/m/f41566aa-a057-6628-4b7c-b48770ecb84a@deepbluecap.com
* Move PartitioPruneInfo out of plan nodes into PlannedStmtAlvaro Herrera2022-12-01
| | | | | | | | | | | | | | | | | | | | The planner will now add a given PartitioPruneInfo to PlannedStmt.partPruneInfos instead of directly to the Append/MergeAppend plan node. What gets set instead in the latter is an index field which points to the list element of PlannedStmt.partPruneInfos containing the PartitioPruneInfo belonging to the plan node. A later commit will make AcquireExecutorLocks() do the initial partition pruning to determine a minimal set of partitions to be locked when validating a plan tree and it will need to consult the PartitioPruneInfos referenced therein to do so. It would be better for the PartitioPruneInfos to be accessible directly than requiring a walk of the plan tree to find them, which is easier when it can be done by simply iterating over PlannedStmt.partPruneInfos. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
* Make materialized views participate in predicate lockingMichael Paquier2022-12-01
| | | | | | | | | | | | | | | | | | | | | | | Matviews have been discarded from needing predicate locks since 3bf3ab8 and their introduction. At this point, there was no concurrent flavor of REFRESH yet, hence there was no meaning in having materialized views look at read/write conflicts with concurrent transactions using themselves the serializable isolation level because they could only be refreshed with an access exclusive lock. CONCURRENTLY, on the contrary, allows reads and writes during a refresh as it holds a share update exclusive lock. Some isolation tests are added to show the effect of the change, with a combination of one table and a matview based on it, using a mix of REFRESH CONCURRENTLY and read/write queries. This could arguably be considered as a bug, but as it is a subtle behavior change potentially impacting applications no backpatch is done. Author: Yugo Nagata Reviewed-by: Richard Guo, Dilip Kumar, Michael Paquier Discussion: https://postgr.es/m/20220726164434.42d4e33911b4b4fcf751c4e7@sraoss.co.jp
* Stop accessing checkAsUser via RTE in some casesAlvaro Herrera2022-11-30
| | | | | | | | | | | | | | | | | | | | A future commit will move the checkAsUser field from RangeTblEntry to a new node that, unlike RTEs, will only be created for tables mentioned in the query but not for the inheritance child relations added to the query by the planner. So, checkAsUser value for a given child relation will have to be obtained by referring to that for its ancestor mentioned in the query. In preparation, it seems better to expand the use of RelOptInfo.userid during planning in place of rte->checkAsUser so that there will be fewer places to adjust for the above change. Given that the child-to-ancestor mapping is not available during the execution of a given "child" ForeignScan node, add a checkAsUser field to ForeignScan to carry the child relation's RelOptInfo.userid. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqGFCs2uq7VRKi7g+FFKbP6Ea_2_HkgZb2HPhUfaAKT3ng@mail.gmail.com
* Improve heuristics for compressing the KnownAssignedXids array.Tom Lane2022-11-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we'd compress only when the active range of array entries reached Max(4 * PROCARRAY_MAXPROCS, 2 * pArray->numKnownAssignedXids). If max_connections is large, the first term could result in not compressing for a long time, resulting in much wastage of cycles in hot-standby backends scanning the array to take snapshots. Get rid of that term, and just bound it to 2 * pArray->numKnownAssignedXids. That however creates the opposite risk, that we might spend too much effort compressing. Hence, consider compressing only once every 128 commit records. (This frequency was chosen by benchmarking. While we only tried one benchmark scenario, the results seem stable over a fairly wide range of frequencies.) Also, force compression when processing RecoveryInfo WAL records (which should be infrequent); the old code could perform compression then, but would do so only after the same array-range check as for the transaction-commit path. Also, opportunistically run compression if the startup process is about to wait for WAL, though not oftener than once a second. This should prevent cases where we waste lots of time by leaving the array not-compressed for long intervals due to low WAL traffic. Lastly, add a simple check to keep us from uselessly compressing when the array storage is already compact. Back-patch, as the performance problem is worse in pre-v14 branches than in HEAD. Simon Riggs and Michail Nikolaev, with help from Tom Lane and Andres Freund. Discussion: https://postgr.es/m/CALdSSPgahNUD_=pB_j=1zSnDBaiOtqVfzo8Ejt5J_k7qZiU1Tw@mail.gmail.com
* Prevent clobbering of utility statements in SQL function caches.Tom Lane2022-11-29
| | | | | | | | | | | | | | | | | | | | | | This is an oversight in commit 7c337b6b5: I apparently didn't think about the possibility of a SQL function being executed multiple times within a query. In that case, functions.c's primitive caching mechanism allows the same utility parse tree to be presented for execution more than once. We have to tell ProcessUtility to make a working copy of the parse tree, or bad things happen. Normally I'd add a regression test, but I think the reported crasher is dependent on some rather random implementation choices that are nowhere near functions.c, so its usefulness as a long-lived test feels questionable. In any case, this fix is clearly correct given the design choices of 7c337b6b5. Per bug #17702 from Xin Wen. Thanks to Daniel Gustafsson for analysis. Back-patch to v14 where the faulty commit came in (before that, the responsibility for copying scribble-able utility parse trees lay elsewhere). Discussion: https://postgr.es/m/17702-ad24fdcdd1e9047a@postgresql.org
* Remove bogus Assert and dead code in remove_useless_results_recurse().Tom Lane2022-11-29
| | | | | | | | | | | | | | | | | | | | The JOIN_SEMI case Assert'ed that there are no PlaceHolderVars that need to be evaluated at the semijoin's RHS, which is wrong because there could be some in the semijoin's qual condition. However, there could not be any references further up than that, and within the qual there is not any way that such a PHV could have gone to null yet, so we don't really need the PHV and there is no need to avoid making the RHS-removal optimization. The upshot is that there's no actual bug in production code, and we ought to just remove this misguided Assert. While we're here, also drop the JOIN_RIGHT case, which is dead code because reduce_outer_joins() already got rid of JOIN_RIGHT. Per bug #17700 from Xin Wen. Uselessness of the JOIN_RIGHT case pointed out by Richard Guo. Back-patch to v12 where this code was added. Discussion: https://postgr.es/m/17700-2b5c10d917c30687@postgresql.org
* Add 'missing_ok' argument to build_attrmap_by_nameAlvaro Herrera2022-11-29
| | | | | | | | | | | | When it's given as true, return a 0 in the position of the missing column rather than raising an error. This is currently unused, but it allows us to reimplement column permission checking in a subsequent commit. It seems worth breaking into a separate commit because it affects unrelated code. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqFfiai=qBxPDTjaio_ZcaqUKh+FC=prESrB8ogZgFNNNQ@mail.gmail.com
* Fix comment in snapbuild.cMichael Paquier2022-11-29
| | | | | Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoAmf-PkSnMGAJg2DtGhp7O7vpHoexCxfQLKZg8xrbRwsg@mail.gmail.com
* Remove promote_trigger_file.Thomas Munro2022-11-29
| | | | | | | | | | | | | | | | | | | Previously, an idle startup (recovery) process would wake up every 5 seconds to have a chance to poll for promote_trigger_file, even if that GUC was not configured. That promotion triggering mechanism was effectively superseded by pg_ctl promote and pg_promote() a long time ago. There probably aren't many users left and it's very easy to change to the modern mechanisms, so we agreed to remove the feature. This is part of a campaign to reduce wakeups on idle systems. Author: Simon Riggs <simon.riggs@enterprisedb.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Ian Lawrence Barwick <barwick@gmail.com> Discussion: https://postgr.es/m/CANbhV-FsjnzVOQGBpQ589%3DnWuL1Ex0Ykn74Nh1hEjp2usZSR5g%40mail.gmail.com
* Provide non-superuser predefined roles for vacuum and analyzeAndrew Dunstan2022-11-28
| | | | | | | | | | | | | | | | This provides two new predefined roles: pg_vacuum_all_tables and pg_analyze_all_tables. Roles which have been granted these roles can perform vacuum or analyse respectively on any or all tables as if they were a superuser. This removes the need to grant superuser privilege to roles just so they can perform vacuum and/or analyze. Nathan Bossart Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael Paquier. Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13
* Provide per-table permissions for vacuum and analyze.Andrew Dunstan2022-11-28
| | | | | | | | | | | | | | Currently a table can only be vacuumed or analyzed by its owner or a superuser. This can now be extended to any user by means of an appropriate GRANT. Nathan Bossart Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael Paquier. Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13
* Fix typo in hba.cMichael Paquier2022-11-26
| | | | Spotted while reading through the git history.
* Correct some SQL feature namesPeter Eisentraut2022-11-25
| | | | | Maybe these were initially typos or they have been changed upstream since we first added them. This brings it all in line with SQL:2016.
* Fix gen_node_support.pl for changed AclMode sizeAndrew Dunstan2022-11-25
| | | | | | omitted from 7b378237aa, mea culpa. Complaint and fix from Amit Langote.
* Fix rule-detection code for MERGE.Dean Rasheed2022-11-25
| | | | | | | | | | | Use the relation's rd_rules structure to test whether it has rules, rather than the relhasrules flag, which might be out of date. Reviewed by Tom Lane. Backpatch to 15, where MERGE was added. Discussion: https://postgr.es/m/CAEZATCVkBVZABfw71sYvkcPf6tarcOFST5Bc6AOi-LFT9YdccQ%40mail.gmail.com
* Fix handling of pending inserts in nodeModifyTable.c.Etsuro Fujita2022-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit b663a4136, which allowed FDWs to INSERT rows in bulk, added to nodeModifyTable.c code to flush pending inserts to the foreign-table result relation(s) before completing processing of the ModifyTable node, but the code failed to take into account the case where the INSERT query has modifying CTEs, leading to incorrect results. Also, that commit failed to flush pending inserts before firing BEFORE ROW triggers so that rows are visible to such triggers. In that commit we scanned through EState's es_tuple_routing_result_relations or es_opened_result_relations list to find the foreign-table result relations to which pending inserts are flushed, but that would be inefficient in some cases. So to fix, 1) add a List member to EState to record the insert-pending result relations, and 2) modify nodeModifyTable.c so that it adds the foreign-table result relation to the list in ExecInsert() if appropriate, and flushes pending inserts properly using the list where needed. While here, fix a copy-and-pasteo in a comment in ExecBatchInsert(), which was added by that commit. Back-patch to v14 where that commit appeared. Discussion: https://postgr.es/m/CAPmGK16qutyCmyJJzgQOhfBq%3DNoGDqTB6O0QBZTihrbqre%2BoxA%40mail.gmail.com
* Introduce variables for initial and max nesting depth on configuration filesMichael Paquier2022-11-25
| | | | | | | | | | The code has been assuming already in a few places that the initial recursion nesting depth is 0, and the recent changes in hba.c (mainly 783e8c6) have relies on this assumption in more places. The maximum recursion nesting level is assumed to be 10 for hba.c and GUCs. Author: Julien Rouhaud Discussion: https://postgr.es/m/20221124090724.n7amf5kpdhx6vb76@jrouhaud
* Fix some 32-bit shift warnings in MSVCDavid Rowley2022-11-25
| | | | | | | | | 7b378237a widened AclMode to 64 bits which resulted in 3 new additional warnings on MSVC. Here we make use of UINT64CONST to reassure the compiler that we do intend the bit shift expression to yield a 64-bit result. Discussion: https://postgr.es/m/CAApHDvo=pn01Y_3zASZZqn+cotF1c4QFCwWgk6MiF0VscaE5ug@mail.gmail.com
* Improve indenting in _hash_pgaddtupDavid Rowley2022-11-25
| | | | | | | | | The Assert added in d09dbeb9b came out rather ugly after having run pgindent on that code. Here we adjust things to use some local variables so that the Assert remains within the 80-character margin. Author: Ted Yu Discussion: https://postgr.es/m/CALte62wLSir1=x93Jf0xZvHaO009FEJfhVMFwnaR8q=csPP8kQ@mail.gmail.com
* Make multixact error message more explicitAlvaro Herrera2022-11-24
| | | | | | | | | | | | | | There are recent reports involving a very old error message that we have no history of hitting -- perhaps a recently introduced bug. Improve the error message in an attempt to improve our chances of investigating the bug. Per reports from Dimos Stamatakis and Bob Krier. Backpatch to 11. Discussion: https://postgr.es/m/CO2PR0801MB2310579F65529380A4E5EDC0E20A9@CO2PR0801MB2310.namprd08.prod.outlook.com Discussion: https://postgr.es/m/17518-04e368df5ad7f2ee@postgresql.org
* Add the database name to the ps display of logical WAL sendersMichael Paquier2022-11-24
| | | | | | | | | | | | | | | | Logical WAL senders display now as follows, gaining a database name: postgres: walsender USER DATABASE HOST(PORT) STATE Physical WAL senders show up the same, as of: postgres: walsender USER HOST(PORT) STATE This information was missing, hence it was not possible to know from ps if a WAL sender was a logical or a physical one, and on which database it is connected when it is logical. Author: Tatsuhiro Nakamori Reviewed-by: Fujii Masao, Bharath Rupireddy Discussion: https://postgr.es/m/36a3b137e82e0ea9fe7e4234f03b64a1@oss.nttdata.com
* Add support for file inclusions in HBA and ident configuration filesMichael Paquier2022-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | pg_hba.conf and pg_ident.conf gain support for three record keywords: - "include", to include a file. - "include_if_exists", to include a file, ignoring it if missing. - "include_dir", to include a directory of files. These are classified by name (C locale, mostly) and need to be prefixed by ".conf", hence following the same rules as GUCs. This commit relies on the refactoring pieces done in efc9816, ad6c528, 783e8c6 and 1b73d0b, adding a small wrapper to build a list of TokenizedAuthLines (tokenize_include_file), and the code is shaped to offer some symmetry with what is done for GUCs with the same options. pg_hba_file_rules and pg_ident_file_mappings gain a new field called file_name, to track from which file a record is located, taking advantage of the addition of rule_number in c591300 to offer an organized view of the HBA or ident records loaded. Bump catalog version. Author: Julien Rouhaud Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Speedup hash index builds by skipping needless binary searchesDavid Rowley2022-11-24
| | | | | | | | | | | | | | | | | When building hash indexes using the spool method, tuples are added to the index page in hashkey order. Because of this, we can safely skip performing the binary search on the existing tuples on the page to find the location to insert the tuple based on its hashkey value. For this case, we can just always put the tuple at the end of the item array as the tuples will always arrive in hashkey order. Testing has shown that this can improve hash index build speeds by 5-15% with a unique set of integer values. Author: Simon Riggs Reviewed-by: David Rowley Tested-by: David Zhang, Tomas Vondra Discussion: https://postgr.es/m/CANbhV-GBc5JoG0AneUGPZZW3o4OK5LjBGeKe_icpC3R1McrZWQ@mail.gmail.com