aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Simplify signature of RewriteTableÁlvaro Herrera2025-01-09
| | | | | | | | | | | This function doesn't need the lockmode to be passed: it was being used to lock the new heap, but that's bogus, because the only caller has already obtained the appropriate lock on the new heap (which is unimportant anyway, because the relation's creation is not yet committed and so no other session can see it). Noticed while reviewed Antonin Houska's patch to add VACUUM FULL CONCURRENTLY.
* Fix SLRU bank selection codeÁlvaro Herrera2025-01-09
| | | | | | | | | | | | | | | | | | | | The originally submitted code (using bit masking) was correct when the number of slots was restricted to be a power of two -- but that limitation was removed during development that led to commit 53c2a97a9266, which made the bank selection code incorrect. This led to always using a smaller number of banks than available. Change said code to use integer modulo instead, which works correctly with an arbitrary number of banks. It's likely that we could improve on this to avoid runtime use of integer division. But with this change we're, at least, not wasting memory on unused banks, and more banks mean less contention, which is likely to have a much higher performance impact than a single instruction's latency. Author: Yura Sokolov <y.sokolov@postgrespro.ru> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://postgr.es/m/9444dc46-ca47-43ed-9058-89c456316306@postgrespro.ru
* Fix off_t overflow in pg_basebackup on Windows.Thomas Munro2025-01-09
| | | | | | | | | | | | walmethods.c used off_t to navigate around a pg_wal.tar file that could exceed 2GB, which doesn't work on Windows and would fail with misleading errors. Use pgoff_t instead. Back-patch to all supported branches. Author: Davinder Singh <davinder.singh@enterprisedb.com> Reported-by: Jakub Wartak <jakub.wartak@enterprisedb.com> Discussion: https://postgr.es/m/CAKZiRmyM4YnokK6Oenw5JKwAQ3rhP0YTz2T-tiw5dAQjGRXE3Q%40mail.gmail.com
* Provide 64-bit ftruncate() and lseek() on Windows.Thomas Munro2025-01-09
| | | | | | | | | | Change our ftruncate() macro to use the 64-bit variant of chsize(), and add a new macro to redirect lseek() to _lseeki64(). Back-patch to all supported releases, in preparation for a bug fix. Tested-by: Davinder Singh <davinder.singh@enterprisedb.com> Discussion: https://postgr.es/m/CAKZiRmyM4YnokK6Oenw5JKwAQ3rhP0YTz2T-tiw5dAQjGRXE3Q%40mail.gmail.com
* Fix duplicate typedef from commit a2f17f004d.Jeff Davis2025-01-08
| | | | Reported-by: Thomas Munro
* Control collation behavior with a method table.Jeff Davis2025-01-08
| | | | | | | | | | Previously, behavior branched based on the provider. A method table is less error-prone and more flexible. The ctype behavior will be addressed in an upcoming commit. Reviewed-by: Andreas Karlsson Discussion: https://postgr.es/m/2830211e1b6e6a2e26d845780b03e125281ea17b.camel%40j-davis.com
* Move code for collation version into provider-specific files.Jeff Davis2025-01-08
| | | | | Author: Andreas Karlsson Discussion: https://postgr.es/m/4548a168-62cd-457b-8d06-9ba7b985c477%40proxel.se
* Disallow NAMEDTUPLESTORE RTEs in stored views, rules, etc.Tom Lane2025-01-08
| | | | | | | | | | | | | | | | | | | A named tuplestore is necessarily a transient object, so it makes no sense to reference one in a persistent object such as a view. We didn't previously prevent that, with the result that if you tried you would get some weird failure about how the executor couldn't find the tuplestore. We can mechanize a check for this case cheaply by making dependency extraction complain if it comes across such an RTE. This is a plausible way of dealing with it since part of the problem is that we have no way to make a pg_depend representation of a named tuplestore. Report and fix by Yugo Nagata. Although this is an old problem, it's a very weird corner case and there have been no reports from end users. So it seems sufficient to fix it in master. Discussion: https://postgr.es/m/20240726160714.e74d0db579f2c017e1ca0b7e@sraoss.co.jp
* Set exit status for pgindent if pg_bsd_indent failsAndrew Dunstan2025-01-08
| | | | | | | | | | | Also document the exit codes in the script. The new exit code is 3, and is not overridden by the exit code set in --check mode. Author: Ashutosh Bapat Discussion: https://postgr.es/m/CAExHW5sPRSiFeLdP-u1Fa5ba7YS2f0gvLjmKOobopKadJwQ_GQ@mail.gmail.com
* plpgsql: pure parser and reentrant scannerPeter Eisentraut2025-01-08
| | | | | | | | | | | | | | | | | | | | | | The plpgsql scanner is a wrapper around the core scanner, which already uses the flex %option reentrant. This patch only pushes up a few levels the place where the scanner handle is allocated. Before, it was allocated in pl_scanner.c in a global variable, so to the outside the scanner was not reentrant. Now, it is allocated in pl_comp.c and is passed as an argument to yyparse(), similar to how it is handled in other reentrant scanners. Also use flex yyextra to handle context information, instead of global variables. Again, this uses the existing yyextra support in the core scanner. This complements the other changes to make the scanner reentrant. The bison option %pure-parser is used to make the generated parser pure. This happens in the usual way, since plpgsql has its own bison parser definition. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* Remove useless function declarationPeter Eisentraut2025-01-08
| | | | This function apparently never existed.
* ExecInitAgg: update aggstate->numaggs and ->numtrans earlier.Jeff Davis2025-01-07
| | | | | | | | | Functions hash_agg_entry_size() and build_hash_tables() make use of those values for memory size estimates. Because this change only affects memory estimates, don't backpatch. Discussion: https://postgr.es/m/7530bd8783b1a78d53a3c70383e38d8da0a5ffe5.camel%40j-davis.com
* nodeSetOp.c: missing additionalsize for BuildTupleHashTable().Jeff Davis2025-01-07
| | | | | | | | Provide additionalsize argument, which can affect the calculations for 'nbuckets'. Also, future work for Hash Aggregation will rely on the correct additionalsize. Discussion: https://postgr.es/m/7530bd8783b1a78d53a3c70383e38d8da0a5ffe5.camel%40j-davis.com
* Remove unused TupleHashTableData->entrysize.Jeff Davis2025-01-07
| | | | Discussion: https://postgr.es/m/7530bd8783b1a78d53a3c70383e38d8da0a5ffe5.camel%40j-davis.com
* Add missing typedefs.list entry for AggStatePerGroupData.Jeff Davis2025-01-07
| | | | Discussion: https://postgr.es/m/7530bd8783b1a78d53a3c70383e38d8da0a5ffe5.camel%40j-davis.com
* Use PqMsg_* macros in postgres.c.Nathan Bossart2025-01-07
| | | | | | | | | | Commit f4b54e1ed9, which introduced macros for protocol characters, missed updating a couple of places in postgres.c. Author: Dave Cramer Reviewed-by: Fabrízio de Royes Mello Discussion: https://postgr.es/m/CADK3HHJUVBPoVOmFesPB-fN8_dYt%2BQELV2UB6jxOW2Z40qF-qw%40mail.gmail.com Backpatch-through: 17
* Lower default value of autovacuum_worker_slots in initdb as needed.Nathan Bossart2025-01-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit c758119e5b increased the default number of semaphores required for autovacuum workers from 3 to 16. Unfortunately, some systems have very low default settings for SEMMNS, and this change moved the minimum required for Postgres well beyond that limit (see commit 38da053463 for more details). With this commit, initdb will lower the default value for autovacuum_worker_slots as needed, just like it already does for parameters such as max_connections and shared_buffers. We test for (max_connections / 6) slots, which conveniently has the following properties: * For the initial max_connections default of 100, the default of autovacuum_worker_slots will be 16, which is its initial default value specified in the documentation and in guc_tables.c. * For the lowest possible max_connections default of 25, the default of autovacuum_worker_slots will be 4, which means we only need one additional semaphore for autovacuum workers (as compared to before commit c758119e5b). This leaves some wiggle room for new auxiliary workers, etc. on systems with low SEMMNS, and it ensures that the default number of slots will be greater than or equal to the default value of autovacuum_max_workers (3). Reported-by: Tom Lane Suggested-by: Andres Freund Reviewed-by: Tom Lane Discussion: https://postgr.es/m/1346002.1736198977%40sss.pgh.pa.us
* Fix error message wordingÁlvaro Herrera2025-01-07
| | | | | | | The originals are ambiguous and a bit out of style. Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/202412141243.efesjyyvzxsz@alvherre.pgsql
* Remove unnecessary code to handle CONSTR_NOTNULLÁlvaro Herrera2025-01-07
| | | | | | | | | | | | Commit 14e87ffa5c54 needlessly added support for CONSTR_NOTNULL entries to StoreConstraints. It's dead code, so remove it. To make the situation regarding constraint creation clearer, change comments in heap_create_with_catalog, StoreConstraints, MergeAttributes to explain which types of constraint are used on each. Author: 何建 (Jian He) <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxFxzqrCiUNfjJ0tQU+=nKQkQCGtGzUBude=SMOwj5VNjQ@mail.gmail.com
* Improve nbtree unsatisfiable RowCompare detection.Peter Geoghegan2025-01-07
| | | | | | | | | | | | | | | | | Move nbtree's detection of RowCompare quals that are unsatisfiable due to having a NULL in their first row element: rather than detecting these cases at the point where _bt_first builds its insertion scan key, do so earlier, during preprocessing proper. This brings the RowCompare case in line every other case involving an unsatisfiable-due-to-NULL qual. nbtree now consistently detects such unsatisfiable quals -- even when they happen to involve a key that isn't examined by _bt_first at all. Affected cases thereby avoid useless full index scans that cannot possibly return any matching rows. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-WzmySVXst2hFrOATC-zw1Byg1XC-jYUS314=mzuqsNwk+Q@mail.gmail.com
* nbtree: Simplify _bt_first parallel scan handling.Peter Geoghegan2025-01-07
| | | | | | | | | | | | This new structure relieves _bt_first from having separate calls to _bt_start_array_keys for the serial case and parallel case. This saves code, and seems clearer. Follow-up to work from commits 4e6e375b and b5ee4e52. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-Wz=XjUZjBjHJdhTvuH5MwoJObWGoM2RG2LyFg5WUdWyk=A@mail.gmail.com
* Remove unused parameter in lookup_var_attr_statsRichard Guo2025-01-07
| | | | | | | | | | | | | The parameter 'rel' in lookup_var_attr_stats was once used to draw an ERROR when ANALYZE failed to acquire sufficient data to build extended statistics. bf2a691e0 changed the logic to raise a WARNING in the caller instead. As a result, this parameter is no longer needed and can be removed. Since this is a static function, we can always easily reintroduce the parameter if it's ever needed in the future. Author: Ilia Evdokimov Reviewed-by: Fabrízio de Royes Mello Discussion: https://postgr.es/m/b3880f22-5808-4206-88d4-1553a81c3440@tantorlabs.com
* Allow changing autovacuum_max_workers without restarting.Nathan Bossart2025-01-06
| | | | | | | | | | | | | | | | | | | | | | | This commit introduces a new parameter named autovacuum_worker_slots that controls how many autovacuum worker slots to reserve during server startup. Modifying this new parameter's value does require a server restart, but it should typically be set to the upper bound of what you might realistically need to set autovacuum_max_workers. With that new parameter in place, autovacuum_max_workers can now be changed with a SIGHUP (e.g., pg_ctl reload). If autovacuum_max_workers is set higher than autovacuum_worker_slots, a WARNING is emitted, and the server will only start up to autovacuum_worker_slots workers at a given time. If autovacuum_max_workers is set to a value less than the number of currently-running autovacuum workers, the existing workers will continue running, but no new workers will be started until the number of running autovacuum workers drops below autovacuum_max_workers. Reviewed-by: Sami Imseih, Justin Pryzby, Robert Haas, Andres Freund, Yogesh Sharma Discussion: https://postgr.es/m/20240410212344.GA1824549%40nathanxps13
* Remove duplicate definitions in proc.hHeikki Linnakangas2025-01-06
| | | | | | | These are also present in procnumber.h Reported-by: Peter Eisentraut Discussion: https://www.postgresql.org/message-id/bd04d675-4672-4f87-800a-eb5d470c15fc@eisentraut.org
* flex code modernization: Replace YY_EXTRA_TYPE define with flex optionPeter Eisentraut2025-01-06
| | | | | | | | Replace #define YY_EXTRA_TYPE with %option extra-type. The latter is the way recommended by the flex manual (available since flex 2.5.34). Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* Always use the caller-provided context for radix tree leavesJohn Naylor2025-01-06
| | | | | | | | | | | | | | | | | | | | Previously, it would not have worked for a caller to pass a slab context, since it would have been used for other things which likely had incompatible size. In an attempt to be helpful and avoid possible space wastage due to aset's power-of-two rounding, RT_CREATE would create an additional slab context if the value type was fixed-length and larger than pointer size. The problem was, we have since added the bump context type, and the generation context was a possibility as well, so silently overriding the caller's choice may actually be worse. Commit e8a6f1f908d arranged so that the caller-provided context is used only for leaves, so it's safe for the caller to use slab here if they wish. As demonstration, use slab in one of the radix tree regression tests. Reviewed by Masahiko Sawada Discussion: https://postgr.es/m/CANWCAZZDCo4k5oURg_pPxM6+WZ1oiG=sqgjmQiELuyP0Vtrwig@mail.gmail.com
* Get rid of radix tree's general purpose memory contextJohn Naylor2025-01-06
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously, this was notionally used only for the entry point of the tree and as a convenient parent for other contexts. For shared memory, the creator previously allocated the entry point in this context, but attaching backends didn't have access to that, so they just used the caller's context. For the sake of consistency, allocate every instance of an entry point in the caller's context. For local memory, allocate the control object in the caller's context as well. This commit also makes the "leaf context" the notional parent of the child contexts used for nodes, so it's a bit of a misnomer, but a future commit will make the node contexts independent rather than children, so leave it this way for now to avoid code churn. The memory context parameter for RT_CREATE is now unused in the case of shared memory, so remove it and adjust callers to match. In passing, remove unused "context" member from struct TidStore, which seems to have been an oversight. Reviewed by Masahiko Sawada Discussion: https://postgr.es/m/CANWCAZZDCo4k5oURg_pPxM6+WZ1oiG=sqgjmQiELuyP0Vtrwig@mail.gmail.com
* Use caller's memory context for radix tree iteration stateJohn Naylor2025-01-06
| | | | | | | | | | | | | | | Typically only one iterator is present at any time, so it's overkill to devote an entire context for this. Get rid of it and use the caller's context. This is tidy-up work, so no backpatch in this form. However, a hypothetical extension to v17 that tried to start iteration from an attaching backend would result in a crash, so that'll be fixed separately in a way that doesn't change behavior in core. Patch by me, reported and reviewed by Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoBB2U47V=F+wQRB1bERov_of5=BOZGaybjaV8FLQyqG3Q@mail.gmail.com
* Document strange jsonb sort order for empty top level arraysAndrew Dunstan2025-01-03
| | | | | | | | | | | | | | | | Slightly faulty logic in the original jsonb code (commit d9134d0a355) results in an empty top level array sorting less than a json null. We can't change the sort order now since it would affect btree indexes over jsonb, so document the anomaly. Backpatch to all live branches (13 .. 17) In master, also add a code comment noting the anomaly. Reported-by: Yan Chengpen Reviewed-by: Jian He Discussion: https://postgr.es/m/OSBPR01MB45199DD8DA2D1CECD50518188E272@OSBPR01MB4519.jpnprd01.prod.outlook.com
* Ignore nullingrels when looking up statisticsRichard Guo2025-01-02
| | | | | | | | | | | | | | | | | | | | | | | | | | When looking up statistical data about an expression, we do not need to concern ourselves with the outer joins that could null the Vars/PHVs contained in the expression. Accounting for nullingrels in the expression could cause estimate_num_groups to count the same Var multiple times if it's marked with different nullingrels. This is incorrect, and could lead to "ERROR: corrupt MVNDistinct entry" when searching for multivariate n-distinct. Furthermore, the nullingrels could prevent us from matching an expression to expressional index columns or to the expressions in extended statistics, leading to inaccurate estimates. To fix, strip out all the nullingrels from the expression before we look up statistical data about it. There is one ensuing plan change in the regression tests, but it looks reasonable and does not compromise its original purpose. This patch could result in plan changes, but it fixes an actual bug, so back-patch to v16 where the outer-join-aware-Var infrastructure was introduced. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs4-2Z4k+nFTiZe0Qbu5n8juUWenDAtMzi98bAZQtwHx0-w@mail.gmail.com
* Fix outdated CHUNKHDRSZ value in nodeAgg.cDavid Rowley2025-01-02
| | | | | | | | | | | | | | CHUNKHDRSZ was defined as 16 bytes, which was true when that code went in, but since c6e0fe1f2, 8 is a more accurate value. Here we adjust it to use sizeof(MemoryChunk), which is normally 8, or 16 for cassert builds. c6e0fe1f2 first appeared in v16, so this is technically wrong in v16 up to master, but let's apply this only to master as adjusting this does influence the estimated number of batches in the aggregate costing code and we don't want to cause plan instability in released versions. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAApHDvpMpRQvsTqZo3FinXkgytwxwF8sCyZm83xDj-1s_hLe+w@mail.gmail.com
* Fix an assortment of spelling mistakes and typosDavid Rowley2025-01-02
| | | | | Author: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/5812a0b9-b0cf-4151-9a14-d9f00e4f2858@gmail.com
* Update copyright for 2025Bruce Momjian2025-01-01
| | | | Backpatch-through: 13
* Update obsolete reference to plpgsql's gram.y file.Tom Lane2024-12-30
| | | | | | | | | This was evidently missed in 05346c131, which renamed that file to pl_gram.y. Japin Li Discussion: https://postgr.es/m/ME0P300MB0445F7CA7456C2AC67D37A01B6092@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM
* injection_points: Tweak variable-numbered stats to work with pending dataMichael Paquier2024-12-30
| | | | | | | | | | | | | | | | As coded, the module was not using pending entries to store its data locally before doing a flush to the central dshash with a timed pgstat_report_stat() call. Hence, the flush callback was defined, but finished by being not used. As a template, this is more efficient than the original logic of updating directly the shared memory entries as this reduces the interactions that need to be done with the pgstats hash table in shared memory. injection_stats_flush_cb() was also missing a pgstat_unlock_entry(), so add one, while on it. Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Z3JbLhKFFm6kKfT8@ip-10-97-1-34.eu-west-3.compute.internal
* Fix memory leak in pgoutput with relation attribute mapMichael Paquier2024-12-30
| | | | | | | | | | | | | | | | | | | | pgoutput caches the attribute map of a relation, that is free()'d only when validating a RelationSyncEntry. However, this code path is not taken when calling any of the SQL functions able to do some logical decoding, like pg_logical_slot_{get,peek}_changes(), leaking some memory into CacheMemoryContext on repeated calls. To address this, a relation's attribute map is allocated in PGOutputData's cachectx, free()'d at the end of the execution of these SQL functions when logical decoding ends. This is available down to 15. v13 and v14 have a similar leak, which will be dealt with later. Reported-by: Masahiko Sawada Author: Vignesh C Reviewed-by: Hou Zhijie Discussion: https://postgr.es/m/CAD21AoDkAhQVSukOfH3_reuF-j4EU0-HxMqU3dU+bSTxsqT14Q@mail.gmail.com Discussion: https://postgr.es/m/CALDaNm1hewNAsZ_e6FF52a=9drmkRJxtEPrzCB6-9mkJyeBBqA@mail.gmail.com Backpatch-through: 15
* Remove redundant wording in pg_statistic.hMichael Paquier2024-12-30
| | | | | Author: Junwang Zhao Discussion: https://postgr.es/m/CAEG8a3JbMCHna=N5ZSx6huLnTDfW34kw7Pf2n8+3M-9UrrwesA@mail.gmail.com
* Fix failures with incorrect epoch handling for 2PC files at recoveryMichael Paquier2024-12-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | At the beginning of recovery, an orphaned two-phase file in an epoch different than the one defined in the checkpoint record could not be removed based on the assumptions that AdjustToFullTransactionId() relies on, assuming that all files would be either from the current epoch or from the previous epoch. If the checkpoint epoch was 0 while the 2PC file was orphaned and in the future, AdjustToFullTransactionId() would underflow the epoch used to build the 2PC file path. In non-assert builds, this would create a WARNING message referring to a 2PC file with an epoch of "FFFFFFFF" (or UINT32_MAX), as an effect of the underflow calculation, leaving the orphaned file around. Some tests are added with dummy 2PC files in the past and the future, checking that these are properly removed. Issue introduced by 5a1dfde8334b, that has switched two-phase state files to use FullTransactionIds. Reported-by: Vitaly Davydov Author: Michael Paquier Reviewed-by: Vitaly Davydov Discussion: https://postgr.es/m/13b5b6-676c3080-4d-531db900@47931709 Backpatch-through: 17
* Fix handling of orphaned 2PC files in the future at recoveryMichael Paquier2024-12-30
| | | | | | | | | | | | | | | | | | | | | | | | | | Before 728bd991c3c4, that has improved the support for 2PC files during recovery, the initial logic scanning files in pg_twophase was done so as files in the future of the transaction ID horizon were checked first, followed by a check if a transaction ID is aborted or committed which could involve a pg_xact lookup. After this commit, these checks have been done in reverse order. Files detected as in the future do not have a state that can be checked in pg_xact, hence this caused recovery to fail abruptly should an orphaned 2PC file in the future of the transaction ID horizon exist in pg_twophase at the beginning of recovery. A test is added to check for this scenario, using an empty 2PC with a transaction ID large enough to be in the future when running the test. This test is added in 16 and older versions for now. 17 and newer versions are impacted by a second bug caused by the addition of the epoch in the 2PC file names. An equivalent test will be added in these branches in a follow-up commit, once the second set of issues reported are fixed. Author: Vitaly Davydov, Michael Paquier Discussion: https://postgr.es/m/11e597-676ab680-8d-374f23c0@145466129 Backpatch-through: 13
* Fix overly large values/nulls arraysDavid Rowley2024-12-29
| | | | | | | | | | | | | These arrays were sized with Natts_pg_trigger (19) when they should have been sized with Natts_pg_event_trigger (7). We'd better fix this as it's clearly a mistake and it could become problematic if pg_event_trigger were to gain a dozen or so more columns in the future. No backpatch as there's no actual bug and the column count on those tables isn't going to change in released versions. Author: Xin Zhang <zhanghien@qq.com> Discussion: https://postgr.es/m/tencent_05AD0FB321A414EC3661204D2102AA6EF605@qq.com
* Replace PGPROC.isBackgroundWorker with isRegularBackend.Tom Lane2024-12-28
| | | | | | | | | Commit 34486b609 effectively redefined isBackgroundWorker as meaning "not a regular backend", whereas before it had the narrower meaning of AmBackgroundWorkerProcess(). For clarity, rename the field to isRegularBackend and invert its sense. Discussion: https://postgr.es/m/1808397.1735156190@sss.pgh.pa.us
* Exclude parallel workers from connection privilege/limit checks.Tom Lane2024-12-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Cause parallel workers to not check datallowconn, rolcanlogin, and ACL_CONNECT privileges. The leader already checked these things (except for rolcanlogin which might have been checked for a different role). Re-checking can accomplish little except to induce unexpected failures in applications that might not even be aware that their query has been parallelized. We already had the principle that parallel workers rely on their leader to pass a valid set of authorization information, so this change just extends that a bit further. Also, modify the ReservedConnections, datconnlimit and rolconnlimit logic so that these limits are only enforced against regular backends, and only regular backends are counted while checking if the limits were already reached. Previously, background processes that had an assigned database or role were subject to these limits (with rather random exclusions for autovac workers and walsenders), and the set of existing processes that counted against each limit was quite haphazard as well. The point of these limits, AFAICS, is to ensure the availability of PGPROC slots for regular backends. Since all other types of processes have their own separate pools of PGPROC slots, it makes no sense either to enforce these limits against them or to count them while enforcing the limit. While edge-case failures of these sorts have been possible for a long time, the problem got a good deal worse with commit 5a2fed911 (CVE-2024-10978), which caused parallel workers to make some of these checks using the leader's current role where before we had used its AuthenticatedUserId, thus allowing parallel queries to fail after SET ROLE. The previous behavior was fairly accidental and I have no desire to return to it. This patch includes reverting 73c9f91a1, which was an emergency hack to suppress these same checks in some cases. It wasn't complete, as shown by a recent bug report from Laurenz Albe. We can also revert fd4d93d26 and 492217301, which hacked around the same problems in one regression test. In passing, remove the special case for autovac workers in CheckMyDatabase; it seems cleaner to have AutoVacWorkerMain pass the INIT_PG_OVERRIDE_ALLOW_CONNS flag, now that that does what's needed. Like 5a2fed911, back-patch to supported branches (which sadly no longer includes v12). Discussion: https://postgr.es/m/1808397.1735156190@sss.pgh.pa.us
* Reserve a PGPROC slot and semaphore for the slotsync worker process.Tom Lane2024-12-28
| | | | | | | | | | | | | | | | | | | | | | | | | The need for this was missed in commit 93db6cbda, with the result being that if we launch a slotsync worker it would consume one of the PGPROCs in the max_connections pool. That could lead to inability to launch the worker, or to subsequent failures of connection requests that should have succeeded according to the configured settings. Rather than create some one-off infrastructure to support this, let's group the slotsync worker with the existing autovac launcher in a new category of "special worker" processes. These are kind of like auxiliary processes, but they cannot use that infrastructure because they need to be able to run transactions. For the moment, make these processes share the PGPROC freelist used for autovac workers (which previously supplied the autovac launcher too). This is partly to avoid an ABI change in v17, and partly because it seems silly to have a freelist with at most two members. This might be worth revisiting if we grow enough workers in this category. Tom Lane and Hou Zhijie. Back-patch to v17. Discussion: https://postgr.es/m/1808397.1735156190@sss.pgh.pa.us
* In REASSIGN OWNED of a database, lock the tuple as mandated.Noah Misch2024-12-28
| | | | | | | | | | | | | | | | | | | | Commit aac2c9b4fde889d13f859c233c2523345e72d32b mandated such locking and attempted to fulfill that mandate, but it missed REASSIGN OWNED. Hence, it remained possible to lose VACUUM's inplace update of datfrozenxid if a REASSIGN OWNED processed that database at the same time. This didn't affect the other inplace-updated catalog, pg_class. For pg_class, REASSIGN OWNED calls ATExecChangeOwner() instead of the generic AlterObjectOwner_internal(), and ATExecChangeOwner() fulfills the locking mandate. Like in GRANT, implement this by following the locking protocol for any catalog subject to the generic AlterObjectOwner_internal(). It would suffice to do this for IsInplaceUpdateOid() catalogs only. Back-patch to v13 (all supported versions). Kirill Reshke. Reported by Alexander Kukushkin. Discussion: https://postgr.es/m/CAFh8B=mpKjAy4Cuun-HP-f_vRzh2HSvYFG3rhVfYbfEBUhBAGg@mail.gmail.com
* Speedup tuple deformation with additional function inliningDavid Rowley2024-12-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This adjusts slot_deform_heap_tuple() to add special-case loops to eliminate much of the branching that was done within the body of the main deform loop. Previously, while looping over each attribute to deform, slot_deform_heap_tuple() would always recheck if the given attribute was NULL by looking at HeapTupleHasNulls() and if so, went on to check the tuple's NULL bitmap. Since many tuples won't contain any NULLs, we can just check HeapTupleHasNulls() once and when there are no NULLs, use a more compact version of the deforming loop which contains no NULL checking code at all. The same is possible for the "slow" mode checking part of the loop. That variable was checked several times for each attribute, once to determine if the offset to the attribute value could be taken from the attcacheoff, and again to check if the offset could be cached for next time. These "slow" checks can mostly be eliminated by instead having multiple loops. Initially, we can start in the non-slow loop and break out of that loop if and only if we must stop caching the offset. This eliminates branching for both slow and non-slow deforming methods. The amount of code required for the no nulls / non-slow version is very small. It's possible to have separate loops like this due to the fact that once we move into slow mode, we never need to switch back into non-slow mode for a given tuple. We have the compiler take care of writing out the multiple required loops by having a pg_attribute_always_inline function which gets called various times passing in constant values for the "slow" and "hasnulls" parameters. This allows the compiler to eliminate const-false branches and remove comparisons for const-true ones. This commit has shown overall query performance increases of around 5-20% in deform-heavy OLAP-type workloads. Author: David Rowley Reviewed-by: Victor Yegorov Discussion: https://postgr.es/m/CAGnEbog92Og2CpC2S8=g_HozGsWtt_3kRS1sXjLz0jKSoCNfLw@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvo9e0XG71WrefYaRv5n4xNPLK4k8LjD0mSR3c9KR2vi2Q@mail.gmail.com
* Improve handling of date_trunc() units for infinite input valuesMichael Paquier2024-12-27
| | | | | | | | | | | | | | | | | | | | Previously, if an infinite value was passed to date_trunc(), then the same infinite value would always be returned regardless of the field unit given by the caller. This commit updates the function so that an error is returned when an invalid unit is passed to date_trunc() with an infinite value. This matches the behavior of date_trunc() with a finite value and date_part() with an infinite value, making the handling of interval, timestamp and timestamptz more consistent across the board for these two functions. Some tests are added to cover all these new failure cases, with an unsupported unit and infinite values for the three data types. There were no test cases in core that checked all these patterns up to now. Author: Joseph Koshakow Discussion: https://postgr.es/m/CAAvxfHc4084dGzEJR0_pBZkDuqbPGc5wn7gK_M0XR_kRiCdUJQ@mail.gmail.com
* Remove unused totalrows parameter in compute_expr_statsDavid Rowley2024-12-27
| | | | | | | | | The totalrows parameter in compute_expr_stats is unused, so remove it. This is a static function, so the parameter can easily be added again if it's ever needed. Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.ru> Discussion: https://postgr.es/m/667b92d2-f953-4fcb-9377-3765f5b94187@tantorlabs.com
* plpgsql: Rename a variable for clarityPeter Eisentraut2024-12-26
| | | | | | | Rename "core_yy_extra_type core_yy" to "core_yy_extra". The previous name was a bit unclear and confusing. The new name matches the name used elsewhere for the same purpose, for example in src/backend/parser/gramparse.h.
* Fix typo in comment of compute_return_type() in functioncmds.cMichael Paquier2024-12-26
| | | | | Author: Japin Li Discussion: https://postgr.es/m/ME0P300MB0445D51BCFA8680F0B35FD6EB60C2@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM
* meson: Export all libcommon functions in Windows buildsHeikki Linnakangas2024-12-25
| | | | | | | | | | | | | This fixes "unresolved external symbol" errors with extensions that use functions from libpgport that need special CFLAGS to compile. Currently, that includes the CRC-32 functions. Commit 2571c1d5cc did this for libcommon, but I missed that libpqport has the same issue. Reported-by: Tom Lane Backpatch-through: 16, where Meson was introduced Discussion: https://www.postgresql.org/message-id/CAOdR5yF0krWrxycA04rgUKCgKugRvGWzzGLAhDZ9bzNv8g0Lag@mail.gmail.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-24 20:44:23 +0000