aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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
* 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
* 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
* 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
* 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
* Refactor some SQL/JSON error messagesÁlvaro Herrera2024-12-14
| | | | | Turn type names into "%s" specifiers to 1) avoid getting them translated and 2) reduce the total number of messages.
* Fix warnings about declaration of environ on MinGW.Thomas Munro2024-12-15
| | | | | | | | | | | | | | | | | | POSIX says that the global variable environ shouldn't be declared in a header, and that you have to declare it yourself. MinGW declares it in <stdlib.h> with some macrology that messes up our declarations. Visual Studio doesn't warn (there are clues that it may also declare it, but if so, apparently compatibly). Suppress our declarations, on MinGW only. This clears the last warnings on CI's optional MinGW task, and hopefully on build farm animal fairywren too. Like 1319997d, no back-patch for now as it's not known to be breaking anything, and my humble goal is just to keep the MinGW build clean going forward. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (earlier version) Discussion: https://postgr.es/m/CA%2BhUKGJLMh%2B6W5E4M_jSFb43gnrA_-Q6-%2BBf3HkBXyGfRFcBsQ%40mail.gmail.com
* Remove EXTENSION_DONT_CHECK_SIZE from md.c.Thomas Munro2024-12-14
| | | | | | | | | | | | Commits 7bb3102c and 3eb77eba removed the only user of the EXTENSION_DONT_CHECK_SIZE flag, which had previously been required to checkpoint truncated relations. Since 7bb3102c, segments have been opened directly for synchronization without calling _mdfd_getseg(), so it doesn't need a mode that tolerates non-final short segments. Remove the redundant flag and associated comments. Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/nyj4k7yur5t27rtygvx2i2lrlp6rqfvvhoiiwx4fznynksf2et%404hj2sp42alpe
* Fix typoJohn Naylor2024-12-14
| | | | | | Ryo Kanbayashi Discussion: https://postgr.es/m/CANOn0ExEQiPVrzkjULkENVac_n4Lknm6dxsU69MSncQap0kJVA%40mail.gmail.com
* Fix possible crash in pg_dump with identity sequences.Tom Lane2024-12-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If an owned sequence is considered interesting, force its owning table to be marked interesting too. This ensures, in particular, that we'll fetch the owning table's column names so we have the data needed for ALTER TABLE ... ADD GENERATED. Previously there were edge cases where pg_dump could get SIGSEGV due to not having filled in the column names. (The known case is where the owning table has been made part of an extension while its identity sequence is not a member; but there may be others.) Also, if it's an identity sequence, force its dumped-components mask to exactly match the owning table: dump definition only if we're dumping the table's definition, dump data only if we're dumping the table's data, etc. This generalizes the code introduced in commit b965f2617 that set the sequence's dump mask to NONE if the owning table's mask is NONE. That's insufficient to prevent failures, because for example the table's mask might only request dumping ACLs, which would lead us to still emit ALTER TABLE ADD GENERATED even though we didn't create the table. It seems better to treat an identity sequence as though it were an inseparable aspect of the table, matching the treatment used in the backend's dependency logic. Perhaps this policy needs additional refinement, but let's wait to see some field use-cases before changing it further. While here, add a comment in pg_dump.h warning against writing tests like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this case. There is one other example in getPublicationNamespaces, which if it's not a bug is at least remarkably unclear and under-documented. Changing that requires a separate discussion, however. Per report from Artur Zakirov. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com
* Rewrite maybe_reread_subscription() commentÁlvaro Herrera2024-12-13
| | | | One sentence was gramatically wrong, but also too terse. Expand on it.
* Dump not-null constraints on inherited columns correctlyÁlvaro Herrera2024-12-13
| | | | | | | | | | | | | | | | | | | | | | | | | | With not-null constraints defined in child tables for columns that are coming from their parent tables, we were printing ALTER TABLE SET NOT NULL commands that were missing the constraint name, so the original constraint name was being lost, which is bogus. Fix by instead adding a table-constraint constraint declaration with the correct constraint name in the CREATE TABLE instead. Oversight in commit 14e87ffa5c54. We could have fixed it by changing the ALTER TABLE SET NOT NULL to ALTER TABLE ADD CONSTRAINT, but I'm not sure that's any better. A potential problem here might be that if sent to a non-Postgres server, the new pg_dump output would fail because the "CONSTRAINT foo NOT NULL colname" syntax isn't SQL-conforming. However, Postgres' implementation of inheritance is already non-SQL-conforming, so that'd likely fail anyway. This problem was only noticed by Ashutosh's proposed test framework for pg_dump, https://postgr.es/m/CAExHW5uF5V=Cjecx3_Z=7xfh4rg2Wf61PT+hfquzjBqouRzQJQ@mail.gmail.com Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CAExHW5tbdgAKDfqjDJ-7Fk6PJtHg8D4zUF6FQ4H2Pq8zK38Nyw@mail.gmail.com
* Revert "Don't truncate database and user names in startup packets."Nathan Bossart2024-12-12
| | | | | | | | | | | | | | | This reverts commit 562bee0fc13dc95710b8db6a48edad2f3d052f2e. We received a report from the field about this change in behavior, so it seems best to revert this commit and to add proper multibyte-aware truncation as a follow-up exercise. Fixes bug #18711. Reported-by: Adam Rauch Reviewed-by: Tom Lane, Bertrand Drouvot, Bruce Momjian, Thomas Munro Discussion: https://postgr.es/m/18711-7503ee3e449d2c47%40postgresql.org Backpatch-through: 17
* Adjust some comments about structure properties in pg_stat.hMichael Paquier2024-12-12
| | | | | | | | | | | | | | | | | | One comment of PgStat_TableCounts mentioned that its pending stats use memcmp() to check for the all-zero case if there is any activity. This is not true since 07e9e28b56, as pg_memory_is_all_zeros() is used. PgStat_FunctionCounts incorrectly documented that it relied on memcpy(). This has never been correct, and not relevant because function statistics do not have an all-zero check for pending stats. Checkpoint and bgwriter statistics have been always relying on memcmp() or pg_memory_is_all_zeros() (since 07e9e28b56 for the latter), and never mentioned the dependency on event counters for their all-zero checks. Let's document these properties, like the table statistics. Author: Bertrand Drouvot Discussion: https://postgr.es/m/Z1hNLvcPgVLPxCoc@ip-10-97-1-34.eu-west-3.compute.internal
* Detect redundant GROUP BY columns using UNIQUE indexesDavid Rowley2024-12-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | d4c3a156c added support that when the GROUP BY contained all of the columns belonging to a relation's PRIMARY KEY, all other columns belonging to that relation would be removed from the GROUP BY clause. That's possible because all other columns are functionally dependent on the PRIMARY KEY and those columns alone ensure the groups are distinct. Here we expand on that optimization and allow it to work for any unique indexes on the table rather than just the PRIMARY KEY index. This normally requires that all columns in the index are defined with NOT NULL, however, we can relax that requirement when the index is defined with NULLS NOT DISTINCT. When there are multiple suitable indexes to allow columns to be removed, we prefer the index with the least number of columns as this allows us to remove the highest number of GROUP BY columns. One day, we may want to revisit that decision as it may make more sense to use the narrower set of columns in terms of the width of the data types and stored/queried data. This also adjusts the code to make use of RelOptInfo.indexlist rather than looking up the catalog tables. In passing, add another short-circuit path to allow bailing out earlier in cases where it's certainly not possible to remove redundant GROUP BY columns. This early exit is now cheaper to do than when this code was originally written as 00b41463c made it cheaper to check for empty Bitmapsets. Patch originally by Zhang Mingli and later worked on by jian he, but after I (David) worked on it, there was very little of the original left. Author: Zhang Mingli, jian he, David Rowley Reviewed-by: jian he, Andrei Lepikhov Discussion: https://postgr.es/m/327990c8-b9b2-4b0c-bffb-462249f82de0%40Spark
* Improve the test case from 5668a857dRichard Guo2024-12-12
| | | | | | | | | | | | | | In commit 5668a857d, we fixed an issue with incorrect results in right semi joins and introduced a test case to verify the fix. The test case involves SubPlans and InitPlans, which may not be immediately apparent in relation to the issue we addressed. This patch simplifies the test case with a more straightforward query. Per discussion with Melanie Plageman. Author: Richard Guo Discussion: https://postgr.es/m/CAAKRu_a-Cip2XCXp13fmxq+T9BhLAVApHTyjr94awL2mbXHC-Q@mail.gmail.com
* Add some regression tests for missing DDL patternsMichael Paquier2024-12-12
| | | | | | | | | | | | | | | | | The following commands gain increased coverage for some of the errors they can trigger: - ALTER TABLE .. ALTER COLUMN - CREATE DOMAIN - CREATE TYPE (LIKE) This has come up while discussing the possibility to add more information about the location of the error in such queries, and it is useful on its own as there was no coverage until now for the patterns added in this commit. Author: Jian He, Kirill Reshke Reviewed-By: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yEePi_aq61GmMpW88i6ZH7CMG_2Z4Cg@mail.gmail.com
* Defer remove_useless_groupby_columns() work until query_planner()David Rowley2024-12-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Traditionally, remove_useless_groupby_columns() was called during grouping_planner() directly after the call to preprocess_groupclause(). While in many ways, it made sense to populate the field and remove the functionally dependent columns from processed_groupClause at the same time, it's just that doing so had the disadvantage that remove_useless_groupby_columns() was being called before the RelOptInfos were populated for the relations mentioned in the query. Not having RelOptInfos available meant we needed to manually query the catalog tables to get the required details about the primary key constraint for the table. Here we move the remove_useless_groupby_columns() call to query_planner() and put it directly after the RelOptInfos are populated. This is fine to do as processed_groupClause still isn't final at this point as it can still be modified inside standard_qp_callback() by make_pathkeys_for_sortclauses_extended(). This commit is just a refactor and simply moves remove_useless_groupby_columns() into initsplan.c. A planned follow-up commit will adjust that function so it uses RelOptInfo instead of doing catalog lookups and also teach it how to use unique indexes as proofs to expand the cases where we can remove functionally dependent columns from the GROUP BY. Reviewed-by: Andrei Lepikhov, jian he Discussion: https://postgr.es/m/CAApHDvqLezKwoEBBQd0dp4Y9MDkFBDbny0f3SzEeqOFoU7Z5+A@mail.gmail.com
* Add UUID version 7 generation function.Masahiko Sawada2024-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit introduces the uuidv7() SQL function, which generates UUID version 7 as specified in RFC 9652. UUIDv7 combines a Unix timestamp in milliseconds and random bits, offering both uniqueness and sortability. In our implementation, the 12-bit sub-millisecond timestamp fraction is stored immediately after the timestamp, in the space referred to as "rand_a" in the RFC. This ensures additional monotonicity within a millisecond. The rand_a bits also function as a counter. We select a sub-millisecond timestamp so that it monotonically increases for generated UUIDs within the same backend, even when the system clock goes backward or when generating UUIDs at very high frequency. Therefore, the monotonicity of generated UUIDs is ensured within the same backend. This commit also expands the uuid_extract_timestamp() function to support UUID version 7. Additionally, an alias uuidv4() is added for the existing gen_random_uuid() SQL function to maintain consistency. Bump catalog version. Author: Andrey Borodin Reviewed-by: Sergey Prokhorenko, Przemysław Sztoch, Nikolay Samokhvalov Reviewed-by: Peter Eisentraut, Jelte Fennema-Nio, Aleksander Alekseev Reviewed-by: Masahiko Sawada, Lukas Fittl, Michael Paquier, Japin Li Reviewed-by: Marcos Pegoraro, Junwang Zhao, Stepan Neretin Reviewed-by: Daniel Vérité Discussion: https://postgr.es/m/CAAhFRxitJv%3DyoGnXUgeLB_O%2BM7J2BJAmb5jqAT9gZ3bij3uLDA%40mail.gmail.com
* Use pg_memory_is_all_zeros() in pgstatfuncs.c.Nathan Bossart2024-12-11
| | | | | | | | | | | | | There are a few places in this file that use memset() and memcmp() to determine whether a section of memory is all zeros. This commit modifies them to use pg_memory_is_all_zeros() instead. These aren't expected to be hot code paths, but this may optimize them a bit. Plus, this allows us to remove some variables that were only needed for the memset() and memcmp(). Author: Bertrand Drouvot Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/Z1hNubHfvMxlW6eu%40ip-10-97-1-34.eu-west-3.compute.internal