aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Use macro NUM_MERGE_MATCH_KINDS instead of '3' in MERGE code.Dean Rasheed2024-04-19
| | | | | | | | Code quality improvement for 0294df2f1f84. Aleksander Alekseev, reviewed by Richard Guo. Discussion: https://postgr.es/m/CAJ7c6TMsiaV5urU_Pq6zJ2tXPDwk69-NKVh4AMN5XrRiM7N%2BGA%40mail.gmail.com
* Remove unused function prototypeDaniel Gustafsson2024-04-19
| | | | | | | | Commit aafc05de1bf5 removed StartSlotSyncWorker() but mistakenly left the prototype in slotsync.h. Fix by removing. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
* Fix incorrect parameter name in prototypeDaniel Gustafsson2024-04-19
| | | | | | | | | The function declaration for select_next_encryption_method use the variable name have_valid_connection, so fix the prototype in the header to match that. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
* Doc: Update link to the mentioned subsectionDaniel Gustafsson2024-04-18
| | | | | | | | | This updates the link from pg_createsubscriber to initial data sync to actually link to the subsection in question as opposed to the main logical replication section. Author: Pavel Luzanov <p.luzanov@postgrespro.ru> Discussion: https://postgr.es/m/a4af555a-ac60-4416-877d-0440d29b8763@postgrespro.ru
* Fix typos and duplicate wordsDaniel Gustafsson2024-04-18
| | | | | | | | | | | | This fixes various typos, duplicated words, and tiny bits of whitespace mainly in code comments but also in docs. Author: Daniel Gustafsson <daniel@yesql.se> Author: Heikki Linnakangas <hlinnaka@iki.fi> Author: Alexander Lakhin <exclusion@gmail.com> Author: David Rowley <dgrowleyml@gmail.com> Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Discussion: https://postgr.es/m/3F577953-A29E-4722-98AD-2DA9EFF2CBB8@yesql.se
* Remove spurious "the".Robert Haas2024-04-18
| | | | | | Spotted by Martin Marqués. Discussion: http://postgr.es/m/CABeG9LvQMtsKrOkhcA_mKJu1duArw4v+smeJKurYGjPVBZFecg@mail.gmail.com
* Don't try to fix eliminated nbtree array scan keys.Peter Geoghegan2024-04-18
| | | | | | | | | | | | | | | | | Preprocessing for nbtree index scans allowed array "input" scan keys already marked eliminated during array-specific preprocessing to be "fixed up" during preprocessing proper. This allowed eliminated scan keys on DESC index columns to spurious have their strategy commuted, causing assertion failures. To fix, teach _bt_fix_scankey_strategy to ignore these scan keys. This brings it in line with its only caller, _bt_preprocess_keys. Oversight in commit 5bf748b8, which enhanced nbtree ScalarArrayOp execution. Reported-By: Donghang Lin <donghanglin@gmail.com> Discussion: https://postgr.es/m/CAA=D8a2sHK6CAzZ=0CeafC-Y-MFXbYxnRSHvZTi=+JHu6kAa8Q@mail.gmail.com
* Restrict where INCREMENTAL.${NAME} files are recognized.Robert Haas2024-04-18
| | | | | | | | | | | | | | Previously, they were recognized anywhere in an incremental backup directory; now, we restrict this to places where they are expected to appear. That means this code will need updating if we ever do incremental backups of files in other places (e.g. SLRU files), but it lets you create a file called INCREMENTAL.config (or something like that) at the top level of the data directory and still have things work. Patch by me, per request from David Steele, who also reviewed. Discussion: http://postgr.es/m/5a7817da-6349-4653-8056-470300b6e512@pgmasters.net
* Don't try to assign smart names to constraintsAlvaro Herrera2024-04-18
| | | | | | This part of my previous commit seems to have broken pg_upgrade on crake, at least from 9.2. I'll see if there's a better fix, but in the meantime this should suffice to keep the buildfarm green.
* docs: Mention that pg_combinebackup does not verify backups.Robert Haas2024-04-18
| | | | | | | | | | | | We don't want users to think that pg_combinebackup is trying to check the validity of individual backups, because it isn't. Adjust the wording about sanity checks to make it clear that verification of individual backups is the job of pg_verifybackup, and that the checks performed by pg_combinebackup are around the relationships between the backups. Per discussion with David Steele. Discussion: http://postgr.es/m/e6f930c3-590c-47b9-b094-217bb2a3e22e@pgmasters.net
* Fix restore of not-null constraints with inheritanceAlvaro Herrera2024-04-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In tables with primary keys, pg_dump creates tables with primary keys by initially dumping them with throw-away not-null constraints (marked "no inherit" so that they don't create problems elsewhere), to later drop them once the primary key is restored. Because of a unrelated consideration, on tables with children we add not-null constraints to all columns of the primary key when it is created. If both a table and its child have primary keys, and pg_dump happens to emit the child table first (and its throw-away not-null) and later its parent table, the creation of the parent's PK will fail because the throw-away not-null constraint collides with the permanent not-null constraint that the PK wants to add, so the dump fails to restore. We can work around this problem by letting the primary key "take over" the child's not-null. This requires no changes to pg_dump, just two changes to ALTER TABLE: first, the ability to convert a no-inherit not-null constraint into a regular inheritable one (including recursing down to children, if there are any); second, the ability to "drop" a constraint that is defined both directly in the table and inherited from a parent (which simply means to mark it as no longer having a local definition). Secondarily, change ATPrepAddPrimaryKey() to acquire locks all the way down the inheritance hierarchy, in case we need to recurse when propagating constraints. These two changes allow pg_dump to reproduce more cases involving inheritance from versions 16 and older. Lastly, make two changes to pg_dump: 1) do not try to drop a not-null constraint that's marked as inherited; this allows a dump to restore with no errors if a table with a PK inherits from another which also has a PK; 2) avoid giving inherited constraints throwaway names, for the rare cases where such a constraint survives after the restore. Reported-by: Andrew Bille <andrewbille@gmail.com> Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/CAJnzarwkfRu76_yi3dqVF_WL-MpvT54zMwAxFwJceXdHB76bOA@mail.gmail.com Discussion: https://postgr.es/m/Zh0aAH7tbZb-9HbC@pryzbyj2023
* Update src/tools/pginclude/README to match recent changes to cpluspluscheckPeter Eisentraut2024-04-18
| | | | | | | | Commit 7b8e2ae2f has turned cpluspluscheck from separate script into a --cplusplus option for headerscheck. Update README correspondingly. Author: Anton Voloshin <a.voloshin@postgrespro.ru> Discussion: https://www.postgresql.org/message-id/02e69fa9-885d-4f41-9057-15a1d212eaf8@postgrespro.ru
* Fix object name clash in recently introduced testAmit Langote2024-04-18
| | | | | | | | | | | c0fc0751862 wasn't careful about naming the DOMAIN used in some new tests in sqljson_queryfunc.sql so as not to clash with the name of a DOMAIN used in the nearby sqljson_jsontable.sql. Fix by using a different name for the newly added DOMAIN in sqljson_queryfuncs.sql. Per buildfarm members canebrake and urutu. Discussion: https://postgr.es/m/CA+HiwqEjkbDxqqD3VJamc6R9+B102H7=SFYYOM7gKrxzJO35TQ@mail.gmail.com
* SQL/JSON: Miscellaneous fixes and improvementsAmit Langote2024-04-18
| | | | | | | | | | | | | | | | | | | | This addresses some post-commit review comments for commits 6185c973, de3600452, and 9425c596a0, with the following changes: * Fix JSON_TABLE() syntax documentation to use the term "path_expression" for JSON path expressions instead of "json_path_specification" to be consistent with the other SQL/JSON functions. * Fix a typo in the example code in JSON_TABLE() documentation. * Rewrite some newly added comments in jsonpath.h. * In JsonPathQuery(), add missing cast to int before printing an enum value. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com
* SQL/JSON: Fix issues with DEFAULT .. ON ERROR / EMPTYAmit Langote2024-04-18
| | | | | | | | | | | | | | | | | | | | | | | SQL/JSON query functions allow specifying an expression to return when either of ON ERROR or ON EMPTY condition occurs when evaluating the JSON path expression. The parser (transformJsonBehavior()) checks that the specified expression is one of the supported expressions, but there are two issues with how the check is done that are fixed in this commit: * No check for some expressions related to coercion, such as CoerceViaIO, that may appear in the transformed user-specified expressions that include cast(s) * An unsupported expression may be masked by a coercion-related expression, which must be flagged by checking the latter's argument expression recursively Author: Jian He <jian.universality@gmail.com> Author: Amit Langote <amitlangote09@gmail.com> Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxEqhqsfrg_p7EMyo5zak3d767iFDL8vz_4%3DZBHpOtrghw@mail.gmail.com Discussion: https://postgr.es/m/CACJufxGOerH1QJknm1noh-Kz5FqU4p7QfeZSeVT2tN_4SLXYNg@mail.gmail.com
* SQL/JSON: Improve some error messagesAmit Langote2024-04-18
| | | | | | | | | | | | | | | This improves some error messages emitted by SQL/JSON query functions by mentioning column name when available, such as when they are invoked as part of evaluating JSON_TABLE() columns. To do so, a new field column_name is added to both JsonFuncExpr and JsonExpr that is only populated when creating those nodes for transformed JSON_TABLE() columns. While at it, relevant error messages are reworded for clarity. Reported-by: Jian He <jian.universality@gmail.com> Suggested-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxG_e0QLCgaELrr2ZNz7AxPeGCNKAORe3fHtFCQLsH4J4Q@mail.gmail.com
* Refactoring for CommitTransactionCommand()/AbortCurrentTransaction()Alexander Korotkov2024-04-18
| | | | | | | | | | | | | | | fefd9a3fed turned tail recursion of CommitTransactionCommand() and AbortCurrentTransaction() into iteration. However, it splits the handling of cases between different functions. This commit puts the handling of all the cases into AbortCurrentTransactionInternal() and CommitTransactionCommandInternal(). Now CommitTransactionCommand() and AbortCurrentTransaction() are just doing the repeated calls of internal functions. Reported-by: Andres Freund Discussion: https://postgr.es/m/20240415224834.w6piwtefskoh32mv%40awork3.anarazel.de Author: Andres Freund
* Remove GlobalVisTestNonRemovable[Full]Horizon, not used anymoreAndres Freund2024-04-17
| | | | | | | | | GlobalVisTestNonRemovableHorizon() was only used for the implementation of snapshot_too_old, which was removed in f691f5b80a8. As using GlobalVisTestNonRemovableHorizon() is not particularly efficient, no new uses for it should be added. Therefore remove. Discussion: https://postgr.es/m/20240415185720.q4dg4dlcyvvrabz4@awork3.anarazel.de
* Cleanup parallel BRIN index build codeTomas Vondra2024-04-17
| | | | | | | | | | | | | | | | | | Commit b43757171470 added support for parallel builds of BRIN indexes, using code similar to BTREE. But there were to be a couple unnecessary differences, particularly in how the leader waits for the workers, and merges the results. So remove these, to make the code more similar. The leader never waited on the workersdonecv condition variable, but simply called WaitForParallelWorkersToFinish() in _brin_end_parallel() and then merged the per-worker results. This worked correctly, but it seems better to do the wait and merge before _brin_end_parallel(). This commit moves the relevant code to _brin_parallel_heapscan/merge(), which means _brin_end_parallel() remains responsible only for exiting the parallel mode and accumulating WAL usage data. Discussion: https://postgr.es/m/3733d042-71e1-6ae6-5fac-00c12db62db6@enterprisedb.com
* Stabilize test of BRIN parallel createTomas Vondra2024-04-17
| | | | | | | | | | | | | | | As explained in 4d916dd876, the test instability is caused by delayed cleanup of deleted rows. This commit removes the DELETE, stabilizing the test without accidentally disabling parallel builds. The intent of the delete however was to produce empty ranges, and test that the parallel index build populates those correctly. But there's another way to create empty ranges - partial indexes, which does not rely on cleanup of deleted rows. Idea to use partial indexes by Matthias van de Meent, patch by me. Discussion: https://postgr.es/m/95d9cd43-5a92-407c-b7e4-54cd303630fe%40enterprisedb.com
* Revert "Stabilize test of BRIN parallel create"Tomas Vondra2024-04-17
| | | | | | | | This reverts commit 4d916dd876c3. The goal of that commit was to stabilize a test of parallel BRIN build, but using a TEMPORARY table disables parallel index builds on that table, making the test useless. Discussion: https://postgr.es/m/95d9cd43-5a92-407c-b7e4-54cd303630fe%40enterprisedb.com
* meson: Add some missing LLVM function checksPeter Eisentraut2024-04-17
| | | | | | | | | | | | The checks for HAVE_DECL_LLVMCREATEGDBREGISTRATIONLISTENER and HAVE_DECL_LLVMCREATEPERFJITEVENTLISTENER are in configure but are missing on the meson side. This adds those. Reported-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/5539b16c-cff7-46d5-9621-c3fb6b549e9e@iki.fi
* Remove dead codePeter Eisentraut2024-04-17
| | | | | | | | | | The configure check for HAVE_DECL_LLVMORCREGISTERPERF was removed by e9a9843e138, but some code guarded by it was left. (That commit removed the "register" calls but left the "unregister" calls.) That code cannot be reached anymore, so remove it. Reported-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/5539b16c-cff7-46d5-9621-c3fb6b549e9e@iki.fi
* Add missing source file to libpq/nls.mkPeter Eisentraut2024-04-17
|
* doc: Fix COPY ON_ERROR option syntax synopsis.Masahiko Sawada2024-04-17
| | | | | | | | | | | ON_ERROR option values don't require quotations, which was inconsistent with the syntax synopsis in the documentation. Oversight in b725b7eec43. Author: Atsushi Torikoshi Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoC%3Dn4xR3%2BKQiqodnfT9chSB62XwZqmMff39H%3Dx9DS4scQ%40mail.gmail.com
* Fix typos with function name in event_trigger.cMichael Paquier2024-04-17
| | | | | | | | | Databases exist, contrary to datatabases. Oversight in e83d1b0c40cc. Author: Japin Li Discussion: https://postgr.es/m/ME3P282MB316611A2F7BF43919F695228B6082@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
* Disallow specifying ON_ERROR option without value.Masahiko Sawada2024-04-17
| | | | | | | | | | | | | | The ON_ERROR option of the COPY command previously allowed omitting its value, which was inconsistent with the syntax synopsis in the documentation and the behavior of other non-boolean COPY options. This change enforces providing a value for the ON_ERROR option, ensuring consistency across other non-boolean options and aligning with the documented syntax. Author: Atsushi Torikoshi Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/a9770bf57646d90dedc3d54cf32634b2%40oss.nttdata.com
* Update mmgr's README to mention BumpContextDavid Rowley2024-04-17
| | | | | | | | | | | | Oversight in 29f6a959c. In passing, since we now have 4 memory context types to choose from, provide a brief overview of the specialities of each memory context type. Reported-by: Amul Sul Author: Amul Sul, David Rowley Discussion: https://postgr.es/m/CAAJ_b94U2s9nHh--DEK=sPEZUQ+x7vQJ7529fF8UAH97QJ9NXg@mail.gmail.com
* Push dedicated BumpBlocks to the tail of the blocks listDavid Rowley2024-04-17
| | | | | | | | | | | | | | | | | | | | | | | BumpContext relies on using the head block from its 'blocks' field to use as the current block to allocate new chunks to. When we receive an allocation request larger than allocChunkLimit, we place these chunks on a new dedicated block and, until now, we pushed the block onto the *head* of the 'blocks' list. This behavior caused the previous bump block to no longer be available for new normal-sized (non-large) allocations and would result in blocks only being partially filled if a large allocation request arrived before the block became full. Here adjust the code to push these dedicated blocks onto the *tail* of the blocks list so that the head block remains intact and available to be used by normal allocation request sizes until it becomes full. In passing, make the elog(ERROR) calls for the unsupported callbacks consistent. Likewise for the header comments for those functions. Discussion: https://postgr.es/m/CAApHDvp9___r-ayJj0nZ6GD3MeCGwGZ0_6ZptWpwj+zqHtmwCw@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvqerXpzUnuDQfUEi3DZA+9=Ud9WSt3ruxN5b6PcOosx2g@mail.gmail.com
* Mark some new location fields as ParseLocPeter Eisentraut2024-04-16
| | | | | Some new code probably didn't see 605721f819f and continued to use type int for parse location fields. Fix those.
* Clean up more indent breakage from 6377e12a5.Tom Lane2024-04-16
| | | | Per buildfarm member koel.
* Fix assorted bugs in ecpg's macro mechanism.Tom Lane2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The code associated with EXEC SQL DEFINE was unreadable and full of bugs, notably: * It'd attempt to free a non-malloced string if the ecpg program tries to redefine a macro that was defined on the command line. * Possible memory stomp if user writes "-D=foo". * Undef'ing or redefining a macro defined on the command line would change the state visible to the next file, when multiple files are specified on the command line. (While possibly that could have been an intentional choice, the code clearly intends to revert to the original macro state; it's just failing to consider this interaction.) * Missing "break" in defining a new macro meant that redefinition of an existing name would cause an extra entry to be added to the definition list. While not immediately harmful, a subsequent undef would result in the prior entry becoming visible again. * The interactions with input buffering are subtle and were entirely undocumented. It's not that surprising that we hadn't noticed these bugs, because there was no test coverage at all of either the -D command line switch or multiple input files. This patch adds such coverage (in a rather hacky way I guess). In addition to the code bugs, the user documentation was confused about whether the -D switch defines a C macro or an ecpg one, and it failed to mention that you can write "-Dsymbol=value". These problems are old, so back-patch to all supported branches. Discussion: https://postgr.es/m/998011.1713217712@sss.pgh.pa.us
* Fix nbtree "deduce NOT NULL" scan key comment.Peter Geoghegan2024-04-16
| | | | Oversight in commit c9c0589fda.
* Undo incorrect typedefs.list change.Tom Lane2024-04-16
| | | | | 6377e12a5 should not have removed AcquireSampleRowsFunc, as that's still in use. Per buildfarm member koel.
* Stabilize test of BRIN parallel createTomas Vondra2024-04-16
| | | | | | | | | | | | | | | | | | The test for parallel create of BRIN indexes added by commit 8225c2fd40 happens to be unstable - a background transaction (e.g. auto-analyze) may hold back global xmin for the initial VACUUM / CREATE INDEX. If the cleanup happens before the next CREATE INDEX, the indexes will not be exactly the same. This is the same issue as e2933a6e11, so fix it the same way by making the table TEMPORARY, which uses an up-to-date cutoff xmin that is not held back by other processes. Reported by Alexander Lakhin, who also suggested the fix. Author: Alexander Lakhin Discussion: https://postgr.es/m/b58901cd-a7cc-29c6-e2b1-e3d7317c3c69@gmail.com Discussion: https://postgr.es/m/2892135.1668976646@sss.pgh.pa.us
* Ensure generated join clauses for child rels have correct relids.Tom Lane2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When building a join clause derived from an EquivalenceClass, if the clause is to be used with an appendrel child relation then make sure its clause_relids include the relids of that child relation. Normally this would be true already because the EquivalenceMember would be a Var of that relation. However, if the appendrel represents a flattened UNION ALL construct then some child EquivalenceMembers could be constants with no relids. The resulting under-marked clause is problematic because it could mislead join_clause_is_movable_into about where the clause should be evaluated. We do not have an example showing incorrect plan generation, but there are existing cases in the regression tests that will fail the Asserts this patch adds to get_baserel_parampathinfo. A similarly wrong conclusion about a clause being considered by get_joinrel_parampathinfo would lead to wrong placement of the clause. (This also squares with the way that clause_relids is calculated for non-equijoin clauses in adjust_appendrel_attrs.) The other reason for wanting these new Asserts is that the previous blithe assumption that the results of generate_join_implied_equalities "necessarily satisfy join_clause_is_movable_into" turns out to be wrong pre-v16. If it's still wrong it'd be good to find out. Per bug #18429 from Benoît Ryder. The bug as filed was fixed by commit 2489d76c4, but these changes correlate with the fix we will need to apply in pre-v16 branches. Discussion: https://postgr.es/m/18429-8982d4a348cc86c6@postgresql.org
* Fix nbtree posting list comment.Peter Geoghegan2024-04-16
| | | | Oversight in commit 0d861bbb70.
* Fix nbtree page recycling comment.Peter Geoghegan2024-04-16
| | | | Oversight in commit e5d8a99903.
* revert: Generalize relation analyze in table AM interfaceAlexander Korotkov2024-04-16
| | | | | | This commit reverts 27bc1772fc and dd1f6b0c17. Per review by Andres Freund. Discussion: https://postgr.es/m/20240415201057.khoyxbwwxfgzomeo%40awork3.anarazel.de
* Improve test coverage in bump.cDavid Rowley2024-04-16
| | | | | | | | | | | | There were no callers of BumpAllocLarge() in the regression tests, so here we add a sort with a tuple large enough to use that path in bump.c. Also, BumpStats() wasn't being called, so add a test to sysviews.sql to call pg_backend_memory_contexts() while a bump context exists in the backend. Reported-by: Andres Freund Discussion: https://postgr.es/m/20240414223305.m3i5eju6zylabvln@awork3.anarazel.de
* Add missing PGDLLIMPORT markingsMichael Paquier2024-04-16
| | | | | | | | | | | | | All backend-side variables should be marked with PGDLLIMPORT, as per policy introduced in 8ec569479f. aafc05de1bf5 has forgotten MyClientSocket, and 05c3980e7f47 LoadedSSL. These can be spotted with a command like this one (be careful of not switching __pg_log_level): src/tools/mark_pgdllimport.pl $(git ls-files src/include/) Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/ZhzkRzrkKhbeQMRm@paquier.xyz
* docs: Consolidate into new "WAL for Extensions" chapter.Robert Haas2024-04-15
| | | | | | | | | | Previously, we had consecutive, very short chapters called "Generic WAL" and "Custom WAL Resource Managers," explaining different approaches to the same problem. Merge them into a single chapter. Explain most of the differences between the approaches in the chapter's introductory text, rather than in the individual sections. Discussion: http://postgr.es/m/46ac50c1-6b2a-404f-a683-b67af6ab56e9@eisentraut.org
* doc: Note exceptions for SET ROLE's effect on privilege checks.Nathan Bossart2024-04-15
| | | | | | | | | | | | | | | | The documentation for SET ROLE states that superusers who switch to a non-superuser role lose their superuser privileges. While this is true for most commands, there are exceptions such as SET ROLE and SET SESSION AUTHORIZATION, which continue to use the current session user and the authenticated user, respectively. Furthermore, the description of this command already describes its effect, so it is arguably unnecessary to include this special case. This commit removes the note about the superuser case and adds a sentence about the aforementioned exceptions to the description. Co-authored-by: Yurii Rashkovskii Reviewed-by: Shubham Khanna, Robert Haas, Michael Paquier Discussion: https://postgr.es/m/CA%2BRLCQysHtME0znk2KUMJN343ksboSRQSU-hCnOjesX6VK300Q%40mail.gmail.com
* Fix type-checking of RECORD-returning functions in FROM, redux.Tom Lane2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 2ed8f9a01 intended to institute a policy that if a RangeTblFunction has a coldeflist, then the function return type is certainly RECORD, and we should use the coldeflist as the source of truth about what the columns of the record type are. When the original function has been folded to a constant, inspection of the constant might give a different answer. This situation will lead to a tuple-type-mismatch error at execution, but up until that point we need to consistently believe the coldeflist, or we'll have problems from different bits of code reaching different conclusions. expandRTE didn't get that memo though, and would try to produce a tupdesc based on the constant in this situation, leading to an assertion failure. (Desultory testing suggests that non-assert builds often manage to give the expected error, although I also saw a "cache lookup failed for type 0" error, and it seems at least possible that a crash could happen.) Some other callers of get_expr_result_type and get_expr_result_tupdesc were also being incautious about this. While none of them seem to have actual bugs, they're working harder than necessary in this case, besides which it seems safest to have an explicit policy of not using those functions on an RTE with a coldeflist. Adjust the code accordingly, and add commentary to funcapi.c about this policy. Also fix an obsolete comment that claimed "get_expr_result_type() doesn't know how to extract type info from a RECORD constant". That hasn't been true since commit d57534740. Per bug #18422 from Alexander Lakhin. As with the previous commit, back-patch to all supported branches. Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
* ATTACH PARTITION: Don't match a PK with a UNIQUE constraintAlvaro Herrera2024-04-15
| | | | | | | | | | When matching constraints in AttachPartitionEnsureIndexes() we weren't testing the constraint type, which could make a UNIQUE key lacking a not-null constraint incorrectly satisfy a primary key requirement. Fix this by testing that the constraint types match. (Other possible mismatches are verified by comparing index properties.) Discussion: https://postgr.es/m/202402051447.wimb4xmtiiyb@alvherre.pgsql
* Grammar fixes for split/merge partitions codeAlexander Korotkov2024-04-15
| | | | | | | | | | The fixes relate to comments, error messages, and corresponding expected output of regression tests. Discussion: https://postgr.es/m/CAMbWs49DDsknxyoycBqiE72VxzL_sYHF6zqL8dSeNehKPJhkKg%40mail.gmail.com Discussion: https://postgr.es/m/86bfd241-a58c-479a-9a72-2c67a02becf8%40postgrespro.ru Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com Author: Richard Guo, Dmitry Koval, Tender Wang
* Fix propagating attnotnull in multiple inheritanceAlvaro Herrera2024-04-15
| | | | | | | | | | | | | | | | | | | In one of the many strange corner cases of multiple inheritance being used, commit b0e96f311985 missed a CommandCounterIncrement() call after updating the attnotnull flag during ALTER TABLE ADD COLUMN, which caused a catalog tuple to be update attempted twice in the same command, giving rise to a "tuple already updated by self" error. Add the missing call to solve that, and a test case that reproduces the scenario. As a (perhaps surprising) secondary effect, this CCI addition triggers another behavior change: when a primary key is added to a parent partitioned table and the column in an existing partition does not have a not-null constraint, we no longer error out. This will probably be a welcome change by some users, and I think it's unlikely that anybody will miss the old behavior. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: http://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f6992306c7@gmail.com
* psql: Make output of \dD more stablePeter Eisentraut2024-04-15
| | | | | | | \dD showed domain check constraints in arbitrary order, which can cause regression test failures, which was exposed by commit 9895b35cb8. To fix, order the constraints by conname, which matches what psql does in other queries listing constraints.
* Fix ALTER DOMAIN NOT NULL syntaxPeter Eisentraut2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This addresses a few problems with commit e5da0fe3c22 ("Catalog domain not-null constraints"). In CREATE DOMAIN, a NOT NULL constraint looks like CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL (Before e5da0fe3c22, the constraint name was accepted but ignored.) But in ALTER DOMAIN, a NOT NULL constraint looks like ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE where VALUE is where for a table constraint the column name would be. (This works as of e5da0fe3c22. Before e5da0fe3c22, this syntax resulted in an internal error.) But for domains, this latter syntax is confusing and needlessly inconsistent between CREATE and ALTER. So this changes it to just ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL (None of these syntaxes are per SQL standard; we are just living with the bits of inconsistency that have built up over time.) In passing, this also changes the psql \dD output to not show not-null constraints in the column "Check", since it's already shown in the column "Nullable". This has also been off since e5da0fe3c22. Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
* Put back initialization of 'sslmode', to silence CoverityHeikki Linnakangas2024-04-14
| | | | | | | | | | | | | | | | Coverity pointed out that the function checks for conn->sslmode != NULL, which implies that it might be NULL, but later we access it without a NULL-check anyway. It doesn't know that it is in fact always initialized earlier, in conninfo_add_defaults(), and hence the NULL-check is not necessary. However, there is a lot of distance between conninfo_add_defaults() and pqConnectOptions2(), so it's not surprising that it doesn't see that. Put back the initialization code, as it existed before commit 05fd30c0e7, to silence the warning. In the long run, I'd like to refactor the libpq options handling and initalization code. It seems silly to strdup() and copy strings, for things like sslmode that have a limited set of possible values; it should be an enum. But that's for another day.