aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Ensure that "pg_restore -l" reports dependent TOC entries correctly.Tom Lane2024-05-07
| | | | | | | | | | | | | | If -l was specified together with selective-restore options such as -n or -N, dependent TOC entries such as comments would be omitted from the listing, even when an actual restore would have selected them. This happened because PrintTOCSummary neglected to update the te->reqs marking of the entry they depended on. Per report from Justin Pryzby. This has been wrong since 0d4e6ed30 taught _tocEntryRequired to sometimes look at the "reqs" marking of other TOC entries, so back-patch to all supported branches. Discussion: https://postgr.es/m/ZjoeirG7yxODdC4P@pryzbyj2023
* Don't corrupt plpython's "TD" dictionary in a recursive trigger call.Tom Lane2024-05-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | If a plpython-language trigger caused another one to be invoked, the "TD" dictionary created for the inner one would overwrite the outer one's "TD" dictionary. This is more or less the same problem that 1d2fe56e4 fixed for ordinary functions in plpython, so fix it the same way, by saving and restoring "TD" during a recursive invocation. This fix makes an ABI-incompatible change in struct PLySavedArgs. I'm not too worried about that because it seems highly unlikely that any extension is messing with those structs. We could imagine doing something weird to preserve nominal ABI compatibility in the back branches, like keeping the saved TD object in an extra element of namedargs[]. However, that would only be very nominal compatibility: if anything *is* touching PLySavedArgs, it would likely do the wrong thing due to not knowing about the additional value. So I judge it not worth the ugliness to do something different there. (I also changed struct PLyProcedure, but its added field fits into formerly-padding space, so that should be safe.) Per bug #18456 from Jacques Combrink. This bug is very ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/3008982.1714853799@sss.pgh.pa.us
* Stamp 12.19.REL_12_19Tom Lane2024-05-06
|
* Translation updatesPeter Eisentraut2024-05-06
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 9a37846122eee9aa9c8f8d1cea1bbe7afb28796b
* Release notes for 16.3, 15.7, 14.12, 13.15, 12.19.Tom Lane2024-05-05
|
* doc: Fix description of deterministic flag of CREATE COLLATIONPeter Eisentraut2024-05-02
| | | | | | | | | | | The documentation said that you need to pick a suitable LC_COLLATE setting in addition to setting the DETERMINISTIC flag. This would have been correct if the libc provider supported nondeterministic collations, but since it doesn't, you actually need to set the LOCALE option. Reviewed-by: Kashif Zeeshan <kashi.zeeshan@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/a71023c2-0ae0-45ad-9688-cf3b93d0d65b%40eisentraut.org
* Ensure we allocate NAMEDATALEN bytes for names in Index Only ScansDavid Rowley2024-05-01
| | | | | | | | | | | | | | | As an optimization, we store "name" columns as cstrings in btree indexes. Here we modify it so that Index Only Scans convert these cstrings back to names with NAMEDATALEN bytes rather than storing the cstring in the tuple slot, as was happening previously. Bug: #17855 Reported-by: Alexander Lakhin Reviewed-by: Alexander Lakhin, Tom Lane Discussion: https://postgr.es/m/17855-5f523e0f9769a566@postgresql.org Backpatch-through: 12, all supported versions
* Disallow converting a table to a view within an outer SQL command.Tom Lane2024-04-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have long disallowed all forms of ALTER TABLE if the table is already opened by some outer SQL command in the same session. This has the same purpose as obtaining AccessExclusiveLock, but since a session's own locks don't conflict the lock only blocks use of the table by other sessions, not our own. Without this check, the ALTER might confuse the outer SQL command since any previous inspection of the table would potentially become invalid. However, the RelisBecomingView code path in DefineQueryRewrite never got that memo, and assumed that AccessExclusiveLock is sufficient for performing something morally equivalent to a rather invasive ALTER TABLE. Unsurprisingly, this can confuse an outer command that is trying to do something with the table. This was submitted as a security issue, but the security team has been unable to identify any consequence worse than a null pointer dereference (from trying to access rd_tableam methods that the relation no longer has). Therefore, in accordance with our usual policy, it's not security material and should just be fixed as a routine bug. Fix by disallowing the operation if the table is open locally, exactly as ALTER TABLE does it. Per an anonymous security researcher, via Bundesamt für Sicherheit in der Informationstechnik. Patch v12-v15 only. In v16 and later, we removed this code altogether (cf. commit b23cd185f), so that there's no issue.
* Close race condition between datfrozen and relfrozen updates.Noah Misch2024-04-29
| | | | | | | | | | | | | vac_update_datfrozenxid() did multiple loads of relfrozenxid and relminmxid from buffer memory, and it assumed each would get the same value. Not so if a concurrent vac_update_relstats() did an inplace update. Commit 2d2e40e3befd8b9e0d2757554537345b15fa6ea2 fixed the same kind of bug in vac_truncate_clog(). Today's bug could cause the rel-level field and XIDs in the rel's rows to precede the db-level field. A cluster having such values should VACUUM affected tables. Back-patch to v12 (all supported versions). Discussion: https://postgr.es/m/20240423003956.e7.nmisch@google.com
* Detect more overflows in timestamp[tz]_pl_interval.Tom Lane2024-04-28
| | | | | | | | | | | | | | | | | In commit 25cd2d640 I (tgl) opined that "The additions of the months and microseconds fields could also overflow, of course. However, I believe we need no additional checks there; the existing range checks should catch such cases". This is demonstrably wrong however for the microseconds field, and given that discovery it seems prudent to be paranoid about the months addition as well. Report and patch by Joseph Koshakow. As before, back-patch to all supported branches. (However, the test case doesn't work before v15 because we didn't allow wider-than-int32 numbers in interval literals. A variant test could probably be built that fits within that restriction, but it didn't seem worth the trouble.) Discussion: https://postgr.es/m/CAAvxfHf77sRHKoEzUw9_cMYSpbpNS2C+J_+8Dq4+0oi8iKopeA@mail.gmail.com
* Doc: fix minor oversight in ALTER DEFAULT PRIVILEGES ref page.Tom Lane2024-04-24
| | | | | | | | | | Since schemas have more than one kind of privilege, we should use the synopsis form that shows the privilege being possibly repeated. Yugo Nagata Discussion: https://postgr.es/m/20240424155052.7ac0d0773e4ae27ab723faea@sraoss.co.jp
* doc: Correct jsonpath string literal escapes descriptionPeter Eisentraut2024-04-24
| | | | | | | | | | | | The paragraph describing the JavaScript string literals allowed in jsonpath expressions unnecessarily mentions JSON by erroneously listing \v as allowed by JSON and mentioning the \xNN and \u{N...} backslash escapes as deviations from JSON when in fact both are accepted by ECMAScript/JavaScript. Fix this by only referring to JavaScript. Author: Erik Wienhold <ewie@ewie.name> Discussion: https://www.postgresql.org/message-id/flat/1EB17DF9-2636-484B-9DD0-3CAB19C4F5C4@justatheory.com
* Make postgres_fdw request remote time zone 'GMT' not 'UTC'.Tom Lane2024-04-21
| | | | | | | | | | | | | | | | | | This should have the same results for all practical purposes. The advantage of selecting 'GMT' is that it's guaranteed to work even when the remote system's timezone database is missing entries, because pg_tzset() hard-wires handling of that, at least in 9.2 and later. (It seems like it would be a good idea to similarly hard-wire correct handling of 'UTC', but that'll be a little more invasive than I want to consider back-patching. Leave that for another day when we're not in feature freeze.) Per trouble report from Adnan Dautovic. Back-patch to all supported branches. Discussion: https://postgr.es/m/465248.1712211585@sss.pgh.pa.us
* Doc: document cases where queryid is stableDavid Rowley2024-04-20
| | | | | | | | | | | | | | The documents were clear that queryid should not be assumed to be stable between major versions but said nothing about minor versions and left the reader to guess if that was implied by the mention of the instability of queryid between major versions. Here we give minor versions an explicit mention to indicate queryid can generally be assumed stable between minor versions. Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAApHDvpYGE6h0cD9UO-eHySPynPj1L3J%3DHxT%2BA7Ud8_Yo6AuzA%40mail.gmail.com Backpatch-through: 12
* Fix MSVC recipe for ecpg regression tests, redux.Tom Lane2024-04-19
| | | | | | | | Forgot to inject -DCMDLINESYM=123 ... Per buildfarm. Discussion: https://postgr.es/m/4cc4dc47-ca2b-4129-8784-db69b5f82777@dunslane.net
* Fix MSVC recipe for ecpg regression tests.Tom Lane2024-04-18
| | | | | | | | | | | While back-patching commit 6f0cef935, I forgot that the MSVC build scripts would also need adjustment in the back branches. This is a blind attempt at a fix, but it's basically copying nearby code so I think it will work. Per buildfarm (via Andrew Dunstan) Discussion: https://postgr.es/m/4cc4dc47-ca2b-4129-8784-db69b5f82777@dunslane.net
* 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 generation of EC join conditions at the wrong plan level.Tom Lane2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | get_baserel_parampathinfo previously assumed without checking that the results of generate_join_implied_equalities "necessarily satisfy join_clause_is_movable_into". This turns out to be wrong in the presence of outer joins, because the generated clauses could include Vars that mustn't be evaluated below a relevant outer join. That led to applying clauses at the wrong plan level and possibly getting incorrect query results. We must check each clause's nullable_relids, and really the right thing to do is test join_clause_is_movable_into. However, trying to fix it that way exposes an oversight in equivclass.c: it wasn't careful about marking join clauses for appendrel children with the correct clause_relids. That caused the modified get_baserel_parampathinfo code to reject some clauses it still needs to accept. (See parallel commit for HEAD/v16 for more commentary about that.) Per bug #18429 from Benoît Ryder. This misbehavior existed for a long time before commit 2489d76c4, so patch v12-v15 this way. Discussion: https://postgr.es/m/18429-8982d4a348cc86c6@postgresql.org
* xml2: Replace deprecated routines with recommended onesMichael Paquier2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | | | Some functions are used in the tree and are currently marked as deprecated by upstream. This commit refreshes the code to use the recommended functions, leading to the following changes: - xmlSubstituteEntitiesDefault() is gone, and needs to be replaced with XML_PARSE_NOENT for the paths doing the parsing. - xmlParseMemory() -> xmlReadMemory(). These functions, as well as more functions setting global states, have been officially marked as deprecated by upstream in August 2022. Their replacements exist since the 2001-ish area, as far as I have checked, so that should be safe. This has been originally applied as 65c5864d7fac without a backpatch, and this has come up as well when working on 400928b83. Per request from Tom Lane, for new buildfarm member indri that is able to see deprecation warnings with xmlSubstituteEntitiesDefault() in 16 and older stable branches. Author: Dmitry Koval Discussion: https://postgr.es/m/18274-98d16bc03520665f@postgresql.org Discussion: https://postgr.es/m/1012981.1713222862@sss.pgh.pa.us Bakpatch-through: 12
* 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
* Doc: fix bogus to_date() examples.Tom Lane2024-04-11
| | | | | | | | | November doesn't have 31 days. Remarkably, this thinko has escaped detection since commit 3f1998727. Noted by Y. Saburov. Discussion: https://postgr.es/m/171276122213.681.531905738590773705@wrigleys.postgresql.org
* Fix WaitEventSet resource leak in WaitLatchOrSocket().Etsuro Fujita2024-04-11
| | | | | | | | | | | | | | | This function would have the same issue we solved in commit 501cfd07d: If an error is thrown after calling CreateWaitEventSet(), the file descriptor (on epoll- or kqueue-based systems) or handles (on Windows) that the WaitEventSet contains are leaked. Like that commit, use PG_TRY-PG_FINALLY (PG_TRY-PG_CATCH in v12) to make sure the WaitEventSet is freed properly. Back-patch to all supported versions, but as we do not have this issue in HEAD (cf. commit 50c67c201), no need to apply this patch to it. Discussion: https://postgr.es/m/CAPmGK16MqdDoD8oatp8SQWaEa4vS3nfQqDN_Sj9YRuu5J3Lj9g%40mail.gmail.com
* Fix plpgsql's handling of -- comments following expressions.Tom Lane2024-04-10
| | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, read_sql_construct() has collected all the source text from the statement or expression's initial token up to the character just before the "until" token. It normally tries to strip trailing whitespace from that, largely for neatness. If there was a "-- text" comment after the expression, this resulted in removing the newline that terminates the comment, which creates a hazard if we try to paste the collected text into a larger SQL construct without inserting a newline after it. In particular this caused our handling of CASE constructs to fail if there's a comment after a WHEN expression. Commit 4adead1d2 noticed a similar problem with cursor arguments, and worked around it through the rather crude hack of suppressing the whitespace-trimming behavior for those. Rather than do that and leave the hazard open for future hackers to trip over, let's fix it properly. pl_scanner.c already has enough infrastructure to report the end location of the expression's last token, so we can copy up to that location and never collect any trailing whitespace or comment to begin with. Erik Wienhold and Tom Lane, per report from Michal Bartak. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAAVzF_FjRoi8fOVuLCZhQJx6HATQ7MKm=aFOHWZODFnLmjX-xA@mail.gmail.com
* Doc: Update ulinks to RFC documents to avoid redirectDaniel Gustafsson2024-04-10
| | | | | | | | | | | | | The tools.ietf.org site has been decommissioned and replaced by a number of sites serving various purposes. Links to RFCs and BCPs are now 301 redirected to their new respective IETF sites. Since this serves no purpose and only adds network overhead, update our links to the new locations. Backpatch to all supported versions. Discussion: https://postgr.es/m/3C1CEA99-FCED-447D-9858-5A579B4C6687@yesql.se Backpatch-through: v12
* Fix illegal attribute propagation in LLVM JIT.Thomas Munro2024-04-10
| | | | | | | | | | | | | | | | | | | Commit 72559438 started copying more attributes from AttributeTemplate to the functions we generate on the fly. In the case of deform functions, which return void, this meant that "noundef", from AttributeTemplate's return value (a Datum) was copied to a void type. Older LLVM releases were OK with that, but LLVM 18 crashes. Update our llvm_copy_attributes() function to skip copying the attribute for the return value, if the target function returns void. Thanks to Dmitry Dolgov for help chasing this down. Back-patch to all supported releases, like 72559438. Reported-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com
* doc: Remove stray comma from list of psql optionsDaniel Gustafsson2024-04-09
| | | | | | | | | | | Back in 7.2 the list of options had short options and long options on the same line separated by comma, but since 7.3 they are listed separate lines. The comma on -X was left behind so fix by removing and backpatching all the way. Reported-by: y.saburov@gmail.com Discussion: https://postgr.es/m/171267154345.684.7212826057932148541@wrigleys.postgresql.org Backpatch-through: v12
* simplehash: Free collisions array in SH_STATAndres Freund2024-04-07
| | | | | | | | | | | | While SH_STAT() is only used for debugging, the allocated array can be large, and therefore should be freed. It's unclear why coverity started warning now. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Coverity Discussion: https://postgr.es/m/3005248.1712538233@sss.pgh.pa.us Backpatch: 12-
* Don't clobber test exit code at cleanup in LDAP/Kerberors testsHeikki Linnakangas2024-04-07
| | | | | | | | | | If the test script die()d before running the first test, the whole test was interpreted as SKIPped rather than failed. The PostgreSQL::Cluster module got this right. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/fb898a70-3a88-4629-88e9-f2375020061d@iki.fi
* Improve check in LDAP test to find the OpenLDAP installationHeikki Linnakangas2024-04-07
| | | | | | | | | | | | | | | | | | | | | | | If the OpenLDAP installation directory is not found, set $setup to 0 so that the LDAP tests are skipped. The macOS checks were already doing that, but the checks on other OS's were not. While we're at it, improve the error message when the tests are skipped, to specify whether the OS is supported at all, or if we just didn't find the installation directory. This was accidentally "working" without this, i.e. we were skipping the tests if the OpenLDAP installation was not found, because of a bug in the LdapServer test module: the END block clobbered the exit code so if the script die()s before running the first subtest, the whole test script was marked as SKIPped. The next commit will fix that bug, but we need to fix the setup code first. These checks should probably go into configure/meson, but this is better than nothing and allows fixing the bug in the END block. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/fb898a70-3a88-4629-88e9-f2375020061d@iki.fi
* Fix ecpg's mechanism for detecting unsupported cases in the grammar.Tom Lane2024-04-04
| | | | | | | | | | | | | | | | | | | | | | | | | ecpg wants to emit a warning if it parses a SQL construct that the backend can parse but will immediately throw a FEATURE_NOT_SUPPORTED error for. The way it was testing for this was to see if the string ERRCODE_FEATURE_NOT_SUPPORTED appeared anywhere in the gram.y code. This is, of course, not nearly good enough, as there are plenty of rules in gram.y that throw that error only conditionally. There was a hack dating to 2008 to suppress the warning in one rule that doesn't even exist anymore, but nothing for other cases we've created since then. End result was that you could get "unsupported feature will be passed to server" warnings while compiling perfectly good SQL code in ecpg. Somehow we'd not heard complaints about this, but it was exposed by the recent addition of an ecpg test for a SQL/JSON construct. To fix, suppress the warning if the rule contains any "if" statement. Manual comparison of gram.y with the generated preproc.y file shows that the warning is now emitted only in rules where it's sensible. This problem has existed for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/603615.1712245382@sss.pgh.pa.us
* Fix the parameters order for TableAmRoutine.relation_copy_for_cluster()Alexander Korotkov2024-04-03
| | | | | | | | | | | | | Specify OldTable first, NewTable second as used by table_relation_copy_for_cluster() and as implemented in heapam_relation_copy_for_cluster(). Backpatch to PostgreSQL 12, where TableAmRoutine was introduced. Discussion: https://postgr.es/m/ME3P282MB3166860D4911AE82F92DF7C5B63F2%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM Author: Japin Li Reviewed-by: Pavel Borisov Backpatch-through: 12
* Avoid deadlock during orphan temp table removal.Tom Lane2024-04-02
| | | | | | | | | | | | | | | | | | | | | | | If temp tables have dependencies (such as sequences) then it's possible for autovacuum's cleanup of orphan temp tables to deadlock against an incoming backend that's trying to clean out the temp namespace for its own use. That can happen because RemoveTempRelations' performDeletion call can visit objects within the namespace in an order different from the order in which a per-table deletion will visit them. To fix, observe that performDeletion will begin by taking an exclusive lock on the temp namespace (even though it won't actually delete it). So, if we can get a shared lock on the namespace, we can be sure we're not running concurrently with RemoveTempRelations, while also not conflicting with ordinary use of the namespace. This requires introducing a conditional version of LockDatabaseObject, but that's no big deal. (It's surprising we've got along without that this long.) Report and patch by Mikhail Zhilin. Back-patch to all supported branches. Discussion: https://postgr.es/m/c43ce028-2bc2-4865-9b89-3f706246eed5@postgrespro.ru
* Avoid "unused variable" warning on non-USE_SSL_ENGINE platforms.Tom Lane2024-04-01
| | | | | | | | | | | If we are building with openssl but USE_SSL_ENGINE didn't get set, initialize_SSL's variable "pkey" is declared but used nowhere. Apparently this combination hasn't been exercised in the buildfarm before now, because I've not seen this warning before, even though the code has been like this a long time. Move the declaration to silence the warning (and remove its useless initialization). Per buildfarm member sawshark. Back-patch to all supported branches.
* Avoid possible longjmp-induced logic error in PLy_trigger_build_args.Tom Lane2024-04-01
| | | | | | | | | | | | | | | The "pltargs" variable wasn't marked volatile, which makes it unsafe to change its value within the PG_TRY block. It looks like the worst outcome would be to fail to release a refcount on Py_None during an (improbable) error exit, which would likely go unnoticed in the field. Still, it's a bug. A one-liner fix could be to mark pltargs volatile, but on the whole it seems cleaner to arrange things so that we don't change its value within PG_TRY. Per report from Xing Guo. This has been there for quite awhile, so back-patch to all supported branches. Discussion: https://postgr.es/m/CACpMh+DLrk=fDv07MNpBT4J413fDAm+gmMXgi8cjPONE+jvzuw@mail.gmail.com
* Fix unnecessary use of moving-aggregate mode with non-moving frame.Tom Lane2024-03-27
| | | | | | | | | | | | | | | | | | | | When a plain aggregate is used as a window function, and the window frame start is specified as UNBOUNDED PRECEDING, the frame's head cannot move so we do not need to use moving-aggregate mode. The check for that was put into initialize_peragg(), failing to notice that ExecInitWindowAgg() calls that function before it's filled in winstate->frameOptions. Since makeNode() would have zeroed the field, this didn't provoke uninitialized-value complaints, nor would the erroneous decision have resulted in more than a little inefficiency. Still, it's wrong, so move the initialization of winstate->frameOptions earlier to make it work properly. While here, also fix a thinko in a comment. Both errors crept in in commit a9d9acbf2 which introduced the moving-aggregate mode. Spotted by Vallimaharajan G. Back-patch to all supported branches. Discussion: https://postgr.es/m/18e7f2a5167.fe36253866818.977923893562469143@zohocorp.com
* Fix failure of ALTER FOREIGN TABLE SET SCHEMA to move sequences.Tom Lane2024-03-26
| | | | | | | | | | | | | | | | | | Ordinary ALTER TABLE SET SCHEMA will also move any owned sequences into the new schema. We failed to do likewise for foreign tables, because AlterTableNamespaceInternal believed that only certain relkinds could have indexes, owned sequences, or constraints. We could simply add foreign tables to that relkind list, but it seems likely that the same oversight could be made again in future. Instead let's remove the relkind filter altogether. These functions shouldn't cost much when there are no objects that they need to process, and surely this isn't an especially performance-critical case anyway. Per bug #18407 from Vidushi Gupta. Back-patch to all supported branches. Discussion: https://postgr.es/m/18407-4fd07373d252c6a0@postgresql.org
* Allow "make check"-style testing to work with musl C library.Tom Lane2024-03-26
| | | | | | | | | | | | | | | | | | | | | The musl dynamic linker saves a pointer to the process' environment value of LD_LIBRARY_PATH very early in startup. When we move/clobber the environment to make more room for ps status strings, we clobber that value and thereby prevent libraries from being found via LD_LIBRARY_PATH, which breaks the use of a temporary installation for testing purposes. To fix, stop collecting usable space for ps status if we notice that the variable we are about to clobber is LD_LIBRARY_PATH. This will result in some reduction in how long the ps status can be, but it's only likely to occur in temporary test contexts, so it doesn't seem like a big problem. In any case, we don't have to do it if we see we are on glibc, which surely is where the majority of our Linux testing is done. Thomas Munro, Bruce Momjian, and Tom Lane, per report from Wolfgang Walther. Back-patch to all supported branches, with the hope that we'll set up a buildfarm animal to test on this platform. Discussion: https://postgr.es/m/fddd1cd6-dc16-40a2-9eb5-d7fef2101488@technowledgy.de
* amcheck: Normalize index tuples containing uncompressed varlenaAlexander Korotkov2024-03-24
| | | | | | | | | | | | | It might happen that the varlena value wasn't compressed by index_form_tuple() due to current storage parameters. If compression is currently enabled, we need to compress such values to match index tuple coming from the heap. Backpatch to all supported versions. Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru Author: Andrey Borodin Reviewed-by: Alexander Lakhin, Michael Zhilin, Jian He, Alexander Korotkov Backpatch-through: 12
* amcheck: Support for different header sizes of short varlena datumAlexander Korotkov2024-03-23
| | | | | | | | | | | | | In the heap, tuples may contain short varlena datum with both 1B header and 4B headers. But the corresponding index tuple should always have such varlena's with 1B headers. So, for fingerprinting, we need to convert. Backpatch to all supported versions. Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru Author: Michael Zhilin Reviewed-by: Alexander Lakhin, Andrey Borodin, Jian He, Alexander Korotkov Backpatch-through: 12
* Fix typo in pg_dumpall role comments fixDaniel Gustafsson2024-03-22
| | | | | | | Some last minute polish of the patch managed to break the SQL query for extracting the role comments due to fat-fingering. Per the buildfarm Xversion tests.
* Fix dumping role comments when using --no-role-passwordsDaniel Gustafsson2024-03-21
| | | | | | | | | | | | | | | | | Commit 9a83d56b38c added support for allowing pg_dumpall to dump roles without including passwords, which accidentally made dumps omit COMMENTs on roles. This fixes it by using pg_authid to get the comment. Backpatch to all supported versions. Patch simultaneously written independently by Álvaro and myself. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Author: Daniel Gustafsson <daniel@yesql.se> Reported-by: Bartosz Chroł <bartosz.chrol@handen.pl> Discussion: https://postgr.es/m/AS8P194MB1271CDA0ADCA7B75FCD8E767F7332@AS8P194MB1271.EURP194.PROD.OUTLOOK.COM Discussion: https://postgr.es/m/CAEP4nAz9V4H41_4ESJd1Gf0v%3DdevkqO1%3Dpo91jUw-GJSx8Hxqg%40mail.gmail.com Backpatch-through: v12
* Review wording on tablespaces w.r.t. partitioned tablesAlvaro Herrera2024-03-20
| | | | | | | | | | | Remove a redundant comment, and document pg_class.reltablespace properly in catalogs.sgml. After commits a36c84c3e4a9, 87259588d0ab and others. Backpatch to 12. Discussion: https://postgr.es/m/202403191013.w2kr7wqlamqz@alvherre.pgsql
* Fix EXPLAIN Bitmap heap scan to count pages with no visible tuplesHeikki Linnakangas2024-03-18
| | | | | | | | | | | | | | | | | Previously, bitmap heap scans only counted lossy and exact pages for explain when there was at least one visible tuple on the page. heapam_scan_bitmap_next_block() returned true only if there was a "valid" page with tuples to be processed. However, the lossy and exact page counters in EXPLAIN should count the number of pages represented in a lossy or non-lossy way in the constructed bitmap, regardless of whether or not the pages ultimately contained visible tuples. Backpatch to all supported versions. Author: Melanie Plageman Discussion: https://www.postgresql.org/message-id/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA@mail.gmail.com Discussion: https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA@mail.gmail.com
* Make INSERT-from-multiple-VALUES-rows handle domain target columns.Tom Lane2024-03-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit a3c7a993d fixed some cases involving target columns that are arrays or composites by applying transformAssignedExpr to the VALUES entries, and then stripping off any assignment ArrayRefs or FieldStores that the transformation added. But I forgot about domains over arrays or composites :-(. Such cases would either fail with surprising complaints about mismatched datatypes, or insert unexpected coercions that could lead to odd results. To fix, extend the stripping logic to get rid of CoerceToDomain if it's atop an ArrayRef or FieldStore. While poking at this, I realized that there's a poorly documented and not-at-all-tested behavior nearby: we coerce each VALUES column to the domain type separately, and rely on the rewriter to merge those operations so that the domain constraints are checked only once. If that merging did not happen, it's entirely possible that we'd get unexpected domain constraint failures due to checking a partially-updated container value. There's no bug there, but while we're here let's improve the commentary about it and add some test cases that explicitly exercise that behavior. Per bug #18393 from Pablo Kharo. Back-patch to all supported branches. Discussion: https://postgr.es/m/18393-65fedb1a0de9260d@postgresql.org
* Fix confusion about the return rowtype of SQL-language procedures.Tom Lane2024-03-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | There is a very ancient hack in check_sql_fn_retval that allows a single SELECT targetlist entry of composite type to be taken as supplying all the output columns of a function returning composite. (This is grotty and fundamentally ambiguous, but it's really hard to do nested composite-returning functions without it.) As far as I know, that doesn't cause any problems in ordinary functions. It's disastrous for procedures however. All procedures that have any output parameters are labeled with prorettype RECORD, and the CALL code expects it will get back a record with one column per output parameter, regardless of whether any of those parameters is composite. Doing something else leads to an assertion failure or core dump. This is simple enough to fix: we just need to not apply that rule when considering procedures. However, that requires adding another argument to check_sql_fn_retval, which at least in principle might be getting called by external callers. Therefore, in the back branches convert check_sql_fn_retval into an ABI-preserving wrapper around a new function check_sql_fn_retval_ext. Per report from Yahor Yuzefovich. This has been broken since we implemented procedures, so back-patch to all supported branches. Discussion: https://postgr.es/m/CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com
* Disconnect if socket cannot be put into non-blocking modeHeikki Linnakangas2024-03-12
| | | | | | | | | | | | | | | | | | | | Commit 387da18874 moved the code to put socket into non-blocking mode from socket_set_nonblocking() into the one-time initialization function, pq_init(). In socket_set_nonblocking(), there indeed was a risk of recursion on failure like the comment said, but in pq_init(), ERROR or FATAL is fine. There's even another elog(FATAL) just after this, if setting FD_CLOEXEC fails. Note that COMMERROR merely logged the error, it did not close the connection, so if putting the socket to non-blocking mode failed we would use the connection anyway. You might not immediately notice, because most socket operations in a regular backend wait for the socket to become readable/writable anyway. But e.g. replication will be quite broken. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/d40a5cd0-2722-40c5-8755-12e9e811fa3c@iki.fi
* doc: add missing word "the"Bruce Momjian2024-03-11
| | | | | | | | Reported-by: doughale@gmail.com Discussion: https://postgr.es/m/170993253510.640.5664117187431542912@wrigleys.postgresql.org Backpatch-through: 12
* Backpatch missing check_stack_depth() to some recursive functionsAlexander Korotkov2024-03-11
| | | | | | | Backpatch changes from d57b7cc333, 75bcba6cbd to all supported branches per proposal of Egor Chindyaskin. Discussion: https://postgr.es/m/DE5FD776-A8CD-4378-BCFA-3BF30F1F6D60%40mail.ru
* Fix deparsing of Consts in postgres_fdw ORDER BYDavid Rowley2024-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For UNION ALL queries where a union child query contained a foreign table, if the targetlist of that query contained a constant, and the top-level query performed an ORDER BY which contained the column for the constant value, then postgres_fdw would find the EquivalenceMember with the Const and then try to produce an ORDER BY containing that Const. This caused problems with INT typed Consts as these could appear to be requests to order by an ordinal column position rather than the constant value. This could lead to either an error such as: ERROR: ORDER BY position <int const> is not in select list or worse, if the constant value is a valid column, then we could just sort by the wrong column altogether. Here we fix this issue by just not including these Consts in the ORDER BY clause. In passing, add a new section for testing ORDER BY in the postgres_fdw tests and move two existing tests which were misplaced in the WHERE clause testing section into it. Reported-by: Michał Kłeczek Reviewed-by: Ashutosh Bapat, Richard Guo Bug: #18381 Discussion: https://postgr.es/m/0714C8B8-8D82-4ABB-9F8D-A0C3657E7B6E%40kleczek.org Discussion: https://postgr.es/m/18381-137456acd168bf93%40postgresql.org Backpatch-through: 12, oldest supported version
* Cope with a deficiency in OpenSSL 3.x's error reporting.Tom Lane2024-03-07
| | | | | | | | | | | | | | | | In OpenSSL 3.0.0 and later, ERR_reason_error_string randomly refuses to provide a string for error codes representing system errno values (e.g., "No such file or directory"). There is a poorly-documented way to extract the errno from the SSL error code in this case, so do that and apply strerror, rather than falling back to reporting the error code's numeric value as we were previously doing. Problem reported by David Zhang, although this is not his proposed patch; it's instead based on a suggestion from Heikki Linnakangas. Back-patch to all supported branches, since any of them are likely to be used with recent OpenSSL. Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca