aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* bootstrap: pure parser and reentrant scannerPeter Eisentraut2024-12-19
| | | | | | | | | | | | | | | | | Use the flex %option reentrant and the bison option %pure-parser to make the generated scanner and parser pure, reentrant, and thread-safe. Make the generated scanner use palloc() etc. instead of malloc() etc. For the bootstrap scanner and parser, reentrancy and memory management aren't that important, but we make this change here anyway so that all the scanners and parsers in the backend use a similar set of options and APIs. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* Small whitespace improvementPeter Eisentraut2024-12-19
| | | | | Author: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* Prevent redeclaration of typedef yyscan_tPeter Eisentraut2024-12-19
| | | | | | | | | | | | | Fix for 1f0de66ea2a: We need to prevent redeclaration of typedef yyscan_t. (This will work with C11 but not currently with C99.) The generated scanner files provide their own typedef, but we also need to provide one for the interfaces that we expose. So we need to add some preprocessor guards to avoid a redefinition. (This is how the generated scanner files do it internally as well.) This way everything now works independent of the order in which things are included. Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* Add backend-level statistics to pgstatsMichael Paquier2024-12-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This adds a new variable-numbered statistics kind in pgstats, where the object ID key of the stats entries is based on the proc number of the backends. This acts as an upper-bound for the number of stats entries that can exist at once. The entries are created when a backend starts after authentication succeeds, and are removed when the backend exits, making the stats entry exist for as long as their backend is up and running. These are not written to the pgstats file at shutdown (note that write_to_file is disabled, as a safety measure). Currently, these stats include only information about the I/O generated by a backend, using the same layer as pg_stat_io, except that it is now possible to know how much activity is happening in each backend rather than an overall aggregate of all the activity. A function called pg_stat_get_backend_io() is added to access this data depending on the PID of a backend. The existing structure could be expanded in the future to add more information about other statistics related to backends, depending on requirements or ideas. Auxiliary processes are not included in this set of statistics. These are less interesting to have than normal backends as they have dedicated entries in pg_stat_io, and stats kinds of their own. This commit includes also pg_stat_reset_backend_stats(), function able to reset all the stats associated to a single backend. Bump catalog version and PGSTAT_FILE_FORMAT_ID. Author: Bertrand Drouvot Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, Michael Paquier, Nazir Bilal Yavuz Discussion: https://postgr.es/m/ZtXR+CtkEVVE/LHF@ip-10-97-1-34.eu-west-3.compute.internal
* Extract logic filling pg_stat_get_io()'s tuplestore into its own routineMichael Paquier2024-12-19
| | | | | | | | | | | This commit adds pg_stat_io_build_tuples(), a helper routine for pg_stat_get_io(), that fills its result tuplestore based on the contents of PgStat_BktypeIO. This will be used in a follow-up commit that uses the same structures as pg_stat_io for reporting, including the same object types and contexts, but for a different statistics kind. Author: Bertrand Drouvot, Michael Paquier Discussion: https://postgr.es/m/ZtXR+CtkEVVE/LHF@ip-10-97-1-34.eu-west-3.compute.internal
* Optimize grouping equality checks with virtual slotsDavid Rowley2024-12-19
| | | | | | | | | | | | | | | | | | 8f4ee9626 fixed an old Assert failure that could happen when the slot type used to look up the hash table for BuildTupleHashTableExt() users wasn't a TTSOpsMinimalTuple slot. The fix for that in the back branches had to be to pass the TupleTableSlotOps as NULL, however in master, since we have the inputOps parameter as was added by d96d1d515, we can pass that down instead. At least one caller uses a fixed slot that's always TTSOpsVirtual, so passing down inputOps for these cases allows ExecBuildGroupingEqual() to skip adding the EEOP_INNER_FETCHSOME ExprEvalStep. This should increase the performance of hashed subplans very slightly. Author: Tom Lane, David Rowley Discussion: https://postgr.es/m/2543667.1734483723@sss.pgh.pa.us
* Fix Assert failure in WITH RECURSIVE UNION queriesDavid Rowley2024-12-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the non-recursive part of a recursive CTE ended up using TTSOpsBufferHeapTuple as the table slot type, then a duplicate value could cause an Assert failure in CheckOpSlotCompatibility() when checking the hash table for the duplicate value. The expected slot type for the deform step was TTSOpsMinimalTuple so the Assert failed when the TTSOpsBufferHeapTuple slot was used. This is a long-standing bug which we likely didn't notice because it seems much more likely that the non-recursive term would have required projection and used a TTSOpsVirtual slot, which CheckOpSlotCompatibility is ok with. There doesn't seem to be any harm done here other than the Assert failure. Both TTSOpsMinimalTuple and TTSOpsBufferHeapTuple slot types require tuple deformation, so the EEOP_*_FETCHSOME ExprState step would have properly existed in the ExprState. The solution is to pass NULL for the ExecBuildGroupingEqual's 'lops' parameter. This means the ExprState's EEOP_*_FETCHSOME step won't expect a fixed slot type. This makes CheckOpSlotCompatibility() happy as no checking is performed when the ExprEvalStep is not expecting a fixed slot type. Reported-by: Richard Guo Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAMbWs4-8U9q2LAtf8+ghV11zeUReA3AmrYkxzBEv0vKnDxwkKA@mail.gmail.com Backpatch-through: 13, all supported versions
* Remove leftover mentions of XLOG_HEAP2_FREEZE_PAGE recordsMelanie Plageman2024-12-18
| | | | | | | | | | | f83d709760d merged the separate XLOG_HEAP2_FREEZE_PAGE records into a new combined prune, freeze, and vacuum record with opcode XLOG_HEAP2_PRUNE_VACUUM_SCAN. Remove the last few references to XLOG_HEAP2_FREEZE_PAGE records which were accidentally left behind. Reported-by: Tomas Vondra Reviewed-by: Robert Haas Discussion: https://postgr.es/m/CA%2BTgmoY1tYff-1CEn8kYt5FsOrynTbtr%3DUZw%3D7mTC1Hv1HpeBQ%40mail.gmail.com
* Bitmap Table Scans use unified TBMIteratorMelanie Plageman2024-12-18
| | | | | | | | | | | | | | With the repurposing of TBMIterator as an interface for both parallel and serial iteration through TIDBitmaps in commit 7f9d4187e7bab10329cc, bitmap table scans may now use it. Modify bitmap table scan code to use the TBMIterator. This requires moving around a bit of code, so a few variables are initialized elsewhere. Author: Melanie Plageman Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/c736f6aa-8b35-4e20-9621-62c7c82e2168%40vondra.me
* Add common interface for TBMIteratorsMelanie Plageman2024-12-18
| | | | | | | | | | | | | Add and use TBMPrivateIterator, which replaces the current TBMIterator for serial use cases, and repurpose TBMIterator to be a unified interface for both the serial ("private") and parallel ("shared") TID Bitmap iterator interfaces. This encapsulation simplifies call sites for callers supporting both parallel and serial TID Bitmap access. TBMIterator is not yet used in this commit. Author: Melanie Plageman Reviewed-by: Tomas Vondra, Heikki Linnakangas Discussion: https://postgr.es/m/063e4eb4-32d9-439e-a0b1-75565a9835a8%40iki.fi
* Fix overflow danger in SampleHeapTupleVisible()Melanie Plageman2024-12-18
| | | | | | | | | | | 68d9662be1c4b70 made HeapScanDesc->rs_ntuples unsigned but neglected to change how it was being used in SampleHeapTupleVisible(). Return early if rs_ntuples is 0 to avoid overflowing and incorrectly executing the loop code in SampleHeapTupleVisible(). Reported-by: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAot_xQoZyPZjpj1aBUPrPykY5mOPHGyvfe%3Djz%2BWowdA3A%40mail.gmail.com
* Make rs_cindex and rs_ntuples unsignedMelanie Plageman2024-12-18
| | | | | | | | | | | | | | | | | HeapScanDescData.rs_cindex and rs_ntuples can't be less than 0. All scan types using the heap scan descriptor expect these values to be >= 0. Make that expectation clear by making rs_cindex and rs_ntuples unsigned. Also remove the test in heapam_scan_bitmap_next_tuple() that checks if rs_cindex < 0. This was never true, but now that rs_cindex is unsigned, it makes even less sense. While we are at it, initialize both rs_cindex and rs_ntuples to 0 in initscan(). Author: Melanie Plageman Reviewed-by: Dilip Kumar Discussion: https://postgr.es/m/CAAKRu_ZxF8cDCM_BFi_L-t%3DRjdCZYP1usd1Gd45mjHfZxm0nZw%40mail.gmail.com
* seg: pure parser and reentrant scannerPeter Eisentraut2024-12-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use the flex %option reentrant and the bison option %pure-parser to make the generated scanner and parser pure, reentrant, and thread-safe. Make the generated scanner use palloc() etc. instead of malloc() etc. Previously, we only used palloc() for the buffer, but flex would still use malloc() for its internal structures. As a result, there could be some small memory leaks in case of uncaught errors. (We do catch normal syntax errors as soft errors.) Now, all the memory is under palloc() control, so there are no more such issues. Simplify flex scan buffer management: Instead of constructing the buffer from pieces and then using yy_scan_buffer(), we can just use yy_scan_string(), which does the same thing internally. The previous code was necessary because we allocated the buffer with palloc() and the rest of the state was handled by malloc(). But this is no longer the case; everything is under palloc() now. (We could even get rid of the yylex_destroy() call and just let the memory context cleanup handle everything. But for now, we preserve the existing behavior.) Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* cube: pure parser and reentrant scannerPeter Eisentraut2024-12-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use the flex %option reentrant and the bison option %pure-parser to make the generated scanner and parser pure, reentrant, and thread-safe. Make the generated scanner use palloc() etc. instead of malloc() etc. Previously, we only used palloc() for the buffer, but flex would still use malloc() for its internal structures. As a result, there could be some small memory leaks in case of uncaught errors. (We do catch normal syntax errors as soft errors.) Now, all the memory is under palloc() control, so there are no more such issues. Simplify flex scan buffer management: Instead of constructing the buffer from pieces and then using yy_scan_buffer(), we can just use yy_scan_string(), which does the same thing internally. (Actually, we use yy_scan_bytes() here because we already have the length.) The previous code was necessary because we allocated the buffer with palloc() and the rest of the state was handled by malloc(). But this is no longer the case; everything is under palloc() now. (We could even get rid of the yylex_destroy() call and just let the memory context cleanup handle everything. But for now, we preserve the existing behavior.) Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/eb6faeac-2a8a-4b69-9189-c33c520e5b7b@eisentraut.org
* psql: Add more information about service nameMichael Paquier2024-12-18
| | | | | | | | | | | | | | This commit adds support for the following items in psql, able to show a service name, when available: - Variable SERVICE. - Substitution %s in PROMPT{1,2,3}. This relies on 4b99fed7541e, that has made the service name available in PGconn for libpq. Author: Michael Banck Reviewed-by: Greg Sabino Mullane Discussion: https://postgr.es/m/6723c612.050a0220.1567f4.b94a@mx.google.com
* libpq: Add service name to PGconn and PQservice()Michael Paquier2024-12-18
| | | | | | | | | | | | | | This commit adds one field to PGconn for the database service name (if any), with PQservice() as routine to retrieve it. Like the other routines of this area, NULL is returned as result if the connection is NULL. A follow-up patch will make use of this feature to be able to display the service name in the psql prompt. Author: Michael Banck Reviewed-by: Greg Sabino Mullane Discusion: https://postgr.es/m/6723c612.050a0220.1567f4.b94a@mx.google.com
* Fix memory leak in pg_restore with zstd-compressed data.Tom Lane2024-12-17
| | | | | | | | | | EndCompressorZstd() neglected to free everything. This was most visible with a lot of large objects in the dump. Per report from Tomasz Szypowski. Back-patch to v16 where this code came in. Discussion: https://postgr.es/m/DU0PR04MB94193D038A128EF989F922D199042@DU0PR04MB9419.eurprd04.prod.outlook.com
* Fix incorrect slot type in BuildTupleHashTableExtDavid Rowley2024-12-18
| | | | | | | | | | | | | | | | | | | | | | | | | | 0f5738202 adjusted the execGrouping.c code so it made use of ExprStates to generate hash values. That commit made a wrong assumption that the slot type to pass to ExecBuildHash32FromAttrs() is always &TTSOpsMinimalTuple. That's not the case as the slot type depends on the slot type passed to LookupTupleHashEntry(), which for nodeRecursiveunion.c, could be any of the current slot types. Here we fix this by adding a new parameter to BuildTupleHashTableExt() to allow the slot type to be passed in. In the case of nodeSubplan.c and nodeAgg.c the slot type is always &TTSOpsVirtual, so for both of those cases, it's beneficial to pass the known slot type as that allows ExecBuildHash32FromAttrs() to skip adding the tuple deform step to the resulting ExprState. Another possible fix would have been to have ExecBuildHash32FromAttrs() set "fetch.kind" to NULL so that ExecComputeSlotInfo() always determines the EEOP_INNER_FETCHSOME is required, however, that option isn't favorable as slows down aggregation and hashed subplan evaluation due to the extra (needless) deform step. Thanks to Nathan Bossart for bisecting to find the offending commit based on Paul's report. Reported-by: Paul Ramsey <pramsey@cleverelephant.ca> Discussion: https://postgr.es/m/99F064C1-B3EB-4BE7-97D2-D2A0AA487A71@cleverelephant.ca
* Accommodate very large dshash tables.Nathan Bossart2024-12-17
| | | | | | | | | | | | | | | | | If a dshash table grows very large (e.g., the dshash table for cumulative statistics when there are millions of tables), resizing it may fail with an error like: ERROR: invalid DSA memory alloc request size 1073741824 To fix, permit dshash resizing to allocate more than 1 GB by providing the DSA_ALLOC_HUGE flag. Reported-by: Andreas Scherbaum Author: Matthias van de Meent Reviewed-by: Cédric Villemain, Michael Paquier, Andres Freund Discussion: https://postgr.es/m/80a12d59-0d5e-4c54-866c-e69cd6536471%40pgug.de Backpatch-through: 13
* Skip useless calculation of join RTE column names during EXPLAIN.Tom Lane2024-12-17
| | | | | | | | | | | | | | | | | There's no need for set_simple_column_names() to compute unique column names for join RTEs, because a finished plan tree will not contain any join alias Vars that we could need names for. Its other, internal callers will not pass it any join RTEs anyway, so the upshot is we can just skip join RTEs here. Aside from getting rid of a klugy against-its-documentation use of set_relation_column_names, this can speed up EXPLAIN substantially when considering many-join queries, because the upper join RTEs tend to have a lot of columns. Sami Imseih, with cosmetic changes by me Discussion: https://postgr.es/m/CAA5RZ0th3q-0p1pri58z9grG8r8azmEBa8o1rtkwhLmJg_cH+g@mail.gmail.com
* Count pages set all-visible and all-frozen in VM during vacuumMelanie Plageman2024-12-17
| | | | | | | | | | | | | | | | Heap vacuum already counts and logs pages with newly frozen tuples. Now count and log the number of pages newly set all-visible and all-frozen in the visibility map. Pages that are all-visible but not all-frozen are debt for future aggressive vacuums. The counts of newly all-visible and all-frozen pages give us insight into the rate at which this debt is being accrued and paid down. Author: Melanie Plageman Reviewed-by: Masahiko Sawada, Alastair Turner, Nitin Jadhav, Andres Freund, Bilal Yavuz, Tomas Vondra Discussion: https://postgr.es/m/flat/CAAKRu_ZQe26xdvAqo4weHLR%3DivQ8J4xrSfDDD8uXnh-O-6P6Lg%40mail.gmail.com#6d8d2b4219394f774889509bf3bdc13d, https://postgr.es/m/ctdjzroezaxmiyah3gwbwm67defsrwj2b5fpfs4ku6msfpxeia%40mwjyqlhwr2wu
* Make visibilitymap_set() return previous state of vmbitsMelanie Plageman2024-12-17
| | | | | | | | | | | | It can be useful to know the state of a relation page's VM bits before visibilitymap_set(). visibilitymap_set() has the old value on hand, so returning it is simple. This commit does not use visibilitymap_set()'s new return value. Author: Melanie Plageman Reviewed-by: Masahiko Sawada, Andres Freund, Nitin Jadhav, Bilal Yavuz Discussion: https://postgr.es/m/flat/CAAKRu_ZQe26xdvAqo4weHLR%3DivQ8J4xrSfDDD8uXnh-O-6P6Lg%40mail.gmail.com#6d8d2b4219394f774889509bf3bdc13d, https://postgr.es/m/ctdjzroezaxmiyah3gwbwm67defsrwj2b5fpfs4ku6msfpxeia%40mwjyqlhwr2wu
* Rename LVRelState->frozen_pagesMelanie Plageman2024-12-17
| | | | | | | | | | | | | Rename frozen_pages to new_frozen_tuple_pages in LVRelState, the struct used for tracking state during vacuuming of a heap relation. frozen_pages sounds like it tracks pages set all-frozen. That is a misnomer. It only includes pages with at least one newly frozen tuple. It also includes pages that are not all-frozen. Author: Melanie Plageman Reviewed-by: Andres Freund, Masahiko Sawada, Nitin Jadhav, Bilal Yavuz Discussion: https://postgr.es/m/ctdjzroezaxmiyah3gwbwm67defsrwj2b5fpfs4ku6msfpxeia%40mwjyqlhwr2wu
* Set max_safe_fds whenever we create shared memory and semaphores.Tom Lane2024-12-17
| | | | | | | | | | | | | | | Formerly we skipped this in bootstrap/check mode and in single-user mode. That's bad in check mode because it may allow accepting a value of max_connections that doesn't actually work: on platforms where semaphores consume file descriptors, there may not be enough free FDs left over to satisfy fd.c, causing postmaster start to fail. It's also not great in single-user mode, because fd.c will operate with just the minimum allowable value of max_safe_fds, resulting in excess file open/close overhead if anything moderately complicated is done in single-user mode. (There may be some penalty for bootstrap mode too, though probably not much.) Discussion: https://postgr.es/m/2081982.1734393311@sss.pgh.pa.us
* Set the stack_base_ptr in main(), not in random other places.Tom Lane2024-12-17
| | | | | | | | | | | | | | | | | | | Previously we did this in PostmasterMain() and InitPostmasterChild(), which meant that stack depth checking was disabled in non-postmaster server processes, for instance in single-user mode. That seems like a fairly bad idea, since there's no a-priori restriction on the complexity of queries we will run in single-user mode. Moreover, this led to not having quite the same stack depth limit in all processes, which likely has no real-world effect but it offends my inner neatnik. Setting the depth in main() guarantees that check_stack_depth() is armed and has a consistent interpretation of stack depth in all forms of server processes. While at it, move the code associated with checking the stack depth out of tcop/postgres.c (which was never a great home for it) into a new file src/backend/utils/misc/stack_depth.c. Discussion: https://postgr.es/m/2081982.1734393311@sss.pgh.pa.us
* Detect version mismatch in brin_page_itemsTomas Vondra2024-12-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit dae761a87ed modified brin_page_items() to return the new "empty" flag for each BRIN range. But the new output parameter was added in the middle, which may cause crashes when using the new binary with old function definition. The ideal solution would be to introduce API versioning similar to what pg_stat_statements does, but it's too late for that as PG17 was already released (so we can't introduce a new extension version). We could do something similar in brin_page_items() by checking the number of output columns (and ignoring the new flag), but it doesn't seem very nice. Instead, simply error out and suggest updating the extension to the latest version. pageinspect is a superuser-only extension, and there's not much reason to run an older version. Moreover, there's a precedent for this approach in 691e8b2e18. Reported by Ľuboslav Špilák, investigation and patch by me. Backpatch to 17, same as dae761a87ed. Reported-by: Ľuboslav Špilák Reviewed-by: Michael Paquier, Hayato Kuroda, Peter Geoghegan Backpatch-through: 17 Discussion: https://postgr.es/m/VI1PR02MB63331C3D90E2104FD12399D38A5D2@VI1PR02MB6333.eurprd02.prod.outlook.com Discussion: https://postgr.es/m/flat/3385a58f-5484-49d0-b790-9a198a0bf236@vondra.me
* Update comments about index parallel buildsTomas Vondra2024-12-17
| | | | | | | | | | | | Commit b43757171470 allowed parallel builds for BRIN, but left behind two comments claiming only btree indexes support parallel builds. Reported by Egor Rogov, along with similar issues in SGML docs. Backpatch to 17, where parallel builds for BRIN were introduced. Reported-by: Egor Rogov Backpatch-through: 17 Discussion: https://postgr.es/m/114e2d5d-125e-07d8-94aa-5ad175fb7443@postgrespro.ru
* Remove ts_locale.c's lowerstr()Peter Eisentraut2024-12-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | lowerstr() and lowerstr_with_len() in ts_locale.c do the same thing as str_tolower() that the rest of the system uses, except that the former don't use the common locale provider framework but instead use the global libc locale settings. This patch replaces uses of lowerstr*() with str_tolower(..., DEFAULT_COLLATION_OID). For instances that use a libc locale globally, this will result in exactly the same behavior. For instances that use other locale providers, you now get consistent behavior and are no longer dependent on the libc locale settings (for this case; there are others). Most uses of these functions are for processing dictionary and configuration files. In those cases, using the default collation seems appropriate. At least we don't have a more specific collation available. But the code in contrib/pg_trgm should really depend on the collation of the columns being processed. This is not done here, this can be done in a separate patch. (You can probably construct some edge cases where this change would create some locale-related upgrade incompatibility, for example if before you used a combination of ICU and a differently-behaving libc locale. We can document this in the release notes, but I don't think there is anything more we can do about this.) Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://www.postgresql.org/message-id/flat/653f3b84-fc87-45a7-9a0c-bfb4fcab3e7d%40eisentraut.org
* Remove ts_locale.c's t_isdigit(), t_isspace(), t_isprint()Peter Eisentraut2024-12-17
| | | | | | | | | | | | | | | | | | | | | | | | These do the same thing as the standard isdigit(), isspace(), and isprint() but with multibyte and encoding support. But all the callers are only interested in analyzing single-byte ASCII characters. So this extra layer is overkill and we can replace the uses with the standard functions. All the t_is*() functions in ts_locale.c are under scrutiny because they don't use the common locale provider framework but instead use the global libc locale settings. For the functions being touched by this patch, we don't need all that anyway, as mentioned above, so the simplest solution is to just remove them. The few remaining t_is*() functions will need a different treatment in a separate patch. pg_trgm has some compile-time options with macros such as KEEPONLYALNUM. These are not documented, and the non-default variant is not supported by any test cases. As part of this undertaking, I'm removing the non-default variant, as it is in the way of cleanup. So in this case, the not-KEEPONLYALNUM code path is gone. Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://www.postgresql.org/message-id/flat/653f3b84-fc87-45a7-9a0c-bfb4fcab3e7d%40eisentraut.org
* Avoid unnecessary wrapping for more complex expressionsRichard Guo2024-12-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When pulling up a subquery that is under an outer join, if the subquery's target list contains a strict expression that uses a subquery variable, it's okay to pull up the expression without wrapping it in a PlaceHolderVar: if the subquery variable is forced to NULL by the outer join, the expression result will come out as NULL too. If the strict expression does not contain any subquery variables, the current code always wraps it in a PlaceHolderVar. While this is not incorrect, the analysis could be tighter: if the strict expression contains any variables of rels that are under the same lowest nulling outer join as the subquery, we can also avoid wrapping it. This is safe because if the subquery variable is forced to NULL by the outer join, the variables of rels that are under the same lowest nulling outer join will also be forced to NULL, resulting in the expression evaluating to NULL as well. Therefore, it's not necessary to force the expression to be evaluated below the outer join. It could be beneficial to get rid of such PHVs because they could imply lateral dependencies, which force us to resort to nestloop joins. This patch checks if the lateral references in the strict expression contain any variables of rels under the same lowest nulling outer join as the subquery, and avoids wrapping the expression in that case. This is fundamentally a generalization of the optimizations for bare Vars and PHVs introduced in commit f64ec81a8. No backpatch as this could result in plan changes. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs4_ENtfRdLaM_bXAxiKRYO7DmwDBDG4_2=VTDi0mJP-jAw@mail.gmail.com
* Doc: Fix the wrong link on pg_createsubscriber page.Amit Kapila2024-12-17
| | | | | | | | | | | Commit 84db9a0eb1 has added the incorrect link to 'initial data synchronization'. It was a subsection of Row Filter and didn't provide the required information. Author: Peter Smith Reviewed-by: Vignesh C, Pavel Luzanov Backpatch-through: 17, where it was introduced Discussion: https://postgr.es/m/CAHut+PtnA4DB_pcv4TDr4NjUSM1=P2N_cuZx5DX09k7LVmaqUA@mail.gmail.com
* Tweak some comments related to variable-numbered stats in pgstat.cMichael Paquier2024-12-17
| | | | | | | | | These comments referred to database objects, but depending on the stats kind dealt with this may not be true. Issues found while reviewing a different patch in this area. Discussion: https://postgr.es/m/ZtXR+CtkEVVE/LHF@ip-10-97-1-34.eu-west-3.compute.internal
* Print out error position for some more DDLsMichael Paquier2024-12-17
| | | | | | | | | | | | | | The following commands gain some information about the error position in the query, should they fail when looking at the type used: - CREATE TYPE (LIKE) - CREATE TABLE OF Both are related to typenameType() where the type name lookup is done. These calls gain the ParseState that already exists in these paths. Author: Kirill Reshke, Jian He Reviewed-by: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com
* pg_combinebackup: Fix PITR comparison test in 002_compare_backupsMichael Paquier2024-12-17
| | | | | | | | | | | | | | | | | The test was creating both the dumps to compare from the same database on the same node, so it would never detect any mismatches when comparing the logical dumps of the two servers. Fixing this issue has revealed that there is a difference in the dumps: the tablespaces paths are different. This commit uses compare_text() with a custom comparison function to erase the difference (slightly tweaked to be able to work with WIN32 and non-WIN32 paths). This way, the non-relevant parts of the tablespace path are ignored from the check with the basic structure of the query string still compared. Author: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/87h67653ns.fsf@wibble.ilmari.org Backpatch-through: 17
* doc: Mention BRIN indexes support parallel buildsTomas Vondra2024-12-16
| | | | | | | | | | | Two places in the documentation suggest B-tree is the only index access method allowing parallel builds. Commit b4375717 added parallel builds for BRIN too, but failed to update the docs. So fix that, and backpatch to 17, where parallel BRIN builds were introduced. Author: Egor Rogov Backpatch-through: 17 Discussion: https://postgr.es/m/114e2d5d-125e-07d8-94aa-5ad175fb7443@postgrespro.ru
* psql: Tab completion for JOIN ... USING column listTomas Vondra2024-12-16
| | | | | | | | | For JOIN ... USING, offer attribute names for the first member of the column list. Author: Andreas Karlsson Reviewed-By: Tomas Vondra Discussion: https://postgr.es/m/3a7e27bc-d6ed-4cb0-9b21-f21143fc1b37@proxel.se
* psql: Tab completion for JOIN ... ON/USINGTomas Vondra2024-12-16
| | | | | | | | | Offer ON/USING clauses for join types that require join conditions (i.e. anything except for NATURAL/CROSS joins). Author: Andreas Karlsson Reviewed-By: Tomas Vondra Discussion: https://postgr.es/m/3a7e27bc-d6ed-4cb0-9b21-f21143fc1b37@proxel.se
* psql: Tab completion for LATERAL joinsTomas Vondra2024-12-16
| | | | | | | | When listing selectable objects after a JOIN, offer also LATERAL. Author: Andreas Karlsson Reviewed-By: Tomas Vondra Discussion: https://postgr.es/m/3a7e27bc-d6ed-4cb0-9b21-f21143fc1b37@proxel.se
* Refactor string case conversion into provider-specific files.Jeff Davis2024-12-16
| | | | | | | | | Create API entry points pg_strlower(), etc., that work with any provider and give the caller control over the destination buffer. Then, move provider-specific logic into pg_locale_builtin.c, pg_locale_icu.c, and pg_locale_libc.c as appropriate. Discussion: https://postgr.es/m/7aa46d77b377428058403723440862d12a8a129a.camel@j-davis.com
* psql: Tab completion for CREATE MATERIALIZED VIEW ... USINGTomas Vondra2024-12-16
| | | | | | | | | The tab completion didn't offer USING for CREATE MATERIALIZED VIEW, so add it, and offer a list of access methods, followed by SELECT. Author: Kirill Reshke Reviewed-By: Karina Litskevich Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com
* psql: Tab completion for CREATE TEMP TABLE ... USINGTomas Vondra2024-12-16
| | | | | | | | | The USING keyword was offered only for persistent tables, not for temporary ones. So improve that. Author: Kirill Reshke Reviewed-By: Karina Litskevich Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com
* psql: Tab completion for ALTER TYPE ... CASCADE/RESTRICTTomas Vondra2024-12-16
| | | | | | | | | | | | Updates table completion for ALTER TYPE to offer CASCADE/RESTRICT for a number of actions on attributes: ALTER TYPE ... ADD/DROP/RENAME ATTRIBUTE ... [CASCADE|RESTRICT] ALTER TYPE ... TYPE ... [CASCADE|RESTRICT] Author: Kirill Reshke Reviewed-By: Karina Litskevich Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com
* psql: Tab completion for ALTER TYPE ... ADD ATTRIBUTETomas Vondra2024-12-16
| | | | | | | | | Improve psql tab completion for ALTER TYPE ... ADD ATTRIBUTE to offer a list of existing data types (until now no options were offered). Author: Kirill Reshke Reviewed-By: Karina Litskevich Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com
* Make 009_twophase.pl test pass with recovery_min_apply_delay setHeikki Linnakangas2024-12-16
| | | | | | | | | | | | | | The test failed if you ran the regression tests with TEMP_CONFIG with recovery_min_apply_delay = '500ms'. Fix the race condition by waiting for transaction to be applied in the replica, like in a few other tests. The failing test was introduced in commit cbfbda7841. Backpatch to all supported versions like that commit (except v12, which is no longer supported). Reported-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/09e2a70a-a6c2-4b5c-aeae-040a7449c9f2@gmail.com
* Print out error position for CREATE DOMAINMichael Paquier2024-12-16
| | | | | | | | | | | | | This is simply done by pushing down the ParseState available in ProcessUtility() to DefineDomain(), giving more information about the position of an error when running a CREATE DOMAIN query. Most of the queries impacted by this change have been added previously in 0172b4c9449e. Author: Kirill Reshke, Jian He Reviewed-by: Álvaro Herrera, Tom Lane, Michael Paquier Discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com
* Add some tests for encoding conversion in COPY TO/FROMMichael Paquier2024-12-16
| | | | | | | | | | | | | | | This adds a couple of tests to trigger encoding conversion when input and server encodings do not match in COPY FROM/TO, or need_transcoding set to true in the COPY state data. These tests rely on UTF8 <-> LATIN1 for the valid cases as LATIN1 accepts any bytes, and UTF8 <-> EUC_JP for some of the invalid cases where a character cannot be understood, causing a conversion failure. Both ENCODING and client_encoding are covered. Test suggested by Andres Freund. Author: Sutou Kouhei Discussion: https://postgr.es/m/20240206222445.hzq22pb2nye7rm67@awork3.anarazel.de
* Declare a couple of variables inside not outside a PG_TRY block.Tom Lane2024-12-15
| | | | | | | | | | | | | I went through the buildfarm's reports of "warning: variable 'foo' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]". As usual, none of them are live problems according to my understanding of the effects of setjmp/longjmp, to wit that local variables might revert to their values as of PG_TRY entry, due to being kept in registers. But I did happen to notice that XmlTableGetValue's "cstr" variable doesn't need to be declared outside the PG_TRY block at all (thus giving further proof that the -Wclobbered warning has little connection to real problems). We might as well move it inside, and "cur" too, in hopes of eliminating one of the bogus warnings.
* pgbench: fix misprocessing of some nested \if constructs.Tom Lane2024-12-15
| | | | | | | | | | | | | | An \if command appearing within a false (not-to-be-executed) \if branch was incorrectly treated the same as \elif. This could allow statements within the inner \if to be executed when they should not be. Also the missing inner \if stack entry would result in an assertion failure (in assert-enabled builds) when the final \endif is reached. Report and patch by Michail Nikolaev. Back-patch to all supported branches. Discussion: https://postgr.es/m/CANtu0oiA1ke=SP6tauhNqkUdv5QFsJtS1p=aOOf_iU+EhyKkjQ@mail.gmail.com
* doc: Clarify old WAL files are kept until they are summarized.Fujii Masao2024-12-15
| | | | | | | | | | | | | | | The documentation in wal.sgml explains that old WAL files cannot be removed or recycled until they are archived (when WAL archiving is used) or replicated (when using replication slots). However, it did not mention that, similarly, old WAL files are also kept until they are summarized if WAL summarization is enabled. This commit adds that clarification to the documentation. Back-patch to v17 where WAL summarization was added. Author: Fujii Masao Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/fd0eb0a5-f43b-4e06-b450-cbca011b6cff@oss.nttdata.com
* contrib/earthdistance: Use SQL-standard function bodies.Tom Lane2024-12-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The @extschema:name@ feature added by 72a5b1fc8 allows us to make earthdistance's references to the cube extension fully search-path-secure, so long as all those references are resolved at extension installation time not runtime. To do that, we must convert earthdistance's SQL functions to the new SQL-standard style; but we wanted to do that anyway. The functions can be updated in our customary style by running CREATE OR REPLACE FUNCTION in an extension update script. However, there's still problems in the "CREATE DOMAIN earth" command: its references to cube functions could be captured by hostile objects in earthdistance's installation schema, if that's not where the cube extension is. Worse, the reference to the cube type itself as the domain's base could be captured, and that's not something we could fix after-the-fact in the update script. What I've done about that is to change the "CREATE DOMAIN earth" command in the base script earthdistance--1.1.sql. Ordinarily, changing a released extension script is forbidden; but I think it's okay here since the results of successful (non-trojaned) script execution will be identical to before. A good deal of care is still needed to make the extension's scripts proof against search-path-based attacks. We have to make sure that all the function and operator invocations have exact argument-type matches, to forestall attacks based on supplying a better match. Fortunately earthdistance isn't very big, so I've just gone through it and inspected each call to be sure of that. The only actual code changes needed were to spell all floating-point constants in the style '-1'::float8, rather than depending on runtime type conversions and/or negations. (I'm not sure that the shortcuts previously used were attackable, but removing run-time effort is a good thing anyway.) I believe that this fixes earthdistance enough that we could mark it trusted and remove the warnings about it that were added by 7eeb1d986; but I've not done that here. The primary reason for dealing with this now is that we've received reports of pg_upgrade failing for databases that use earthdistance functions in contexts like generated columns. That's a consequence of 2af07e2f7 having restricted the search_path used while evaluating such expressions. The only way to fix that is to make the earthdistance functions independent of run-time search_path. This patch is very much nicer than the alternative of attaching "SET search_path" clauses to earthdistance's functions: it is more secure and doesn't create a run-time penalty. Therefore, I've chosen to back-patch this to v16 where @extschema:name@ was added. It won't help unless users update to 16.7 and issue "ALTER EXTENSION earthdistance UPDATE" before upgrading to 17, but at least there's now a way to deal with the problem without manual intervention in the dump/restore process. Tom Lane and Ronan Dunklau Discussion: https://postgr.es/m/3316564.aeNJFYEL58@aivenlaptop Discussion: https://postgr.es/m/6a6439f1-8039-44e2-8fb9-59028f7f2014@mailbox.org