aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Pass MyPMChildSlot as an explicit argument to child processHeikki Linnakangas2024-11-14
| | | | | | | | | All the other global variables passed from postmaster to child have the same value in all the processes, while MyPMChildSlot is more like a parameter to each child process. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi
* Assign a child slot to every postmaster child processHeikki Linnakangas2024-11-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, only backends, autovacuum workers, and background workers had an entry in the PMChildFlags array. With this commit, all postmaster child processes, including all the aux processes, have an entry. Dead-end backends still don't get an entry, though, and other processes that don't touch shared memory will never mark their PMChildFlags entry as active. We now maintain separate freelists for different kinds of child processes. That ensures that there are always slots available for autovacuum and background workers. Previously, pre-authentication backends could prevent autovacuum or background workers from starting up, by using up all the slots. The code to manage the slots in the postmaster process is in a new pmchild.c source file. Because postmaster.c is just so large. Assigning pmsignal slot numbers is now pmchild.c's responsibility. This replaces the PMChildInUse array in pmsignal.c. Some of the comments in postmaster.c still talked about the "stats process", but that was removed in commit 5891c7a8ed. Fix those while we're at it. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi
* Kill dead-end children when there's nothing else leftHeikki Linnakangas2024-11-14
| | | | | | | | | | | | | | | | Previously, the postmaster would never try to kill dead-end child processes, even if there were no other processes left. A dead-end backend will eventually exit, when authentication_timeout expires, but if a dead-end backend is the only thing that's preventing the server from shutting down, it seems better to kill it immediately. It's particularly important, if there was a bug in the early startup code that prevented a dead-end child from timing out and exiting normally. Includes a test for that case where a dead-end backend previously prevented the server from shutting down. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi
* Replace postmaster.c's own backend type codes with BackendTypeHeikki Linnakangas2024-11-14
| | | | | | | | Introduce a separate BackendType for dead-end children, so that we don't need a separate dead_end flag. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/a102f15f-eac4-4ff2-af02-f9ff209ec66f@iki.fi
* Remove a useless cast to (void *) in hash_search() callPeter Eisentraut2024-11-14
| | | | | This pattern was previously cleaned up in 54a177a948b, but a new instance snuck in around the same time in 31966b151e6.
* contrib/lo: Use SQL-standard function bodiesMichael Paquier2024-11-14
| | | | | Author: Ronan Dunklau Discussion: https://postgr.es/m/3316564.aeNJFYEL58@aivenlaptop
* xml2: Add tests for functions xpath_nodeset() and xpath_list()Michael Paquier2024-11-14
| | | | | | | | These two functions with their different argument lists have never been tested in this module, so let's add something. Author: Ronan Dunklau Discussion: https://postgr.es/m/ZzMSJkiNZhimjXWx@paquier.xyz
* contrib/lo: Add test for function lo_oid()Michael Paquier2024-11-14
| | | | | Author: Ronan Dunklau Discussion: https://postgr.es/m/ZzMSJkiNZhimjXWx@paquier.xyz
* Add nbtree amgettuple return item function.Peter Geoghegan2024-11-13
| | | | | | | | | | This makes it easier to add precondition assertions. We now assert that the last call to _bt_readpage succeeded, and that the current item index is within the bounds of the currPos items array. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com> Discussion: https://postgr.es/m/CAH2-WznFkEs9K1PtNruti5JjawY-dwj+gkaEh_k1ZE+1xLLGkA@mail.gmail.com
* Fix pg_upgrade's cross-version tests when old < 18Álvaro Herrera2024-11-13
| | | | | | | | | | | | | | Because in the 18 cycle we turned checksums on by default with commit 04bec894a04c, and pg_upgrade fails if the setting doesn't match in old and new clusters, the built-in cross-version pg_upgrade test is failing if the old version is older than 18. Fix the script so that it creates the old cluster with checksums enabled (-k) in cross-version scenarios. This went unnoticed because the buildfarm doesn't use the same test code for cross-version testing. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/202411071838.7fgkb7uvavvz@alvherre.pgsql
* configure.ac: Remove useless AC_SUBSTPeter Eisentraut2024-11-13
| | | | No longer used since commit 805e431a386.
* doc: Update pg_constraint.conexclop docs for WITHOUT OVERLAPSPeter Eisentraut2024-11-13
| | | | | | | Fixup for commit fc0438b4e80. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Discussion: https://www.postgresql.org/message-id/57ea0668-5205-426e-b934-efc89f2186c2@illuminatedcomputing.com
* doc: Add PERIOD to ALTER TABLE reference docsPeter Eisentraut2024-11-13
| | | | | | | | | Commit 89f908a6d0a documented foreign keys with PERIOD in the CREATE TABLE docs, but not in ALTER TABLE. This commit adds the new syntax to the ALTER TABLE docs. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Discussion: https://www.postgresql.org/message-id/57ea0668-5205-426e-b934-efc89f2186c2@illuminatedcomputing.com
* doc: Small improvement in CREATE TABLE / PERIOD documentationPeter Eisentraut2024-11-13
| | | | | Use placeholders that are more consistent and match the description better. Fixup for commit 89f908a6d0a.
* doc: Add WITHOUT OVERLAPS to ALTER TABLE reference docsPeter Eisentraut2024-11-13
| | | | | | | | | Commit fc0438b4e80 documented WITHOUT OVERLAPS in the CREATE TABLE docs, but not in ALTER TABLE. This commit adds the new syntax to the ALTER TABLE docs. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Discussion: https://www.postgresql.org/message-id/57ea0668-5205-426e-b934-efc89f2186c2@illuminatedcomputing.com
* Fix comment in injection_point.cMichael Paquier2024-11-13
| | | | | | | | | | InjectionPointEntry->name was described as a hash key, which was fine when introduced in d86d20f0ba79, but it is not now. Oversight in 86db52a5062a, that has changed the way injection points are stored in shared memory from a hash table to an array. Backpatch-through: 17
* Fix obsolete nbtree page reuse FSM comment.Peter Geoghegan2024-11-12
| | | | Oversight in commit d088ba5a.
* Count contrib/bloom index scans in pgstat view.Peter Geoghegan2024-11-12
| | | | | | | | | | | | | Maintain the pg_stat_user_indexes.idx_scan pgstat counter during contrib/Bloom index scans. Oversight in commit 9ee014fc, which added the Bloom index contrib module. Author: Masahiro Ikeda <ikedamsh@oss.nttdata.com> Reviewed-By: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/c48839d881388ee401a01807c686004d@oss.nttdata.com Backpatch: 13- (all supported branches).
* Add missing word in commentAmit Langote2024-11-12
| | | | Discussion: https://postgr.es/m/CA+HiwqFgdp8=0_gi+DU0fPWZbg7qY3KZ_c1Wj1DEvzXC4BCnMQ@mail.gmail.com
* Silence compilers about extractNotNullColumn()Álvaro Herrera2024-11-12
| | | | | | | | | | | | | | | Multiple buildfarm animals warn that a newly added Assert() is impossible to fail; remove it to avoid the noise. While at it, use direct assignment to obtain the value we need, avoiding an unnecessary memcpy(). (I decided to remove the "pfree" call for the detoasted short-datum; because this is only used for DDL, it's not problematic to leak such a small allocation.) Noted by Tom Lane about 14e87ffa5c54. Discussion: https://postgr.es/m/3649828.1731083171@sss.pgh.pa.us
* pg_freespacemap: Use SQL-standard function bodiesMichael Paquier2024-11-12
| | | | | | | | | | | | | 72a5b1fc8804 was the piece missing for the conversion of this module. pg_freespace is bumped to 1.3, with its function pg_freespace(regclass) converted to this new style. There are other modules in the tree that need a similar treatment; these will be handled later. Author: Tom Lane Reviewed-by: Ronan Dunklau Discussion: https://postgr.es/m/3395418.1618352794@sss.pgh.pa.us
* Fix arrays comparison in CompareOpclassOptions()Alexander Korotkov2024-11-12
| | | | | | | | | | | | | The current code calls array_eq() and does not provide FmgrInfo. This commit provides initialization of FmgrInfo and uses C collation as the safe option for text comparison because we don't know anything about the semantics of opclass options. Backpatch to 13, where opclass options were introduced. Reported-by: Nicolas Maus Discussion: https://postgr.es/m/18692-72ea398df3ec6712%40postgresql.org Backpatch-through: 13
* Parallel workers use AuthenticatedUserId for connection privilege checks.Tom Lane2024-11-11
| | | | | | | | | | | | | | | | | | | | | Commit 5a2fed911 had an unexpected side-effect: the parallel worker launched for the new test case would fail if it couldn't use a superuser-reserved connection slot. The reason that test failed while all our pre-existing ones worked is that the connection privilege tests in InitPostgres had been based on the superuserness of the leader's AuthenticatedUserId, but after the rearrangements of 5a2fed911 we were testing the superuserness of CurrentUserId, which the new test case deliberately made to be a non-superuser. This all seems very accidental and probably not the behavior we really want, but a security patch is no time to be redesigning things. Pending some discussion about desirable semantics, hack it so that InitPostgres continues to pay attention to the superuserness of AuthenticatedUserId when starting a parallel worker. Nathan Bossart and Tom Lane, per buildfarm member sawshark. Security: CVE-2024-10978
* Fix cross-version upgrade tests.Tom Lane2024-11-11
| | | | | | | | | | | | | | TestUpgradeXversion knows how to make the main regression database's references to pg_regress.so be version-independent. But it doesn't do that for plperl's database, so that the C function added by commit b7e3a52a8 is causing cross-version upgrade test failures. Path of least resistance is to just drop the function at the end of the new test. In <= v14, also take the opportunity to clean up the generated test files. Security: CVE-2024-10979
* Avoid bizarre meson behavior with backslashes in command arguments.Tom Lane2024-11-11
| | | | | | | | | | | meson makes the backslashes in text2macro.pl's --strip argument into forward slashes, effectively disabling comment stripping. That hasn't caused us issues before, but it breaks the test case for b7e3a52a8. We don't really need the pattern to be adjustable, so just hard-wire it into the script instead. Context: https://github.com/mesonbuild/meson/issues/1564 Security: CVE-2024-10979
* Fix improper interactions between session_authorization and role.Tom Lane2024-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The SQL spec mandates that SET SESSION AUTHORIZATION implies SET ROLE NONE. We tried to implement that within the lowest-level functions that manipulate these settings, but that was a bad idea. In particular, guc.c assumes that it doesn't matter in what order it applies GUC variable updates, but that was not the case for these two variables. This problem, compounded by some hackish attempts to work around it, led to some security-grade issues: * Rolling back a transaction that had done SET SESSION AUTHORIZATION would revert to SET ROLE NONE, even if that had not been the previous state, so that the effective user ID might now be different from what it had been. * The same for SET SESSION AUTHORIZATION in a function SET clause. * If a parallel worker inspected current_setting('role'), it saw "none" even when it should see something else. Also, although the parallel worker startup code intended to cope with the current role's pg_authid row having disappeared, its implementation of that was incomplete so it would still fail. Fix by fully separating the miscinit.c functions that assign session_authorization from those that assign role. To implement the spec's requirement, teach set_config_option itself to perform "SET ROLE NONE" when it sets session_authorization. (This is undoubtedly ugly, but the alternatives seem worse. In particular, there's no way to do it within assign_session_authorization without incompatible changes in the API for GUC assign hooks.) Also, improve ParallelWorkerMain to directly set all the relevant user-ID variables instead of relying on some of them to get set indirectly. That allows us to survive not finding the pg_authid row during worker startup. In v16 and earlier, this includes back-patching 9987a7bf3 which fixed a violation of GUC coding rules: SetSessionAuthorization is not an appropriate place to be throwing errors from. Security: CVE-2024-10978
* Ensure cached plans are correctly marked as dependent on role.Nathan Bossart2024-11-11
| | | | | | | | | | | | | | If a CTE, subquery, sublink, security invoker view, or coercion projection references a table with row-level security policies, we neglected to mark the plan as potentially dependent on which role is executing it. This could lead to later executions in the same session returning or hiding rows that should have been hidden or returned instead. Reported-by: Wolfgang Walther Reviewed-by: Noah Misch Security: CVE-2024-10976 Backpatch-through: 12
* Block environment variable mutations from trusted PL/Perl.Noah Misch2024-11-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many process environment variables (e.g. PATH), bypass the containment expected of a trusted PL. Hence, trusted PLs must not offer features that achieve setenv(). Otherwise, an attacker having USAGE privilege on the language often can achieve arbitrary code execution, even if the attacker lacks a database server operating system user. To fix PL/Perl, replace trusted PL/Perl %ENV with a tied hash that just replaces each modification attempt with a warning. Sites that reach these warnings should evaluate the application-specific implications of proceeding without the environment modification: Can the application reasonably proceed without the modification? If no, switch to plperlu or another approach. If yes, the application should change the code to stop attempting environment modifications. If that's too difficult, add "untie %main::ENV" in any code executed before the warning. For example, one might add it to the start of the affected function or even to the plperl.on_plperl_init setting. In passing, link to Perl's guidance about the Perl features behind the security posture of PL/Perl. Back-patch to v12 (all supported versions). Andrew Dunstan and Noah Misch Security: CVE-2024-10979
* Doc: Add links to clarify the max_replication_slots.Amit Kapila2024-11-11
| | | | | | | | The GUC max_replication_slots has a different meaning for sending servers and subscribers. Add cross-links in each section for easy reference. Author: Tristan Partin Discussion: https://postgr.es/m/D5FNEPMMFHFX.1OQBCML0TU5AH@partin.io
* Add two attributes to pg_stat_database for parallel workers activityMichael Paquier2024-11-11
| | | | | | | | | | | | | | | | | | | | | | Two attributes are added to pg_stat_database: * parallel_workers_to_launch, counting the total number of parallel workers that were planned to be launched. * parallel_workers_launched, counting the total number of parallel workers actually launched. The ratio of both fields can provide hints that there are not enough slots available when launching parallel workers, also useful when pg_stat_statements is not deployed on an instance (i.e. cf54a2c00254). This commit relies on de3a2ea3b264, that has added two fields to EState, that get incremented when executing Gather or GatherMerge nodes. A test is added in select_parallel, where parallel workers are spawned. Bump catalog version. Author: Benoit Lobréau Discussion: https://postgr.es/m/783bc7f7-659a-42fa-99dd-ee0565644e25@dalibo.com
* libpq: Bail out during SSL/GSS negotiation errorsMichael Paquier2024-11-11
| | | | | | | | | | | | | | | | | | | | | | | This commit changes libpq so that errors reported by the backend during the protocol negotiation for SSL and GSS are discarded by the client, as these may include bytes that could be consumed by the client and write arbitrary bytes to a client's terminal. A failure with the SSL negotiation now leads to an error immediately reported, without a retry on any other methods allowed, like a fallback to a plaintext connection. A failure with GSS discards the error message received, and we allow a fallback as it may be possible that the error is caused by a connection attempt with a pre-11 server, GSS encryption having been introduced in v12. This was a problem only with v17 and newer versions; older versions discard the error message already in this case, assuming a failure caused by a lack of support for GSS encryption. Author: Jacob Champion Reviewed-by: Peter Eisentraut, Heikki Linnakangas, Michael Paquier Security: CVE-2024-10977 Backpatch-through: 12
* pg_stat_statements: Avoid some locking during PGSS entry scansMichael Paquier2024-11-11
| | | | | | | | | | | | | | | | | | | | | | A single PGSS entry's spinlock is used to be able to modify "counters" without holding pgss->lock exclusively, as mentioned at the top of pg_stat_statements.c and within pgssEntry. Within a single pgssEntry, stats_since and minmax_stats_since are never modified without holding pgss->lock exclusively, so there is no need to hold an entry's spinlock when reading stats_since and minmax_stats_since, as done when scanning all the PGSS entries for function calls of pg_stat_statements(). This also restores the consistency between the code and the comments about the entry's spinlock usage. This change is a performance improvement (it can be argued that this is a logic bug), so there is no need for a backpatch. This saves two instructions from being read while holding an entry's spinlock. Author: Karina Litskevich Reviewed-by: Michael Paquier, wenhui qiu Discussion: https://postgr.es/m/CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com
* jit: Remove obsolete LLVM version guard.Thomas Munro2024-11-11
| | | | | | Commit 9044fc1d needed a version guard when back-patched, but it is redundant in master as of commit 972c2cd2, and I accidentally left it in there.
* Fix sign-compare warnings in pg_iovec.h.Nathan Bossart2024-11-08
| | | | | | | | | | | | | | The code in question (pg_preadv() and pg_pwritev()) has been around for a while, but commit 15c9ac3629 moved it to a header file. If third-party code that includes this header file is built with -Wsign-compare on a system without preadv() or pwritev(), warnings ensue. This commit fixes said warnings by casting the result of pg_pread()/pg_pwrite() to size_t, which should be safe because we will have already checked for a negative value. Author: Wolfgang Walther Discussion: https://postgr.es/m/16989737-1aa8-48fd-8dfe-b7ada06509ab%40technowledgy.de Backpatch-through: 17
* Assert consistency of currPage that ended scan.Peter Geoghegan2024-11-08
| | | | | | | | | | | | | | | | | When _bt_readnextpage is called with our nbtree parallel scan already seized (i.e. when it is directly called by _bt_first), we never expect a prior call to _bt_readpage for lastcurrblkno to already indicate that the scan should end -- the _bt_first caller's blkno must always be read. After all, the "prior" _bt_readpage call (the call for lastcurrblkno) probably took place in some other backend (and it might not even have finished by the time our backend reaches _bt_first/_bt_readnextpage). Add a documenting assertion to the path where _bt_readnextpage ends the parallel scan based on information about lastcurrblkno from so->currPos. Assert that the most recent _bt_readpage call that set so->currPos is in fact lastcurrblkno's _bt_readpage call. Follow-up to bugfix commit b5ee4e52.
* Move check for USE_AVX512_POPCNT_WITH_RUNTIME_CHECK.Nathan Bossart2024-11-08
| | | | | | | | | | | Unlike TRY_POPCNT_FAST, which is defined in pg_bitutils.h, this macro is defined in c.h (via pg_config.h), so we can check for it earlier and avoid some unnecessary #includes on systems that lack AVX-512 support. Oversight in commit f78667bd91. Discussion: https://postgr.es/m/Zy5K5Qmlb3Z4dsd4%40nathan
* Improve fix for not entering parallel mode when holding interrupts.Tom Lane2024-11-08
| | | | | | | | | | | | | | | | | | | | | | | Commit ac04aa84a put the shutoff for this into the planner, which is not ideal because it doesn't prevent us from re-using a previously made parallel plan. Revert the planner change and instead put the shutoff into InitializeParallelDSM, modeling it on the existing code there for recovering from failure to allocate a DSM segment. However, that code path is mostly untested, and testing a bit harder showed there's at least one bug: ExecHashJoinReInitializeDSM is not prepared for us to have skipped doing parallel DSM setup. I also thought the Assert in ReinitializeParallelWorkers is pretty ill-advised, and replaced it with a silent Min() operation. The existing test case added by ac04aa84a serves fine to test this version of the fix, so no change needed there. Patch by me, but thanks to Noah Misch for the core idea that we could shut off worker creation when !INTERRUPTS_CAN_BE_PROCESSED. Back-patch to v12, as ac04aa84a was. Discussion: https://postgr.es/m/CAC-SaSzHUKT=vZJ8MPxYdC_URPfax+yoA1hKTcF4ROz_Q6z0_Q@mail.gmail.com
* Avoid nbtree parallel scan currPos confusion.Peter Geoghegan2024-11-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 1bd4bc85, which refactored nbtree sibling link traversal, made _bt_parallel_seize reset the scan's currPos so that things were consistent with the state of a serial backend moving between pages. This overlooked the fact that _bt_readnextpage relied on the existing currPos state to decide when to end the scan -- even though it came from before the scan was seized. As a result of all this, parallel nbtree scans could needlessly behave like full index scans. To fix, teach _bt_readnextpage to explicitly allow the use of an already read page's so->currPos when deciding whether to end the scan -- even during parallel index scans (allow it consistently now). This requires moving _bt_readnextpage's seizure of the scan to earlier in its loop. That way _bt_readnextpage either deals with the true so->currPos state, or an initialized-by-_bt_parallel_seize currPos state set from when the scan was seized. Now _bt_steppage (the most important _bt_readnextpage caller) takes the same uniform approach to setting up its call using details taken from so->currPos -- regardless of whether the scan happens to be parallel or serial. The new loop structure in _bt_readnextpage is prone to getting confused by P_NONE blknos set when the rightmost or leftmost page was reached. We could avoid that by adding an explicit check, but that would be ugly. Avoid this problem by teaching _bt_parallel_seize to end the parallel scan instead of returning a P_NONE next block/blkno. Doing things this way was arguably a missed opportunity for commit 1bd4bc85. It allows us to remove a similar "blkno == P_NONE" check from _bt_first. Oversight in commit 1bd4bc85, which refactored sibling link traversal (as part of optimizing nbtree backward scan locking). Author: Peter Geoghegan <pg@bowt.ie> Reported-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com> Diagnosed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com> Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com> Discussion: https://postgr.es/m/f8efb9c0f8d1a71b44fd7f8e42e49c25@oss.nttdata.com
* Add pg_constraint rows for not-null constraintsÁlvaro Herrera2024-11-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We now create contype='n' pg_constraint rows for not-null constraints on user tables. Only one such constraint is allowed for a column. We propagate these constraints to other tables during operations such as adding inheritance relationships, creating and attaching partitions and creating tables LIKE other tables. These related constraints mostly follow the well-known rules of conislocal and coninhcount that we have for CHECK constraints, with some adaptations: for example, as opposed to CHECK constraints, we don't match not-null ones by name when descending a hierarchy to alter or remove it, instead matching by the name of the column that they apply to. This means we don't require the constraint names to be identical across a hierarchy. The inheritance status of these constraints can be controlled: now we can be sure that if a parent table has one, then all children will have it as well. They can optionally be marked NO INHERIT, and then children are free not to have one. (There's currently no support for altering a NO INHERIT constraint into inheriting down the hierarchy, but that's a desirable future feature.) This also opens the door for having these constraints be marked NOT VALID, as well as allowing UNIQUE+NOT NULL to be used for functional dependency determination, as envisioned by commit e49ae8d3bc58. It's likely possible to allow DEFERRABLE constraints as followup work, as well. psql shows these constraints in \d+, though we may want to reconsider if this turns out to be too noisy. Earlier versions of this patch hid constraints that were on the same columns of the primary key, but I'm not sure that that's very useful. If clutter is a problem, we might be better off inventing a new \d++ command and not showing the constraints in \d+. For now, we omit these constraints on system catalog columns, because they're unlikely to achieve anything. The main difference to the previous attempt at this (b0e96f311985) is that we now require that such a constraint always exists when a primary key is in the column; we didn't require this previously which had a number of unpalatable consequences. With this requirement, the code is easier to reason about. For example: - We no longer have "throwaway constraints" during pg_dump. We needed those for the case where a table had a PK without a not-null underneath, to prevent a slow scan of the data during restore of the PK creation, which was particularly problematic for pg_upgrade. - We no longer have to cope with attnotnull being set spuriously in case a primary key is dropped indirectly (e.g., via DROP COLUMN). Some bits of code in this patch were authored by Jian He. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Author: Bernd Helmle <mailings@oopsware.de> Reviewed-by: 何建 (jian he) <jian.universality@gmail.com> Reviewed-by: 王刚 (Tender Wang) <tndrwang@gmail.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://postgr.es/m/202408310358.sdhumtyuy2ht@alvherre.pgsql
* Disallow partitionwise join when collations don't matchAmit Langote2024-11-08
| | | | | | | | | | | | | | | | If the collation of any join key column doesn’t match the collation of the corresponding partition key, partitionwise joins can yield incorrect results. For example, rows that would match under the join key collation might be located in different partitions due to the partitioning collation. In such cases, a partitionwise join would yield different results from a non-partitionwise join, so disallow it in such cases. Reported-by: Tender Wang <tndrwang@gmail.com> Author: Jian He <jian.universality@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/CAHewXNno_HKiQ6PqyLYfuqDtwp7KKHZiH1J7Pqyz0nr+PS2Dwg@mail.gmail.com Backpatch-through: 12
* Disallow partitionwise grouping when collations don't matchAmit Langote2024-11-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the collation of any grouping column doesn’t match the collation of the corresponding partition key, partitionwise grouping can yield incorrect results. For example, rows that would be grouped under the grouping collation may end up in different partitions under the partitioning collation. In such cases, full partitionwise grouping would produce results that differ from those without partitionwise grouping, so disallowed that. Partial partitionwise aggregation is still allowed, as the Finalize step reconciles partition-level aggregates with grouping requirements across all partitions, ensuring that the final output remains consistent. This commit also fixes group_by_has_partkey() by ensuring the RelabelType node is stripped from grouping expressions when matching them to partition key expressions to avoid false mismatches. Bug: #18568 Reported-by: Webbo Han <1105066510@qq.com> Author: Webbo Han <1105066510@qq.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/18568-2a9afb6b9f7e6ed3@postgresql.org Discussion: https://postgr.es/m/tencent_9D9103CDA420C07768349CC1DFF88465F90A@qq.com Discussion: https://postgr.es/m/CAHewXNno_HKiQ6PqyLYfuqDtwp7KKHZiH1J7Pqyz0nr+PS2Dwg@mail.gmail.com Backpatch-through: 12
* Fix inconsistent RestrictInfo serial numbersRichard Guo2024-11-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we generate multiple clones of the same qual condition to cope with outer join identity 3, we need to ensure that all the clones get the same serial number. To achieve this, we reset the root->last_rinfo_serial counter each time we produce RestrictInfo(s) from the qual list (see deconstruct_distribute_oj_quals). This approach works only if we ensure that we are not changing the qual list in any way that'd affect the number of RestrictInfos built from it. However, with b262ad440, an IS NULL qual on a NOT NULL column might result in an additional constant-FALSE RestrictInfo. And different versions of the same qual clause can lead to different conclusions about whether it can be reduced to constant-FALSE. This would affect the number of RestrictInfos built from the qual list for different versions, causing inconsistent RestrictInfo serial numbers across multiple clones of the same qual. This inconsistency can confuse users of these serial numbers, such as rebuild_joinclause_attr_needed, and lead to planner errors such as "ERROR: variable not found in subplan target lists". To fix, reset the root->last_rinfo_serial counter after generating the additional constant-FALSE RestrictInfo. Back-patch to v17 where the issue crept in. In v17, I failed to make a test case that would expose this bug, so no test case for v17. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs4-B6kafn+LmPuh-TYFwFyEm-vVj3Qqv7Yo-69CEv14rRg@mail.gmail.com
* Fix __attribute__((target(...))) usage.Nathan Bossart2024-11-07
| | | | | | | | | | The commonly supported way to specify multiple target options is to surround the entire list with quotes and to use a comma (with no extra spaces) as the delimiter. Oversight in commit f78667bd91. Discussion: https://postgr.es/m/Zy0jya8nF8CPpv3B%40nathan
* Use __attribute__((target(...))) for AVX-512 support.Nathan Bossart2024-11-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | Presently, we check for compiler support for the required intrinsics both with and without extra compiler flags (e.g., -mxsave), and then depending on the results of those checks, we pick which files to compile with which flags. This is tedious and complicated, and it results in unsustainable coding patterns such as separate files for each portion of code may need to be built with different compiler flags. This commit introduces support for __attribute__((target(...))) and uses it for the AVX-512 code. This simplifies both the configure-time checks and the build scripts, and it allows us to place the functions that use the intrinsics in files that we otherwise do not want to build with special CPU instructions. We are careful to avoid using __attribute__((target(...))) on compilers that do not understand it, but we still perform the configure-time checks in case the compiler allows using the intrinsics without it (e.g., MSVC). A similar change could likely be made for some of the CRC-32C code, but that is left as a future exercise. Suggested-by: Andres Freund Reviewed-by: Raghuveer Devulapalli, Andres Freund Discussion: https://postgr.es/m/20240731205254.vfpap7uxwmebqeaf%40awork3.anarazel.de
* doc: Reword ALTER TABLE ATTACH restriction on NO INHERIT constraintsÁlvaro Herrera2024-11-07
| | | | | | | | | | The previous wording is easy to read incorrectly; this change makes it simpler, less ambiguous, and less prominent. Backpatch to all live branches. Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/202411051201.zody6mld7vkw@alvherre.pgsql
* Clarify a foreign key error messagePeter Eisentraut2024-11-07
| | | | | | | | Clarify the message about type mismatch in foreign key definition to indicate which column the referencing and which is the referenced one. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/CACJufxEL82ao-aXOa=d_-Xip0bix-qdSyNc9fcWxOdkEZFko8w@mail.gmail.com
* Remove an obsolete comment in gistinsert()Michael Paquier2024-11-07
| | | | | | | | | This is inconsistent since 1f7ef548ec2e where the definition of gistFormTuple() has changed. Author: Tender Wang Reviewed-by: Aleksander Alekseev Discussion: https://postgr.es/m/CAHewXNkjU95_HdioDVU=5yBq_Xt=GfBv=Od-0oKtiA006pWW7Q@mail.gmail.com
* Replicate generated columns when 'publish_generated_columns' is set.Amit Kapila2024-11-07
| | | | | | | | | | | | | | | | | | | | This patch builds on the work done in commit 745217a051 by enabling the replication of generated columns alongside regular column changes through a new publication parameter: publish_generated_columns. Example usage: CREATE PUBLICATION pub1 FOR TABLE tab_gencol WITH (publish_generated_columns = true); The column list takes precedence. If the generated columns are specified in the column list, they will be replicated even if 'publish_generated_columns' is set to false. Conversely, if generated columns are not included in the column list (assuming the user specifies a column list), they will not be replicated even if 'publish_generated_columns' is true. Author: Vignesh C, Shubham Khanna Reviewed-by: Peter Smith, Amit Kapila, Hayato Kuroda, Shlok Kyal, Ajin Cherian, Hou Zhijie, Masahiko Sawada Discussion: https://postgr.es/m/B80D17B2-2C8E-4C7D-87F2-E5B4BE3C069E@gmail.com
* Improve handling of empty query results in BackgroundPsql::query()Michael Paquier2024-11-07
| | | | | | | | | | | | | | | | | | | | | | | A newline is not added at the end of an empty query result, causing the banner of the hardcoded \echo to not be discarded. This would reflect on scripts that expect an empty result by showing the "QUERY_SEPARATOR" in the output returned back to the caller, which was confusing. This commit changes BackgroundPsql::query() so as empty results are able to work correctly, making the first newline before the banner optional, bringing more flexibility. Note that this change affects 037_invalid_database.pl, where three queries generated an empty result, with the script relying on the data from the hardcoded banner to exist in the expected output. These queries are changed to use query_safe(), leading to a simpler script. The author has also proposed a test in a different patch where empty results would exist when using BackgroundPsql. Author: Jacob Champion Reviewed-by: Andrew Dunstan, Michael Paquier Discussion: https://postgr.es/m/CAOYmi+=60deN20WDyCoHCiecgivJxr=98s7s7-C8SkXwrCfHXg@mail.gmail.com
* Find invalid databases during upgrade check stageDaniel Gustafsson2024-11-06
| | | | | | | | | Before continuing with the check start by checking that all databases allow connections to avoid a hard fail without proper error reporting. Inspired by a larger patch by Thomas Krennwallner. Discussion: https://postgr.es/m/f9315bf0-e03e-4490-9f0d-5b6f7a6d9908@postsubmeta.net