aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Improve documentation and testing of jsonpath string() for datetimes.Tom Lane2024-09-10
| | | | | | | | | | | | | | | | Point out that the output format depends on DateStyle, and test that, along with testing some cases previously not covered. In passing, adjust the horology test to verify that the prevailing DateStyle is 'Postgres, MDY', much as it has long verified the prevailing TimeZone. We expect pg_regress to have set these up, and there are multiple regression tests relying on these settings. Also make the formatting of entries in table 9.50 more consistent. David Wheeler (marginal additional hacking by me); review by jian he Discussion: https://postgr.es/m/56955B33-6959-4FDA-A459-F00363ECDFEE@justatheory.com
* Protect against small overread in SASLprep validationDaniel Gustafsson2024-09-10
| | | | | | | | | | | | In case of torn UTF8 in the input data we might end up going past the end of the string since we don't account for length. While validation won't be performed on a sequence with a NULL byte it's better to avoid going past the end to beging with. Fix by taking the length into consideration. Author: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAOYmi+mTnmM172g=_+Yvc47hzzeAsYPy2C4UBY3HK9p-AXNV0g@mail.gmail.com
* Add amgettreeheight index AM API routinePeter Eisentraut2024-09-10
| | | | | | | | | | | The only current implementation is for btree where it calls _bt_getrootheight(). Other index types can now also use this to pass information to their amcostestimate routine. Previously, btree was hardcoded and other index types could not hook into the optimizer at this point. Author: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com
* Mark expressions nullable by grouping setsRichard Guo2024-09-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When generating window_pathkeys, distinct_pathkeys, or sort_pathkeys, we failed to realize that the grouping/ordering expressions might be nullable by grouping sets. As a result, we may incorrectly deem that the PathKeys are redundant by EquivalenceClass processing and thus remove them from the pathkeys list. That would lead to wrong results in some cases. To fix this issue, we mark the grouping expressions nullable by grouping sets if that is the case. If the grouping expression is a Var or PlaceHolderVar or constructed from those, we can just add the RT index of the RTE_GROUP RTE to the existing nullingrels field(s); otherwise we have to add a PlaceHolderVar to carry on the nullingrel bit. However, we have to manually remove this nullingrel bit from expressions in various cases where these expressions are logically below the grouping step, such as when we generate groupClause pathkeys for grouping sets, or when we generate PathTarget for initial input to grouping nodes. Furthermore, in set_upper_references, the targetlist and quals of an Agg node should have nullingrels that include the effects of the grouping step, ie they will have nullingrels equal to the input Vars/PHVs' nullingrels plus the nullingrel bit that references the grouping RTE. In order to perform exact nullingrels matches, we also need to manually remove this nullingrel bit. Bump catversion because this changes the querytree produced by the parser. Thanks to Tom Lane for the idea to invent a new kind of RTE. Per reports from Geoff Winkless, Tobias Wendorff, Richard Guo from various threads. Author: Richard Guo Reviewed-by: Ashutosh Bapat, Sutou Kouhei Discussion: https://postgr.es/m/CAMbWs4_dp7e7oTwaiZeBX8+P1rXw4ThkZxh1QG81rhu9Z47VsQ@mail.gmail.com
* Introduce an RTE for the grouping stepRichard Guo2024-09-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If there are subqueries in the grouping expressions, each of these subqueries in the targetlist and HAVING clause is expanded into distinct SubPlan nodes. As a result, only one of these SubPlan nodes would be converted to reference to the grouping key column output by the Agg node; others would have to get evaluated afresh. This is not efficient, and with grouping sets this can cause wrong results issues in cases where they should go to NULL because they are from the wrong grouping set. Furthermore, during re-evaluation, these SubPlan nodes might use nulled column values from grouping sets, which is not correct. This issue is not limited to subqueries. For other types of expressions that are part of grouping items, if they are transformed into another form during preprocessing, they may fail to match lower target items. This can also lead to wrong results with grouping sets. To fix this issue, we introduce a new kind of RTE representing the output of the grouping step, with columns that are the Vars or expressions being grouped on. In the parser, we replace the grouping expressions in the targetlist and HAVING clause with Vars referencing this new RTE, so that the output of the parser directly expresses the semantic requirement that the grouping expressions be gotten from the grouping output rather than computed some other way. In the planner, we first preprocess all the columns of this new RTE and then replace any Vars in the targetlist and HAVING clause that reference this new RTE with the underlying grouping expressions, so that we will have only one instance of a SubPlan node for each subquery contained in the grouping expressions. Bump catversion because this changes the querytree produced by the parser. Thanks to Tom Lane for the idea to invent a new kind of RTE. Per reports from Geoff Winkless, Tobias Wendorff, Richard Guo from various threads. Author: Richard Guo Reviewed-by: Ashutosh Bapat, Sutou Kouhei Discussion: https://postgr.es/m/CAMbWs4_dp7e7oTwaiZeBX8+P1rXw4ThkZxh1QG81rhu9Z47VsQ@mail.gmail.com
* Remove emode argument from XLogFileRead() and XLogFileReadAnyTLI()Michael Paquier2024-09-10
| | | | | | | | | | | | | | This change makes the code slightly easier to reason about, because there is actually no need to know if a specific caller of one of these routines should fail hard on a PANIC, or just let it go through with a DEBUG2. The only caller of XLogFileReadAnyTLI() used DEBUG2, and XLogFileRead() has never used its emode. This can be simplified since 1bb2558046cc that has introduced XLogFileReadAnyTLI(), splitting both. Author: Yugo Nagata Discussion: https://postgr.es/m/20240906201043.a640f3b44e755d4db2b6943e@sraoss.co.jp
* Add WAL usage reporting to ANALYZE VERBOSE output.Masahiko Sawada2024-09-09
| | | | | | | | | | | This change adds WAL usage reporting to the output of ANALYZE VERBOSE and autoanalyze reports. It aligns the analyze output with VACUUM, providing consistency. Additionally, it aids in troubleshooting cases where WAL records are generated during analyze operations. Author: Anthonin Bonnefoy Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA%40mail.gmail.com
* Don't bother checking the result of SPI_connect[_ext] anymore.Tom Lane2024-09-09
| | | | | | | | | | | | | | | | | | | | | | | SPI_connect/SPI_connect_ext have not returned any value other than SPI_OK_CONNECT since commit 1833f1a1c in v10; any errors are thrown via ereport. (The most likely failure is out-of-memory, which has always been thrown that way, so callers had better be prepared for such errors.) This makes it somewhat pointless to check these functions' result, and some callers within our code haven't been bothering; indeed, the only usage example within spi.sgml doesn't bother. So it's likely that the omission has propagated into extensions too. Hence, let's standardize on not checking, and document the return value as historical, while not actually changing these functions' behavior. (The original proposal was to change their return type to "void", but that would needlessly break extensions that are conforming to the old practice.) This saves a small amount of boilerplate code in a lot of places. Stepan Neretin Discussion: https://postgr.es/m/CAMaYL5Z9Uk8cD9qGz9QaZ2UBJFOu7jFx5Mwbznz-1tBbPDQZow@mail.gmail.com
* Add PQfullProtocolVersion() to surface the precise protocol version.Robert Haas2024-09-09
| | | | | | | | | | | | The existing function PQprotocolVersion() does not include the minor version of the protocol. In preparation for pending work that will bump that number for the first time, add a new function to provide it to clients that may care, using the (major * 10000 + minor) convention already used by PQserverVersion(). Jacob Champion based on earlier work by Jelte Fennema-Nio Discussion: http://postgr.es/m/CAOYmi+mM8+6Swt1k7XsLcichJv8xdhPnuNv7-02zJWsezuDL+g@mail.gmail.com
* Fix waits of REINDEX CONCURRENTLY for indexes with predicates or expressionsMichael Paquier2024-09-09
| | | | | | | | | | | | | | | | | | | | | As introduced by f9900df5f94, a REINDEX CONCURRENTLY job done for an index with predicates or expressions would set PROC_IN_SAFE_IC in its MyProc->statusFlags, causing it to be ignored by other concurrent operations. Such concurrent index rebuilds should never be ignored, as a predicate or an expression could call a user-defined function that accesses a different table than the table where the index is rebuilt. A test that uses injection points is added, backpatched down to 17. Michail has proposed a different test, but I have added something simpler with more coverage. Oversight in f9900df5f949. Author: Michail Nikolaev Discussion: https://postgr.es/m/CANtu0oj9A3kZVduFTG0vrmGnKB+DCHgEpzOp0qAyOgmks84j0w@mail.gmail.com Backpatch-through: 14
* SQL/JSON: Avoid initializing unnecessary ON ERROR / ON EMPTY stepsAmit Langote2024-09-09
| | | | | | | | | | | | | | | | | When the ON ERROR / ON EMPTY behavior is to return NULL, returning NULL directly from ExecEvalJsonExprPath() suffices. Therefore, there's no need to create separate steps to check the error/empty flag or those to evaluate the the constant NULL expression. This speeds up common cases because the default ON ERROR / ON EMPTY behavior for JSON_QUERY() and JSON_VALUE() is to return NULL. However, these steps are necessary if the RETURNING type is a domain, as constraints on the domain may need to be checked. Reported-by: Jian He <jian.universality@gmail.com> Author: Jian He <jian.universality@gmail.com> Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* Fix order of parameters in a cost_sort callRichard Guo2024-09-09
| | | | | | | | | | | | | | | | In label_sort_with_costsize, the cost_sort function is called with the parameters 'input_disabled_nodes' and 'input_cost' in the wrong order. This does not cause any plan diffs in the regression tests, because label_sort_with_costsize is only used to label the Sort node nicely for EXPLAIN, and cost numbers are not displayed in regression tests. Oversight in e22253467. Fixed by passing arguments in the right order. Per report from Alexander Lakhin running UBSan. Author: Alexander Lakhin Discussion: https://postgr.es/m/a9b7231d-68bc-f117-a07c-96688f3e6aef@gmail.com
* Add callbacks to control flush of fixed-numbered statsMichael Paquier2024-09-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds two callbacks in pgstats to have a better control of the flush timing of pgstat_report_stat(), whose operation depends on the three PGSTAT_*_INTERVAL variables: - have_fixed_pending_cb(), to check if a stats kind has any pending data waiting for a flush. This is used as a fast path if there are no pending statistics to flush, and this check is done for fixed-numbered statistics only if there are no variable-numbered statistics to flush. A flush will need to happen if at least one callback reports any pending data. - flush_fixed_cb(), to do the actual flush. These callbacks are currently used by the SLRU, WAL and IO statistics, generalizing the concept for all stats kinds (builtin and custom). The SLRU and IO stats relied each on one global variable to determine whether a flush should happen; these are now local to pgstat_slru.c and pgstat_io.c, cleaning up a bit how the pending flush states are tracked in pgstat.c. pgstat_flush_io() and pgstat_flush_wal() are still required, but we do not need to check their return result anymore. Reviewed-by: Bertrand Drouvot, Kyotaro Horiguchi Discussion: https://postgr.es/m/ZtaVO0N-aTwiAk3w@paquier.xyz
* Avoid core dump after getpwuid_r failure.Tom Lane2024-09-08
| | | | | | | | Looking up a nonexistent user ID would lead to a null pointer dereference. That's unlikely to happen here, but perhaps not impossible. Thinko in commit 4d5111b3f, noticed by Coverity.
* Update extension lookup routines to use the syscacheMichael Paquier2024-09-07
| | | | | | | | | | | | | | | | The following routines are changed to use the syscache entries added for pg_extension in 490f869d92e5: - get_extension_oid() - get_extension_name() - get_extension_schema() A catalog scan is costly and could easily lead to a noticeable performance impact when called once or more per query, so this is going to be helpful for developers for extension data lookups. Author: Andrei Lepikhov Reviewed-by: Jelte Fennema-Nio Discussion: https://postgr.es/m/529295b2-6ba9-4dae-acd1-20a9c6fb8f9a@gmail.com
* Remove lc_ctype_is_c().Jeff Davis2024-09-06
| | | | | | | | | | | | Instead always fetch the locale and look at the ctype_is_c field. hba.c relies on regexes working for the C locale without needing catalog access, which worked before due to a special case for C_COLLATION_OID in lc_ctype_is_c(). Move the special case to pg_set_regex_collation() now that lc_ctype_is_c() is gone. Author: Andreas Karlsson Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se
* Fix incorrect pg_stat_io output on 32-bit machines.Tom Lane2024-09-06
| | | | | | | | | | | | | | pg_stat_get_io() applied TimestampTzGetDatum twice to the stat_reset_timestamp value. On 64-bit builds that's harmless because TimestampTzGetDatum is a no-op, but on 32-bit builds it results in displaying garbage in the stats_reset column of the pg_stat_io view. Bug dates to commit a9c70b46d which introduced pg_stat_io, so back-patch to v16 where that came in. Bertrand Drouvot Discussion: https://postgr.es/m/Ztrd+XcPTz1zorkg@ip-10-97-1-34.eu-west-3.compute.internal
* Remove useless unconstifyPeter Eisentraut2024-09-06
| | | | | | Digging into the history, this was not necessary even when it was added, but might have been some time before that. In any case, there is no use for this now.
* SQL/JSON: Fix default ON ERROR behavior for JSON_TABLEAmit Langote2024-09-06
| | | | | | | | | | | | Use EMPTY ARRAY instead of EMPTY. This change does not affect the runtime behavior of JSON_TABLE(), which continues to return an empty relation ON ERROR. It only alters whether the default ON ERROR behavior is shown in the deparsed output. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* SQL/JSON: Fix JSON_TABLE() column deparsingAmit Langote2024-09-06
| | | | | | | | | | | | | | | | The deparsing code in get_json_expr_options() unnecessarily emitted the default column-specific ON ERROR / EMPTY behavior when the top-level ON ERROR behavior in JSON_TABLE was set to ERROR. Fix that by not overriding the column-specific default, determined based on the column's JsonExprOp in get_json_table_columns(), with JSON_BEHAVIOR_ERROR when that is the top-level ON ERROR behavior. Note that this only removes redundancy; the current deparsing output is not incorrect, just redundant. Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* Revert recent SQL/JSON related commitsAmit Langote2024-09-06
| | | | | Reverts 68222851d5a8, 565caaa79af, and 3a97460970f, because a few BF animals didn't like one or all of them.
* SQL/JSON: Avoid initializing unnecessary ON ERROR / ON EMPTY stepsAmit Langote2024-09-06
| | | | | | | | | | | | | | | | | When the ON ERROR / ON EMPTY behavior is to return NULL, returning NULL directly from ExecEvalJsonExprPath() suffices. Therefore, there's no need to create separate steps to check the error/empty flag or those to evaluate the the constant NULL expression. This speeds up common cases because the default ON ERROR / ON EMPTY behavior for JSON_QUERY() and JSON_VALUE() is to return NULL. However, these steps are necessary if the RETURNING type is a domain, as constraints on the domain may need to be checked. Reported-by: Jian He <jian.universality@gmail.com> Author: Jian He <jian.universality@gmail.com> Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* SQL/JSON: Fix default ON ERROR behavior for JSON_TABLEAmit Langote2024-09-06
| | | | | | | | | | | | Use EMPTY ARRAY instead of EMPTY. This change does not affect the runtime behavior of JSON_TABLE(), which continues to return an empty relation ON ERROR. It only alters whether the default ON ERROR behavior is shown in the deparsed output. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* SQL/JSON: Fix JSON_TABLE() column deparsingAmit Langote2024-09-06
| | | | | | | | | | | | | | | | The deparsing code in get_json_expr_options() unnecessarily emitted the default column-specific ON ERROR / EMPTY behavior when the top-level ON ERROR behavior in JSON_TABLE was set to ERROR. Fix that by not overriding the column-specific default, determined based on the column's JsonExprOp in get_json_table_columns(), with JSON_BEHAVIOR_ERROR when that is the top-level ON ERROR behavior. Note that this only removes redundancy; the current deparsing output is not incorrect, just redundant. Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* Update comment about ExprState.escontextAmit Langote2024-09-06
| | | | | | | | | The updated comment provides more helpful guidance by mentioning that escontext should be set when soft error handling is needed. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com Backpatch-through: 17
* Be more careful with error paths in pg_set_regex_collation().Jeff Davis2024-09-05
| | | | | | | | | | | | | | Set global variables after error paths so that they don't end up in an inconsistent state. The inconsistent state doesn't lead to an actual problem, because after an error, pg_set_regex_collation() will be called again before the globals are accessed. Change extracted from patch by Andreas Karlsson, though not discussed explicitly. Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se
* Prevent mis-encoding of "trailing junk after numeric literal" errors.Tom Lane2024-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 2549f0661, we reject an identifier immediately following a numeric literal (without separating whitespace), because that risks ambiguity with hex/octal/binary integers. However, that patch used token patterns like "{integer}{ident_start}", which is problematic because {ident_start} matches only a single byte. If the first character after the integer is a multibyte character, this ends up with flex reporting an error message that includes a partial multibyte character. That can cause assorted bad-encoding problems downstream, both in the report to the client and in the postmaster log file. To fix, use {identifier} not {ident_start} in the "junk" token patterns, so that they will match complete multibyte characters. This seems generally better user experience quite aside from the encoding problem: for "123abc" the error message will now say that the error appeared at or near "123abc" instead of "123a". While at it, add some commentary about why these patterns exist and how they work. Report and patch by Karina Litskevich; review by Pavel Borisov. Back-patch to v15 where the problem came in. Discussion: https://postgr.es/m/CACiT8iZ_diop=0zJ7zuY3BXegJpkKK1Av-PU7xh0EDYHsa5+=g@mail.gmail.com
* Fix handling of NULL return value in typarray lookupDaniel Gustafsson2024-09-05
| | | | | | | | | | | Commit 6ebeeae29 accidentally omitted testing the return value from findTypeByOid which can return NULL. Fix by adding a check to make sure that we have a pointer to dereference. Author: Ranier Vilela <ranier.vf@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAEudQAqfMTH8Ya_J6E-NW_y_JyDFDxtQ4V_g6nY_1=0oDbQqdg@mail.gmail.com
* Fix misleading error message contextPeter Eisentraut2024-09-05
| | | | | | Author: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Stepan Neretin <sncfmgg@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRAw+OkVW=FgMKHKyvY3CgtWy3cWdY7XT+S5TJaTttu=oA@mail.gmail.com
* Add callback for backend initialization in pgstatsMichael Paquier2024-09-05
| | | | | | | | | | pgstat_initialize() is currently used by the WAL stats as a code path to take some custom actions when a backend starts. A callback is added to generalize the concept so as all stats kinds can do the same, for builtin and custom kinds, if set. Reviewed-by: Bertrand Drouvot, Kyotaro Horiguchi Discussion: https://postgr.es/m/ZtZr1K4PLdeWclXY@paquier.xyz
* Fix two NULL pointer dereferences when reading custom pgstats from fileMichael Paquier2024-09-05
| | | | | | | | | | | | | | | | | | | There were two spots in pgstat_read_statsfile() where is was possible to finish with a null-pointer-dereference crash for custom pgstats kinds: - When reading stats for a fixed-numbered stats entry. - When reading a variable stats entry with name serialization. For both cases, these issues were reachable by starting a server after changing shared_preload_libraries so as the stats written previously could not be loaded. The code is changed so as the stats are ignored in this case, like the other code paths doing similar sanity checks. Two WARNINGs are added to be able to debug these issues. A test is added for the case of fixed-numbered stats with the module injection_points. Oversights in 7949d9594582, spotted while looking at a different report. Discussion: https://postgr.es/m/Ztj0Jftsn4xXuXtl@paquier.xyz
* Check availability of module injection_points in TAP testsMichael Paquier2024-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | This fixes defects with installcheck for TAP tests that expect the module injection_points to exist in an installation, but the contents of src/test/modules are not installed by default with installcheck. This would cause, for example, failures under installcheck-world for a build with injection points enabled, when the contents of src/test/modules/ are not installed. The availability of the module can be done with a scan of pg_available_extension. This has been introduced in 2cdcae9da696, and it is refactored here as a new routine in Cluster.pm. Tests are changed in different ways depending on what they need: - The libpq TAP test sets up a node even without injection points, so it is enough to check that CREATE EXTENSION can be used. There is no need for the variable enable_injection_points. - In test_misc, 006_signal_autovacuum requires a runtime check. - 041_checkpoint_at_promote in recovery tests and 005_timeouts in test_misc are updated to use the routine introduced in Cluster.pm. - test_slru's 001_multixact, injection_points's 001_stats and modules/gin/ do not require a check as these modules disable installcheck entirely. Discussion: https://postgr.es/m/ZtesYQ-WupeAK7xK@paquier.xyz
* Optimize WindowAgg's use of tuplestoresDavid Rowley2024-09-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When WindowAgg finished one partition of a PARTITION BY, it previously would call tuplestore_end() to purge all the stored tuples before again calling tuplestore_begin_heap() and carefully setting up all of the tuplestore read pointers exactly as required for the given frameOptions. Since the frameOptions don't change between partitions, this part does not make much sense. For queries that had very few rows per partition, the overhead of this was very large. It seems much better to create the tuplestore and the read pointers once and simply call tuplestore_clear() at the end of each partition. tuplestore_clear() moves all of the read pointers back to the start position and deletes all the previously stored tuples. A simple test query with 1 million partitions and 1 tuple per partition has been shown to run around 40% faster than without this change. The additional effort seems to have mostly been spent in malloc/free. Making this work required adding a new bool field to WindowAggState which had the unfortunate effect of being the 9th bool field in a group resulting in the struct being enlarged. Here we shuffle the fields around a little so that the two bool fields for runcondition relating stuff fit into existing padding. Also, move the "runcondition" field to be near those. This frees up enough space with the other bool fields so that the newly added one fits into the padding bytes. This was done to address a very small but apparent performance regression with queries containing a large number of rows per partition. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Tatsuo Ishii <ishii@postgresql.org> Discussion: https://postgr.es/m/CAHoyFK9n-QCXKTUWT_xxtXninSMEv%2BgbJN66-y6prM3f4WkEHw%40mail.gmail.com
* Speedup WindowAgg code by moving uncommon code out-of-lineDavid Rowley2024-09-05
| | | | | | | | | | The code to calculate the frame offsets is only performed once per scan. Moving this code out of line gives a small (around 4-5%) speedup when testing with some CPUs. Other tested CPUs are indifferent to the change. Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Tatsuo Ishii <ishii@postgresql.org> Discussion: https://postgr.es/m/CAApHDvqPgFtwme2Zyf75BpMLwYr2mnUstDyPiP%3DEpudYuQTPPQ%40mail.gmail.com
* Remove lc_collate_is_c().Jeff Davis2024-09-04
| | | | | | | Instead just look up the collation and check collate_is_c field. Author: Andreas Karlsson Discussion: https://postgr.es/m/60929555-4709-40a7-b136-bcb44cff5a3c@proxel.se
* Remove test-case workarounds for ancient libedit versions.Tom Lane2024-09-04
| | | | | | | | | | | | | | | | | This reverts some hacks added in d33a81203 and cd69ec66c. At the time the concern was the already-ancient version of libedit shipped in Debian 10 (Buster). That platform is now two years past EOL, so desupporting it for PG 18 seems fine. (Also, if anyone is really hot to keep testing it, they can use SKIP_READLINE_TESTS to skip this test.) We might have to reconsider if any animals still in the buildfarm don't like this, but the best way to find out is to try it. Anton Melnikov Discussion: https://postgr.es/m/CAGRrpzZU48F2oV3d8eDLr=4TU9xFH5Jt9ED+qU1+X91gMH68Sw@mail.gmail.com
* Remove a couple of strerror() callsPeter Eisentraut2024-09-04
| | | | | | | | | | | | Change to using %m in the error message string. We need to be a bit careful here to preserve errno until we need to print it. This change avoids the use of not-thread-safe strerror() and unifies some error message strings, and maybe makes the code appear more consistent. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/daa87d79-c044-46c4-8458-8d77241ed7b0%40eisentraut.org
* Unify some error messages to ease work of translatorsMichael Paquier2024-09-04
| | | | | | | | | This commit updates a couple of error messages around control file data, GUCs and server settings, unifying to the same message where possible. This reduces the translation burden a bit. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Apply more quoting to GUC names in messagesMichael Paquier2024-09-04
| | | | | | | | | This is a continuation of 17974ec25946. More quotes are applied to GUC names in error messages and hints, taking care of what seems to be all the remaining holes currently in the tree for the GUCs. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Collect statistics about conflicts in logical replication.Amit Kapila2024-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds columns in view pg_stat_subscription_stats to show the number of times a particular conflict type has occurred during the application of logical replication changes. The following columns are added: confl_insert_exists: Number of times a row insertion violated a NOT DEFERRABLE unique constraint. confl_update_origin_differs: Number of times an update was performed on a row that was previously modified by another origin. confl_update_exists: Number of times that the updated value of a row violates a NOT DEFERRABLE unique constraint. confl_update_missing: Number of times that the tuple to be updated is missing. confl_delete_origin_differs: Number of times a delete was performed on a row that was previously modified by another origin. confl_delete_missing: Number of times that the tuple to be deleted is missing. The update_origin_differs and delete_origin_differs conflicts can be detected only when track_commit_timestamp is enabled. Author: Hou Zhijie Reviewed-by: Shveta Malik, Peter Smith, Anit Kapila Discussion: https://postgr.es/m/OS0PR01MB57160A07BD575773045FC214948F2@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Avoid unnecessary post-sort projectionRichard Guo2024-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When generating paths for the ORDER BY clause, one thing we need to ensure is that the output paths project the correct final_target. To achieve this, in create_ordered_paths, we compare the pathtarget of each generated path with the given 'target', and add a post-sort projection step if the two targets do not match. Currently we perform a simple pointer comparison between the two targets. It turns out that this is not sufficient. Each sorted_path generated in create_ordered_paths initially projects the correct target required by the preceding steps of sort. If it is the same pointer as sort_input_target, pointer comparison suffices, because sort_input_target is always identical to final_target when no post-sort projection is needed. However, sorted_path's initial pathtarget may not be the same pointer as sort_input_target, because in apply_scanjoin_target_to_paths, if the target to be applied has the same expressions as the existing reltarget, we only inject the sortgroupref info into the existing pathtargets, rather than create new projection paths. As a result, pointer comparison in create_ordered_paths is not reliable. Instead, we can compare PathTarget.exprs to determine whether a projection step is needed. If the expressions match, we can be confident that a post-sort projection is not required. It could be argued that this change adds extra check cost each time we decide whether a post-sort projection is needed. However, as explained in apply_scanjoin_target_to_paths, by avoiding the creation of projection paths, we save effort both immediately and at plan creation time. This, I think, justifies the extra check cost. There are two ensuing plan changes in the regression tests, but they look reasonable and are exactly what we are fixing here. So no additional test cases are added. No backpatch as this could result in plan changes. Author: Richard Guo Reviewed-by: Peter Eisentraut, David Rowley, Tom Lane Discussion: https://postgr.es/m/CAMbWs48TosSvmnz88663_2yg3hfeOFss-J2PtnENDH6J_rLnRQ@mail.gmail.com
* Check the validity of commutators for merge/hash clausesRichard Guo2024-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When creating merge or hash join plans in createplan.c, the merge or hash clauses may need to get commuted to ensure that the outer var is on the left and the inner var is on the right if they are not already in the expected form. This requires that their operators have commutators. Failing to find a commutator at this stage would result in 'ERROR: could not find commutator for operator xxx', with no opportunity to select an alternative plan. Typically, this is not an issue because mergejoinable or hashable operators are expected to always have valid commutators. But in some artificial cases this assumption may not hold true. Therefore, here in this patch we check the validity of commutators for clauses in the form "inner op outer" when selecting mergejoin/hash clauses, and consider a clause unusable for the current pair of outer and inner relations if it lacks a commutator. There are not (and should not be) any such operators built into Postgres that are mergejoinable or hashable but have no commutators; so we leverage the alias type 'int8alias1' created in equivclass.sql to build the test case. This is why the test case is included in equivclass.sql rather than in join.sql. Although this is arguably a bug fix, it cannot be reproduced without installing an incomplete opclass, which is unlikely to happen in practice, so no back-patch. Reported-by: Alexander Pyhalov Author: Richard Guo Reviewed-by: Tom Lane Discussion: https://postgr.es/m/c59ec04a2fef94d9ffc35a9b17dfc081@postgrespro.ru
* Fix inconsistent LWLock tranche name "CommitTsSLRU"Michael Paquier2024-09-04
| | | | | | | | | | | | | | | | This term was using an inconsistent casing between the code and the documentation, using "CommitTsSLRU" in wait_event_names.txt and "CommitTSSLRU" in the code. Let's update the term in the code to reflect what's in the documentation, "CommitTs" being more commonly used, so as pg_stat_activity shows the same term as the documentation. Oversight in 53c2a97a9266. Author: Alexander Lakhin Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com Backpatch-through: 17
* Avoid installcheck failure in TAP tests using injection_pointsMichael Paquier2024-09-04
| | | | | | | | | | | | | | | | | | These tests depend on the test module injection_points to be installed, but it may not be available as the contents of src/test/modules/ are not installed by default. This commit adds a workaround based on a scan of pg_available_extensions to check if the extension is available, skipping the test if it is not. This allows installcheck to work transparently. There are more tests impacted by this problem on HEAD, but for now this addresses only the tests that exist on HEAD and v17 as the release is close by. Reported-by: Maxim Orlov Discussion: https://postgr.es/m/CACG=ezZkoT-pFz6a9XnyToiuR-Wg8fGELqHLoyBodr+2h-77qA@mail.gmail.com Backpatch-through: 17
* Remember last collation to speed up collation cache.Jeff Davis2024-09-03
| | | | | | | | | This optimization is to avoid a performance regression in an upcoming patch that will remove lc_collate_is_c(). Discussion: https://postgr.es/m/96a559be83329bc66074a3925ebcfa8ceb16dfc5.camel@j-davis.com Discussion: https://postgr.es/m/646f662e145ab38cff1c04d475f4448f53fc5042.camel@j-davis.com Discussion: https://postgr.es/m/54565933-d82f-4d7c-8f47-288b1b570fd8@eisentraut.org
* Simplify makefiles exporting twice enable_injection_pointsMichael Paquier2024-09-04
| | | | | | | | | This is confusing, as it exports twice the same variable. Oversight in 6782709df81f that has spread in more places afterwards. Reported-by: Alvaro Herrera, Tom Lane Discussion: https://postgr.es/m/202408201630.mn6vbohjh7hh@alvherre.pgsql Backpatch-through: 17
* Standardize "read-ahead advice" terminology.Thomas Munro2024-09-04
| | | | | | | | | Commit 6654bb920 added macOS's equivalent of POSIX_FADV_WILLNEED, and changed some explicit references to posix_fadvise to use this more general name for the concept. Update some remaining references. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/0827edec-1317-4917-a186-035eb1e3241d%40eisentraut.org
* Add block_range_read_stream_cb(), to deduplicate code.Noah Misch2024-09-03
| | | | | | | | | This replaces two functions for iterating over all blocks in a range. A pending patch will use this instead of adding a third. Nazir Bilal Yavuz Discussion: https://postgr.es/m/20240820184742.f2.nmisch@google.com
* Use library functions to edit config in SSL testsDaniel Gustafsson2024-09-03
| | | | | | | | | The SSL tests were editing the postgres configuration by directly reading and writing the files rather than using append_conf() from the testcode library. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/01F4684C-8C98-4BBE-AB83-AC8D7C746AF8@yesql.se
* Test for PG_TEST_EXTRA separately in SSL testsDaniel Gustafsson2024-09-03
| | | | | | | | | PG_TEST_EXTRA is an override and should be tested for separately from any other test as there is no dependency on whether OpenSSL is available or not. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/01F4684C-8C98-4BBE-AB83-AC8D7C746AF8@yesql.se