aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Avoid including explain.h in explain_format.h and explain_dr.hRobert Haas2025-02-28
| | | | | | | | | | | As per a suggestion from Tom Lane, we do this by declaring "struct ExplainState" here and refer to that rather than "ExplainState". Also per Tom, CreateExplainSerializeDestReceiver was still defined in explain.h in addition to explain_dr.h. Remove leftover prototype. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: http://postgr.es/m/CA+TgmoYtaad3i21V0jqua-fbr+CR0ix6uBvEX8_s6BG96abd=g@mail.gmail.com
* Fix missing space in EXPLAIN ANALYZE output.Robert Haas2025-02-28
| | | | | | | | | | | Commit ddb17e387aa28d61521227377b00f997756b8a27 introduced this regression. Ideally, the regression tests would have caught this mistake, but apparently they don't test with timing enabled, presumably because that would make the output vary. Author: Thom Brown <thom@linux.com> Reviewed-by: Fabrízio de Royes Mello <fabriziomello@gmail.com> Discussion: http://postgr.es/m/CAA-aLv6nq=UeiyvM7_Mxgo9TVBzs2oh46b9vfyLzuyVEz3j1-g@mail.gmail.com
* Adjust pg_dump tag for relation stats.Jeff Davis2025-02-27
| | | | Do not use fmtId(), just use dobj->name directly, like for table data.
* Invent pgstat_fetch_stat_backend_by_pid()Michael Paquier2025-02-28
| | | | | | | | | | | | | | | This code is extracted from pg_stat_get_backend_io() in pgstatfuncs.c, so as it can be shared with other areas that need backend pgstats entries while having the benefits of the various sanity checks refactored here. As per its name, this retrieves backend statistics based on a PID, with the option of retrieving a BackendType if given in input. Currently, this is used for the backend-level IO statistics. The next move would be to reuse that for the backend-level WAL statistics. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/Z3zqc4o09dM/Ezyz@ip-10-97-1-34.eu-west-3.compute.internal
* pg_upgrade: Fix inconsistency in memory freeingMichael Paquier2025-02-28
| | | | | | | | | | | | | | | | | | | | The function in charge of freeing the memory from a result created by PQescapeIdentifier() has to be PQfreemem(), to ensure that both allocation and free come from libpq. One spot in pg_upgrade was not respecting that for pg_database's datlocale (daticulocale in v16) when the collation provider is libc (aka datlocale/daticulocale is NULL) with an allocation done using pg_strdup() and a free with PQfreemem(). The code is changed to always use PQescapeLiteral() when processing the input. Oversight in 9637badd9f92. This commit is similar to 48e4ae9a0707 and 5b94e2753439. Author: Michael Paquier <michael@paquier.xyz> Co-authored-by: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://postgr.es/m/Z601RQxTmIUohdkV@paquier.xyz Backpatch-through: 16
* Refactor COPY TO to use format callback functions.Masahiko Sawada2025-02-27
| | | | | | | | | | | | | | | | | | | | | | | | This commit introduces a new CopyToRoutine struct, which is a set of callback routines to copy tuples in a specific format. It also makes the existing formats (text, CSV, and binary) utilize these format callbacks. This change is a preliminary step towards making the COPY TO command extensible in terms of output formats. Additionally, this refactoring contributes to a performance improvement by reducing the number of "if" branches that need to be checked on a per-row basis when sending field representations in text or CSV mode. The performance benchmark results showed ~5% performance gain in text or CSV mode. Author: Sutou Kouhei <kou@clear-code.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com
* Create explain_dr.c and move DestReceiver-related code there.Robert Haas2025-02-27
| | | | | | | | | explain.c has grown rather large, and the code that deals with the DestReceiver that supports the SERIALIZE option is pretty easily severable from the rest of explain.c; hence, move it to a separate file. Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: http://postgr.es/m/CA+TgmoYutMw1Jgo8BWUmB3TqnOhsEAJiYO=rOQufF4gPLWmkLQ@mail.gmail.com
* Create explain_format.c and move relevant code there.Robert Haas2025-02-27
| | | | | | | | | | explain.c has grown rather large, so move various functions that are principally concerned with output generation to a new source file, explain_format.c, instead of lumping them in with everything else that is part of explain.c Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: http://postgr.es/m/CA+TgmoYutMw1Jgo8BWUmB3TqnOhsEAJiYO=rOQufF4gPLWmkLQ@mail.gmail.com
* EXPLAIN: Always use two fractional digits for row counts.Robert Haas2025-02-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit ddb17e387aa28d61521227377b00f997756b8a27 attempted to avoid confusing users by displaying digits after the decimal point only when nloops > 1, since it's impossible to have a fraction row count after a single iteration. However, this made the regression tests unstable since parallal queries will have nloops>1 for all nodes below the Gather or Gather Merge in normal cases, but if the workers don't start in time and the leader finishes all the work, they will suddenly have nloops==1, making it unpredictable whether the digits after the decimal point would be displayed or not. Although 44cbba9a7f51a3888d5087fc94b23614ba2b81f2 seemed to fix the immediate failures, it may still be the case that there are lower-probability failures elsewhere in the regression tests. Various fixes are possible here. For example, it has previously been proposed that we should try to display the digits after the decimal point only if rows/nloops is an integer, but currently rows is storead as a float so it's not theoretically an exact quantity -- precision could be lost in extreme cases. It has also been proposed that we should try to display the digits after the decimal point only if we're under some sort of construct that could potentially cause looping regardless of whether it actually does. While such ideas are not without merit, this patch adopts the much simpler solution of always display two decimal digits. If that approach stands up to scrutiny from the buildfarm and human users, it spares us the trouble of doing anything more complex; if not, we can reassess. This commit incidentally reverts 44cbba9a7f51a3888d5087fc94b23614ba2b81f2, which should no longer be needed. Author: Robert Haas <robertmhaas@gmail.com> Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com> Discussion: http://postgr.es/m/CA+TgmoazzVHn8sFOMFAEwoqBTDxKT45D7mvkyeHgqtoD2cn58Q@mail.gmail.com
* Generalize hash and ordering support in amapiPeter Eisentraut2025-02-27
| | | | | | | | | | | | Stop comparing access method OID values against HASH_AM_OID and BTREE_AM_OID, and instead check the IndexAmRoutine for an index to see if it advertises its ability to perform the necessary ordering, hashing, or cross-type comparing functionality. A field amcanorder already existed, this uses it more widely. Fields amcanhash and amcancrosscompare are added for the other purposes. Author: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com
* Avoid unnecessary computation of pgbench's script line number.Tom Lane2025-02-27
| | | | | | | | | | | | | | | | | | | | | | | | | | ParseScript only needs the lineno for meta-commands, so let's not bother computing it otherwise. While this doesn't save much given the previous patch, there's no point in doing unnecessary work. While we're at it, avoid calling psql_scan_get_location() twice for a meta-command. One reason for making this change is that the line number computed in ParseScript's main loop was actually wrong in most cases: it would point just past the semicolon of the previous SQL command, not at what the user thinks the current command's line number is. We could add some code to skip whitespace before capturing the line number, but it would be pretty pointless at present. Just move the call to avoid the temptation to rely on that value. (Once we've lexed the backslash, the computed line number will be right.) This change also means that pgbench never inquires about the location before it's lexed something, so that the care taken in the previous patch to behave sanely in that case is unnecessary. It seems best to keep that logic, though, as future callers might depend on it. Author: Daniel Vérité <daniel@manitou-mail.org> Discussion: https://postgr.es/m/84a8a89e-adb8-47a9-9d34-c13f7150ee45@manitou-mail.org
* Get rid of O(N^2) script-parsing overhead in pgbench.Tom Lane2025-02-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pgbench wants to record the starting line number of each command in its scripts. It was computing that by scanning from the script start and counting newlines, so that O(N^2) work had to be done for an N-command script. In a script with 50K lines, this adds up to about 10 seconds on my machine. To add insult to injury, the results were subtly wrong, because expr_scanner_offset() scanned to find the NUL that flex inserts at the end of the current token --- and before the first yylex call, no such NUL has been inserted. So we ended by computing the script's last line number not its first one. This was visible only in case of \gset at the start of a script, which perhaps accounts for the lack of complaints. To fix, steal an idea from plpgsql and track the current lexer ending position and line count as we advance through the script. (It's a bit simpler than plpgsql since we can't need to back up.) Also adjust a couple of other places that were invoking scans from script start when they didn't really need to. I made a new psqlscan function psql_scan_get_location() that replaces both expr_scanner_offset() and expr_scanner_get_lineno(), since in practice expr_scanner_get_lineno() was only being invoked to find the line number of the current lexer end position. Reported-by: Daniel Vérité <daniel@manitou-mail.org> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/84a8a89e-adb8-47a9-9d34-c13f7150ee45@manitou-mail.org
* Get rid of ojrelid local variable in remove_rel_from_query()Alexander Korotkov2025-02-27
| | | | | | | | | | | | As spotted by Coverity, the calculation of ojrelid mixes signed and unsigned types causes possible overflow and undefined behavior. Instead of trying to fix the expression, this commit eliminates the relied local variable. The explicit branching is used to replace the -1 value. That, in turn, requires changing the signature of the remove_rel_from_eclass() function. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/914330.1740330169%40sss.pgh.pa.us Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
* Remove arbitrary cap on read_stream.c buffer queue.Thomas Munro2025-02-27
| | | | | | | | | | | | | | | | | | | | | | | Previously the internal queue of buffers was capped at max_ios * 4, though not less than io_combine_limit, at allocation time. That was done in the first version based on conservative theories about resource usage and heuristics pending later work. The configured I/O depth could not always be reached with dense random streams generated by ANALYZE, VACUUM, the proposed Bitmap Heap Scan patch, and also sequential streams with the proposed AIO subsystem to name some examples. The new formula is (max_ios + 1) * io_combine_limit, enough buffers for the full configured I/O concurrency level using the full configured I/O combine size, plus the buffers from one finished but not yet consumed full-sized I/O. Significantly more memory would be needed for high GUC values if the client code requests a large per-buffer data size, but that is discouraged (existing and proposed stream users try to keep it under a few words, if not zero). With this new formula, an intermediate variable could have overflowed under maximum GUC values, so its data type is adjusted to cope. Discussion: https://postgr.es/m/CA%2BhUKGK_%3D4CVmMHvsHjOVrK6t4F%3DLBpFzsrr3R%2BaJYN8kcTfWg%40mail.gmail.com
* pg_amcheck: Fix inconsistency in memory freeingMichael Paquier2025-02-27
| | | | | | | | | | | | | | | The function in charge of freeing the memory from a result created by PQescapeIdentifier() has to be PQfreemem(), to ensure that both allocation and free come from libpq, but one spot in pg_amcheck was missing that. Oversight in b859d94c6389. Author: Ranier Vilela <ranier.vf@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Discussion: https://postgr.es/m/CAEudQArD_nKSnYCNUZiPPsJ2tNXgRmLbXGSOrH1vpOF_XtP0Vg@mail.gmail.com Discussion: https://postgr.es/m/CAEudQArbTWVSbxq608GRmXJjnNSQ0B6R7CSffNnj2hPWMUsRNg@mail.gmail.com Backpatch-through: 14
* Fix the race condition in ReplicationSlotAcquire().Amit Kapila2025-02-27
| | | | | | | | | | | | | | | | | After commit f41d8468dd, a process could acquire and use a replication slot that had just been invalidated, leading to failures while accessing WAL. To ensure that we don't accidentally start using invalid slots, we must perform the invalidation check after acquiring the slot or under the spinlock where we associate the slot with a particular process. We choose the earlier method to keep the code simple. Reported-by: Hou Zhijie <houzj.fnst@fujitsu.com> Author: Nisha Moond <nisha.moond412@gmail.com> Reviewed-by: Hou Zhijie <houzj.fnst@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CABdArM7J-LbGoMPGUPiFiLOyB_TZ5+YaZb=HMES0mQqzVTn8Gg@mail.gmail.com
* Refactor code of pg_stat_get_wal() building result tupleMichael Paquier2025-02-27
| | | | | | | | | | | | | | | This commit adds to pgstatfuncs.c a new routine called pg_stat_wal_build_tuple(), helper routine for pg_stat_get_wal(). This is in charge of filling one tuple based on the contents of PgStat_WalStats retrieved from pgstats. This refactoring will be used by an upcoming patch introducing backend-level WAL statistics, simplifying the main patch. Note that it is not possible for stats_reset to be NULL in pg_stat_wal; backend statistics need to be able to handle this case. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/Z3zqc4o09dM/Ezyz@ip-10-97-1-34.eu-west-3.compute.internal
* Fix possible double-release of spinlock in procsignal.cMichael Paquier2025-02-27
| | | | | | | | | | | | | | | | | | | | | | | 9d9b9d46f3c5 has added spinlocks to protect the fields in ProcSignal flags, introducing a code path in ProcSignalInit() where a spinlock could be released twice if the pss_pid field of a ProcSignalSlot is found as already set. Multiple spinlock releases have no effect with most spinlock implementations, but this could cause the code to run into issues when the spinlock is acquired concurrently by a different process. This sanity check on pss_pid generates a LOG that can be delayed until after the spinlock is released as, like older versions up to v17, the code expects the initialization of the ProcSignalSlot to happen even if pss_pid is found incorrect. The code is changed so as the old pss_pid is read while holding the slot's spinlock, with the LOG from the sanity check generated after releasing the spinlock, preventing the double release. Author: Maksim Melnikov <m.melnikov@postgrespro.ru> Co-authored-by: Maxim Orlov <orlovmg@gmail.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://postgr.es/m/dca47527-2d8b-4e3b-b5a0-e2deb73371a4@postgrespro.ru
* Remove stray diff introduced by a5cbdeb98a.Jeff Davis2025-02-26
| | | | | Reported-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/Z77IkjmmfbFfNh3f@paquier.xyz
* Use attnum to identify index columns in pg_restore_attribute_stats().Tom Lane2025-02-26
| | | | | | | | | | | | | | | | | | | | Previously we used attname for both table and index columns, but that is problematic for indexes because their attnames are assigned by internal rules that don't guarantee to preserve the names across dump and reload. (This is what's causing the remaining buildfarm failures in cross-version-upgrade tests.) Fortunately we can use attnum instead, since there's no such thing as adding or dropping columns in an existing index. We met this same problem previously with ALTER INDEX ... SET STATISTICS, and solved it the same way, cf commit 5b6d13eec. In pg_restore_attribute_stats() itself, we accept either attnum or attname, but the policy used by pg_dump is to always use attname for tables and attnum for indexes. Author: Tom Lane <tgl@sss.pgh.pa.us> Author: Corey Huinker <corey.huinker@gmail.com> Discussion: https://postgr.es/m/1457469.1740419458@sss.pgh.pa.us
* Revert "Prepare for Python "Limited API" in PL/Python"Peter Eisentraut2025-02-26
| | | | | | | | This reverts commit c47e8df815c1c45f4e4fc90d5817d67ab088279f. That commit makes the plpython tests crash with Python 3.6.* and 3.7.*. It will need further investigation and testing, so revert for now.
* Fix a typo in 005_char_signedness.pl test.Masahiko Sawada2025-02-26
| | | | | | | | | | | | The test in 005_char_signedness.pl was missing a dash in the --set-char-signedness option. Although the test didn't fail since it doesn't check the error message, it resulted in an unexpected error message instead of the intended one. Oversight in 1aab680591. Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://postgr.es/m/87tt8h1vb7.fsf@wibble.ilmari.org
* Prepare for Python "Limited API" in PL/PythonPeter Eisentraut2025-02-26
| | | | | | | | | | | | | | | | | | | Using the Python Limited API would allow building PL/Python against any Python 3.x version and using another Python 3.x version at run time. This commit does not activate that, but it prepares the code to only use APIs supported by the Limited API. Implementation details: - Convert static types to heap types (https://docs.python.org/3/howto/isolating-extensions.html#heap-types). - Replace PyRun_String() with component functions. - Replace PyList_SET_ITEM() with PyList_SetItem(). Reviewed-by: Jakob Egger <jakob@eggerapps.at> Discussion: https://www.postgresql.org/message-id/flat/ee410de1-1e0b-4770-b125-eeefd4726a24@eisentraut.org
* Adding new PgStat_WalCounters structure in pgstat.hMichael Paquier2025-02-26
| | | | | | | | | | | | | | | This new structure contains the counters and the data related to the WAL activity statistics gathered from WalUsage, separated into its own structure so as it can be shared across more than one Stats structure in pg_stat.h. This refactoring will be used by an upcoming patch introducing backend-level WAL statistics. Bump PGSTAT_FILE_FORMAT_ID. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/Z3zqc4o09dM/Ezyz@ip-10-97-1-34.eu-west-3.compute.internal
* Remove pgstat_flush_wal()Michael Paquier2025-02-26
| | | | | | | | | | | | | All the processes that generate WAL should call pgstat_report_wal() to report all their statistics related to WAL, and this is already what happens in the tree. Keeping pgstat_report_wal() is confusing while the other routine is encouraged. This routine is not required since fc415edf8ca8, where it was lastly used in pgstat_report_stat() before an equivalent callback existed. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/Z71oPkJJICrRB5Ws@paquier.xyz
* Add two-phase option in pg_createsubscriber.Amit Kapila2025-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | This patch introduces the '--enable-two-phase' option to the 'pg_createsubscriber' utility, allowing users to enable two-phase commit for all subscriptions during their creation. Note that even without this option users can enable the two_phase option for the subscriptions created by pg_createsubscriber. However, it requires the subscription to be disabled first which could be inconvenient for users. When two-phase commit is enabled, prepared transactions are sent to the subscriber at the time of 'PREPARE TRANSACTION', and they are processed as two-phase transactions on the subscriber as well. If disabled, prepared transactions are sent only when committed and are processed immediately by the subscriber. Author: Shubham Khanna <khannashubham1197@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Ajin Cherian <itsajin@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAHv8RjLPdFP=kA5LNSmWZ=+GMXmO+LczvV6p9HJjsXxZz10KGA@mail.gmail.com
* Improve FATAL message for invalid TLI history at recoveryMichael Paquier2025-02-26
| | | | | | | | | | | | | | The original message did not mention where the checkpoint record LSN was found, a control file or a backup_label file. A couple of LOG messages are generated before this FATAL check is reached, providing more details about the way recovery is set up. However, knowing this information in this specific message is useful for debugging. This is also useful for instances where log_min_messages is set to FATAL or more, where LOG messages do not show up. Author: Benoit Lobréau <benoit.lobreau@dalibo.com> Reviewed-by: David Steele <david@pgbackrest.org> Discussion: https://postgr.es/m/4ed10bc8-5513-4d8e-8643-8abcaa08336d@dalibo.com
* pg_dump: prepare attribute stats query.Jeff Davis2025-02-25
| | | | | | | | | | | Follow precedent in pg_dump for preparing queries to improve performance. Also, simplify the query by removing unnecessary joins. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Andres Freund <andres@anarazel.de> Co-authored-by: Corey Huinker <corey.huinker@gmail.com> Co-authored-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/CADkLM=dRMC6t8gp9GVf6y6E_r5EChQjMAAh_vPyih_zMiq0zvA@mail.gmail.com
* Avoid unnecessary relation stats query in pg_dump.Jeff Davis2025-02-25
| | | | | | | | | | | The few fields we need can be easily collected in getTables() and getIndexes() and stored in RelStatsInfo. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Andres Freund <andres@anarazel.de> Co-authored-by: Corey Huinker <corey.huinker@gmail.com> Co-authored-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/CADkLM=f0a43aTd88xW4xCFayEF25g-7hTrHX_WhV40HyocsUGg@mail.gmail.com
* Re-add GUC track_wal_io_timingMichael Paquier2025-02-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit is a rework of 2421e9a51d20, about which Andres Freund has raised some concerns as it is valuable to have both track_io_timing and track_wal_io_timing in some cases, as the WAL write and fsync paths can be a major bottleneck for some workloads. Hence, it can be relevant to not calculate the WAL timings in environments where pg_test_timing performs poorly while capturing some IO data under track_io_timing for the non-WAL IO paths. The opposite can be also true: it should be possible to disable the non-WAL timings and enable the WAL timings (the previous GUC setups allowed this possibility). track_wal_io_timing is added back in this commit, controlling if WAL timings should be calculated in pg_stat_io for the read, fsync and write paths, as done previously with pg_stat_wal. pg_stat_wal previously tracked only the sync and write parts (now removed), read stats is new data tracked in pg_stat_io, all three are aggregated if track_wal_io_timing is enabled. The read part matters during recovery or if a XLogReader is used. Extra note: more control over if the types of timings calculated in pg_stat_io could be done with a GUC that lists pairs of (IOObject,IOOp). Reported-by: Andres Freund <andres@anarazel.de> Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Co-authored-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/3opf2wh2oljco6ldyqf7ukabw3jijnnhno6fjb4mlu6civ5h24@fcwmhsgmlmzu
* Remove redundant pg_set_*_stats() variants.Jeff Davis2025-02-25
| | | | | | | | | | | | | After commit f3dae2ae58, the primary purpose of separating the pg_set_*_stats() from the pg_restore_*_stats() variants was eliminated. Leave pg_restore_relation_stats() and pg_restore_attribute_stats(), which satisfy both purposes, and remove pg_set_relation_stats() and pg_set_attribute_stats(). Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://postgr.es/m/1457469.1740419458@sss.pgh.pa.us
* Change _mdfd_segpath() to return paths by valueAndres Freund2025-02-25
| | | | | | | | | This basically mirrors the changes done in the predecessor commit. While there isn't currently a need to get these paths in critical sections, it seems a shame to unnecessarily allocate memory in these paths now that relpath() doesn't allocate anymore. Discussion: https://postgr.es/m/xeri5mla4b5syjd5a25nok5iez2kr3bm26j2qn4u7okzof2bmf@kwdh2vf7npra
* Change relpath() et al to return path by valueAndres Freund2025-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | For AIO, and also some other recent patches, we need the ability to call relpath() in a critical section. Until now that was not feasible, as it allocated memory. The fact that relpath() allocated memory also made it awkward to use in log messages because we had to take care to free the memory afterwards. Which we e.g. didn't do for when zeroing out an invalid buffer. We discussed other solutions, e.g. filling a pre-allocated buffer that's passed to relpath(), but they all came with plenty downsides or were larger projects. The easiest fix seems to be to make relpath() return the path by value. To be able to return the path by value we need to determine the maximum length of a relation path. This patch adds a long #define that computes the exact maximum, which is verified to be correct in a regression test. As this change the signature of relpath(), extensions using it will need to adapt their code. We discussed leaving a backward-compat shim in place, but decided it's not worth it given the use of relpath() doesn't seem widespread. Discussion: https://postgr.es/m/xeri5mla4b5syjd5a25nok5iez2kr3bm26j2qn4u7okzof2bmf@kwdh2vf7npra
* Remove obsolete Python version checkPeter Eisentraut2025-02-25
| | | | | | | The checked version is already the current minimum supported version (3.2). Discussion: https://www.postgresql.org/message-id/flat/ee410de1-1e0b-4770-b125-eeefd4726a24@eisentraut.org
* Eliminate code duplication in replace_rte_variables callbacksRichard Guo2025-02-25
| | | | | | | | | | | | | | | | | | The callback functions ReplaceVarsFromTargetList_callback and pullup_replace_vars_callback are both used to replace Vars in an expression tree that reference a particular RTE with items from a targetlist, and they both need to expand whole-tuple references and deal with OLD/NEW RETURNING list Vars. As a result, currently there is significant code duplication between these two functions. This patch introduces a new function, ReplaceVarFromTargetList, to perform the replacement and calls it from both callback functions, thereby eliminating code duplication. Author: Dean Rasheed <dean.a.rasheed@gmail.com> Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CAEZATCWhr=FM4X5kCPvVs-g2XEk+ceLsNtBK_zZMkqFn9vUjsw@mail.gmail.com
* Expand virtual generated columns in the plannerRichard Guo2025-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 83ea6c540 added support for virtual generated columns that are computed on read. All Var nodes in the query that reference virtual generated columns must be replaced with the corresponding generation expressions. Currently, this replacement occurs in the rewriter. However, this approach has several issues. If a Var referencing a virtual generated column has any varnullingrels, those varnullingrels need to be propagated into the generation expression. Failing to do so can lead to "wrong varnullingrels" errors and improper outer-join removal. Additionally, if such a Var comes from the nullable side of an outer join, we may need to wrap the generation expression in a PlaceHolderVar to ensure that it is evaluated at the right place and hence is forced to null when the outer join should do so. In certain cases, such as when the query uses grouping sets, we also need a PlaceHolderVar for anything that is not a simple Var to isolate subexpressions. Failure to do so can result in incorrect results. To fix these issues, this patch expands the virtual generated columns in the planner rather than in the rewriter, and leverages the pullup_replace_vars architecture to avoid code duplication. The generation expressions will be correctly marked with nullingrel bits and wrapped in PlaceHolderVars when needed by the pullup_replace_vars callback function. This requires handling the OLD/NEW RETURNING list Vars in pullup_replace_vars_callback, as it may now deal with Vars referencing the result relation instead of a subquery. The "wrong varnullingrels" error was reported by Alexander Lakhin. The incorrect result issue and the improper outer-join removal issue were reported by Richard Guo. Author: Richard Guo <guofenglinux@gmail.com> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/75eb1a6f-d59f-42e6-8a78-124ee808cda7@gmail.com
* Fix untranslatable string concatenation in pg_upgradeMichael Paquier2025-02-25
| | | | | | | Oversight in 1aab6805919b. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20250225.140953.1271748916018759840.horikyota.ntt@gmail.com
* Doc: Fix pg_copy_logical_replication_slot description.Amit Kapila2025-02-25
| | | | | | | | | | | | | | This commit documents that the failover option is not copied when using the pg_copy_logical_replication_slot function. In passing, we modify the comments in the function clarifying the reason for this behavior. Reported-by: <duffieldzane@gmail.com> Author: Hou Zhijie <houzj.fnst@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 17, where it was introduced Discussion: https://postgr.es/m/173976850802.682632.11315364077431550250@wrigleys.postgresql.org
* Do not use in-place updates for statistics import.Jeff Davis2025-02-24
| | | | | | | | | The use of in-place updates was originally there to follow the precedent of ANALYZE and to reduce the potential for bloat on pg_class. Per discussion, it's not worth the risks. Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/cpdanvzykcb5o64rmapkx6n5gjypoce3y52hff7ocxupgpbxu4@53jmlyvukijo
* psql: Add pipeline status to prompt and some state variablesMichael Paquier2025-02-25
| | | | | | | | | | | | | | | | | | | | | | This commit adds %P to psql prompts, able to report the status of a pipeline depending on PQpipelineStatus(): on, off or abort. The following variables are added to report the state of an ongoing pipeline: - PIPELINE_SYNC_COUNT: reports the number of piped syncs. - PIPELINE_COMMAND_COUNT: reports the number of piped commands, a command being either \bind, \bind_named, \close or \parse. - PIPELINE_RESULT_COUNT: reports the results available to read with \getresults. These variables can be used with \echo or in a prompt, using "%:name:" in PROMPT1, PROMPT2 or PROMPT3. Some basic regression tests are added for these. The suggestion to use variables to show the details about the status counters comes from me. The original patch proposed was less extensible, hardcoding the output in the prompt. Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Discussion: https://postgr.es/m/CAO6_XqroE7JuMEm1sWz55rp9fAYX2JwmcP_3m_v51vnOFdsLiQ@mail.gmail.com
* Fix bug in cbc127917 to handle nested Append correctlyAmit Langote2025-02-25
| | | | | | | | | | | | | | | | | A non-leaf partition with a subplan that is an Append node was omitted from PlannedStmt.unprunableRelids because it was mistakenly included in PlannerGlobal.prunableRelids due to the way PartitionedRelPruneInfo.leafpart_rti_map[] is constructed. This happened when a non-leaf partition used an unflattened Append or MergeAppend. As a result, ExecGetRangeTableRelation() reported an error when called from CreatePartitionPruneState() to process the partition's own PartitionPruneInfo, since it was treated as prunable when it should not have been. Reported-by: Alexander Lakhin <exclusion@gmail.com> (via sqlsmith) Diagnosed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/74839af6-aadc-4f60-ae77-ae65f94bf607@gmail.com
* Fix assertion when decoding XLOG_PARAMETER_CHANGE on promoted primary.Masahiko Sawada2025-02-24
| | | | | | | | | | | | | | | | | | | | When a standby replays an XLOG_PARAMETER_CHANGE record that lowers wal_level below logical, we invalidate all logical slots in hot standby mode. However, if this record was replayed while not in hot standby mode, logical slots could remain valid even after promotion, potentially causing an assertion failure during WAL record decoding. To fix this issue, this commit adds a check for hot_standby status when restoring a logical replication slot on standbys. This check ensures that logical slots are invalidated when they become incompatible due to insufficient wal_level during recovery. Backpatch to v16 where logical decoding on standby was introduced. Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/CAD21AoABoFwGY_Rh2aeE6tEq3HkJxf0c6UeOXn4VV9v6BAQPSw%40mail.gmail.com Backpatch-through: 16
* oauth: Rename macro to avoid collisions on WindowsDaniel Gustafsson2025-02-24
| | | | | | | | | | | | | | | | | | | | | | | | | | Our json parsing defined the macros OPTIONAL and REQUIRED to decorate the structs with for increased readability. This however collides with macros in the <windef.h> header on Windows. ../src/interfaces/libpq/fe-auth-oauth-curl.c:398:9: warning: "OPTIONAL" redefined 398 | #define OPTIONAL false | ^~~~~~~~ In file included from D:/a/_temp/msys64/ucrt64/include/windef.h:9, from D:/a/_temp/msys64/ucrt64/include/windows.h:69, from D:/a/_temp/msys64/ucrt64/include/winsock2.h:23, from ../src/include/port/win32_port.h:60, from ../src/include/port.h:24, from ../src/include/c.h:1331, from ../src/include/postgres_fe.h:28, from ../src/interfaces/libpq/fe-auth-oauth-curl.c:16: include/minwindef.h:65:9: note: this is the location of the previous definition 65 | #define OPTIONAL | ^~~~~~~~ Rename to avoid compilation errors in anticipation of implementing support for Windows. Reported-by: Dave Cramer (on PostgreSQL Hacking Discord)
* oauth: Fix incorrect const markers in structDaniel Gustafsson2025-02-24
| | | | | | | | | Two members in PGoauthBearerRequest were incorrectly marked as const. While in there, align the name of the struct with the typedef as per project style. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/912516.1740329361@sss.pgh.pa.us
* Delay extraction of TIDBitmap per page offsetsMelanie Plageman2025-02-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pages from the bitmap created by the TIDBitmap API can be exact or lossy. The TIDBitmap API extracts the tuple offsets from exact pages into an array for the convenience of the caller. This was done in tbm_private|shared_iterate() right after advancing the iterator. However, as long as tbm_private|shared_iterate() set a reference to the PagetableEntry in the TBMIterateResult, the offset extraction can be done later. Waiting to extract the tuple offsets has a few benefits. For the shared iterator case, it allows us to extract the offsets after dropping the shared iterator state lock, reducing time spent holding a contended lock. Separating the iteration step and extracting the offsets later also allows us to avoid extracting the offsets for prefetched blocks. Those offsets were never used, so the overhead of extracting and storing them was wasted. The real motivation for this change, however, is that future commits will make bitmap heap scan use the read stream API. This requires a TBMIterateResult per issued block. By removing the array of tuple offsets from the TBMIterateResult and only extracting the offsets when they are used, we reduce the memory required for per buffer data substantially. Suggested-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGLHbKP3jwJ6_%2BhnGi37Pw3BD5j2amjV3oSk7j-KyCnY7Q%40mail.gmail.com
* Add lossy indicator to TBMIterateResultMelanie Plageman2025-02-24
| | | | | | | | TBMIterateResult->ntuples is -1 when the page in the bitmap is lossy. Add an explicit lossy indicator so that we can move ntuples out of the TBMIterateResult in a future commit. Discussion: https://postgr.es/m/CA%2BhUKGLHbKP3jwJ6_%2BhnGi37Pw3BD5j2amjV3oSk7j-KyCnY7Q%40mail.gmail.com
* Fix comment for MAX_BACKENDS.Nathan Bossart2025-02-24
| | | | | | | | | | | | | This comment mentions that we check that the configured number of backends does not exceed MAX_BACKENDS in RegisterBackgroundWorker() and relevant GUC check hooks, neither of which has those checks anymore. To fix, adjust this comment to say that we do the check in InitializeMaxBackends(). Oversights in commits 6bc8ef0b7f and 0b1fe1413e. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/Z7zOEzz8lNjaU9yf%40nathan
* libpq: Trace all NegotiateProtocolVersion fieldsRobert Haas2025-02-24
| | | | | | | | | | Previously, the names of the unsupported protocol options were not traced. Since NegotiateProtocolVersion has not really been used yet, that has not mattered much, but we hope to use it eventually, so let's fix this. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQTfc_O+HXqAo5_-xG4r3EFVsTefUeQzSvhEyyLDba-O9w@mail.gmail.com
* libpq: Add PQfullProtocolVersion to exports.txtRobert Haas2025-02-24
| | | | | | | | This is necessary to be able to actually use the function on Windows; bug introduced in commit cdb6b0fdb0b2face270406905d31f8f513b015cc. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQTfc_O+HXqAo5_-xG4r3EFVsTefUeQzSvhEyyLDba-O9w@mail.gmail.com
* Fix confusion about data type of pg_class.relpages and relallvisible.Tom Lane2025-02-24
| | | | | | | | | | | | | | Although they're exposed as int4 in pg_class, relpages and relallvisible are really of type BlockNumber, that is uint32. Correct type puns in relation_statistics_update() and remove inappropriate range-checks. The type puns are only cosmetic issues, but the range checks would cause failures with huge relations. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Author: Corey Huinker <corey.huinker@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/614341.1740269035@sss.pgh.pa.us