aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* freespace: Don't return blocks past the end of the main fork.Noah Misch2024-04-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GetPageWithFreeSpace() callers assume the returned block exists in the main fork, failing with "could not read block" errors if that doesn't hold. Make that assumption reliable now. It hadn't been guaranteed, due to the weak WAL and data ordering of participating components. Most operations on the fsm fork are not WAL-logged. Relation extension is not WAL-logged. Hence, an fsm-fork block on disk can reference a main-fork block that no WAL record has initialized. That could happen after an OS crash, a replica promote, or a PITR restore. wal_log_hints makes the trouble easier to hit; a replica promote or PITR ending just after a relevant fsm-fork FPI_FOR_HINT may yield this broken state. The v16 RelationAddBlocks() mechanism also makes the trouble easier to hit, since it bulk-extends even without extension lock waiters. Commit 917dc7d2393ce680dea7a59418be9ff341df3c14 stopped trouble around truncation, but vectors involving PageIsNew() pages remained. This implementation adds a RelationGetNumberOfBlocks() call when the cached relation size doesn't confirm a block exists. We've been unable to identify a benchmark that slows materially, but this may show up as additional time in lseek(). An alternative without that overhead would be a new ReadBufferMode such that ReadBufferExtended() returns NULL after a 0-byte read, with all other errors handled normally. However, each GetFreeIndexPage() caller would then need code for the return-NULL case. Back-patch to v14, due to earlier versions not caching relation size and the absence of a pre-v16 problem report. Ronan Dunklau. Reported by Ronan Dunklau. Discussion: https://postgr.es/m/1878547.tdWV9SEqCh%40aivenlaptop
* 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-
* Doc: update documentation about EXCLUDE constraint elements.Tom Lane2024-04-07
| | | | | | | | | | | | | | | | | | What the documentation calls an exclude_element is an index_elem according to gram.y, and it allows all the same options that a CREATE INDEX column specification does. The COLLATE patch neglected to update the CREATE/ALTER TABLE docs about that, and later the opclass-parameters patch made the same oversight. Add those options to the syntax synopses, and polish the associated text a bit. Back-patch to v13 where opclass parameters came in. We could update v12 with just the COLLATE omission, but it doesn't quite seem worth the trouble at this point. Shihao Zhong, reviewed by Daniel Vérité, Shubham Khanna and myself Discussion: https://postgr.es/m/CAGRkXqShbVyB8E3gapfdtuwiWTiK=Q67Qb9qwxu=+-w0w46EBA@mail.gmail.com
* 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 bogus coding in ExecAppendAsyncEventWait().Etsuro Fujita2024-04-04
| | | | | | | | | | | | | | | | No configured-by-FDW events would result in "return" directly out of a PG_TRY block, making the exception stack dangling. Repair. Oversight in commit 501cfd07d; back-patch to v14, like that commit, but as we do not have this issue in HEAD (cf. commit 50c67c201), no need to apply this patch to it. In passing, improve a comment about the handling of in-process requests in a postgres_fdw.c function called from this function. Alexander Pyhalov, with comment adjustment/improvement by me. Discussion: https://postgr.es/m/425fa29a429b21b0332737c42a4fdc70%40postgrespro.ru
* 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
* Clarify comment for LogicalTapeSetBlocks().Jeff Davis2024-03-25
| | | | | Discussion: https://postgr.es/m/1229327.1711160246@sss.pgh.pa.us Backpatch-through: 13
* amcheck: Normalize index tuples containing uncompressed varlenaAlexander Korotkov2024-03-23
| | | | | | | | | | | | | 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
* Remove incorrect Assert introduced in c8aeaf3ab.Jeff Davis2024-03-23
| | | | | | | | | | | | Already removed incidentally in version 15 (c4649cce3), so this commit is only applied to versions 13 and 14. The comment above is misleading in all versions 13 and later, so that will be fixed in a separate commit. Discussion: https://postgr.es/m/cfd84cb8-12fe-433a-a4bb-f460a4515f9c.zhaotinghai.zth%40alibaba-inc.com Reported-by: Tinghai Zhao Backpatch-through: 13
* 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
* Fix incorrect accessing of pfree'd memory in MemoizeDavid Rowley2024-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | For pass-by-reference types, the code added in 0b053e78b, which aimed to resolve a memory leak, was overly aggressive in resetting the per-tuple memory context which could result in pfree'd memory being accessed resulting in failing to find previously cached results in the hash table. What was happening was prepare_probe_slot() was switching to the per-tuple memory context and calling ExecEvalExpr(). ExecEvalExpr() may have required a memory allocation. Both MemoizeHash_hash() and MemoizeHash_equal() were aggressively resetting the per-tuple context and after determining the hash value, the context would have gotten reset before MemoizeHash_equal() was called. This could have resulted in MemoizeHash_equal() looking at pfree'd memory. This is less likely to have caused issues on a production build as some other allocation would have had to have reused the pfree'd memory to overwrite it. Otherwise, the original contents would have been intact. However, this clearly caused issues on MEMORY_CONTEXT_CHECKING builds. Author: Tender Wang, Andrei Lepikhov Reported-by: Tender Wang (using SQLancer) Reviewed-by: Andrei Lepikhov, Richard Guo, David Rowley Discussion: https://postgr.es/m/CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com Backpatch-through: 14, where Memoize was added
* 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
* Revert "Fix parallel-safety check of expressions and predicate for index builds"Michael Paquier2024-03-07
| | | | | | | | | | | This reverts commit eae7be600be7, following a discussion with Tom Lane, due to concerns that this impacts the decisions made by the planner for the number of workers spawned based on the inlining and const-folding of index expressions and predicate for cases that would have worked until this commit. Discussion: https://postgr.es/m/162802.1709746091@sss.pgh.pa.us Backpatch-through: 12
* Fix type-checking of RECORD-returning functions in FROM.Tom Lane2024-03-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the corner case where a function returning RECORD has been simplified to a RECORD constant or an inlined ROW() expression, ExecInitFunctionScan failed to cross-check the function's result rowtype against the coldeflist provided by the calling query. That happened because get_expr_result_type is able to extract a tupdesc from such expressions, which led ExecInitFunctionScan to ignore the coldeflist. (Instead, it used the extracted tupdesc to check the function's output, which of course always succeeds.) I have not been able to demonstrate any really serious consequences from this, because if some column of the result is of the wrong type and is directly referenced by a Var of the calling query, CheckVarSlotCompatibility will catch it. However, we definitely do fail to report the case where the function returns more columns than the coldeflist expects, and in the converse case where it returns fewer columns, we get an assert failure (but, seemingly, no worse results in non-assert builds). To fix, always build the expected tupdesc from the coldeflist if there is one, and consult get_expr_result_type only when there isn't one. Also remove the failing Assert, even though it is no longer reached after this fix. It doesn't seem to be adding anything useful, since later checking will deal with cases with the wrong number of columns. The only other place I could find that is doing something similar is inline_set_returning_function. There's no live bug there because we cannot be looking at a Const or RowExpr, but for consistency change that code to agree with ExecInitFunctionScan. Per report from PetSerAl. After some debate I've concluded that this should be back-patched. There is a small risk that somebody has been relying on such a case not throwing an error, but I judge this outweighed by the risk that I've missed some way in which the failure to cross-check has worse consequences than sketched above. Discussion: https://postgr.es/m/CAKygsHSerA1eXsJHR9wft3Gn3wfHQ5RfP8XHBzF70_qcrrRvEg@mail.gmail.com
* Fix parallel-safety check of expressions and predicate for index buildsMichael Paquier2024-03-06
| | | | | | | | | | | | | | | | | | | | | | | | As coded, the planner logic that calculates the number of parallel workers to use for a parallel index build uses expressions and predicates from the relcache, which are flattened for the planner by eval_const_expressions(). As reported in the bug, an immutable parallel-unsafe function flattened in the relcache would become a Const, which would be considered as parallel-safe, even if the predicate or the expressions including the function are not safe in parallel workers. Depending on the expressions or predicate used, this could cause the parallel build to fail. Tests are included that check parallel index builds with parallel-unsafe predicate and expressions. Two routines are added to lsyscache.h to be able to retrieve expressions and predicate of an index from its pg_index data. Reported-by: Alexander Lakhin Author: Tender Wang Reviewed-by: Jian He, Michael Paquier Discussion: https://postgr.es/m/CAHewXN=UaAaNn9ruHDH3Os8kxLVmtWqbssnf=dZN_s9=evHUFA@mail.gmail.com Backpatch-through: 12
* Fix incorrectly reported stats kind in "can't happen" ERRORDavid Rowley2024-03-05
| | | | | | | | | | The error message(s) were reporting the stats kind of 'f', which is not correct as that's for the "dependencies" statistics kind. Reported-by: Horst Reiterer Reviewed-by: Richard Guo Discussion: https://postgr.es/m/18375-ba99383eb9062d6a@postgresql.org Backpatch-through: 12, where MCV extended stats were added.
* Fix integer underflow in shared memory debuggingDaniel Gustafsson2024-02-29
| | | | | | | | | | | dsa_dump would print a large negative number instead of zero for segment bin 0. Fix by explicitly checking for underflow and add special case for bin 0. Backpatch to all supported versions. Author: Ian Ilyasov <ianilyasov@outlook.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/GV1P251MB1004E0D09D117D3CECF9256ECD502@GV1P251MB1004.EURP251.PROD.OUTLOOK.COM Backpatch-through: v12
* Fix mis-rounding and overflow hazards in date_bin().Tom Lane2024-02-28
| | | | | | | | | | | | | | | | | | | | In the case where the target timestamp is before the origin timestamp and their difference is already an exact multiple of the stride, the code incorrectly subtracted the stride anyway. Also detect several integer-overflow cases that previously produced bogus results. (The submitted patch tried to avoid overflow, but I'm not convinced it's right, and problematic cases are so far out of the plausibly-useful range that they don't seem worth sweating over. Let's just use overflow-detecting arithmetic and throw errors.) timestamp_bin() and timestamptz_bin() are basically identical and so had identical bugs. Fix both. Report and patch by Moaaz Assali, adjusted some by me. Back-patch to v14 where date_bin() was introduced. Discussion: https://postgr.es/m/CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@mail.gmail.com
* Promote assertion about !ReindexIsProcessingIndex to runtime error.Tom Lane2024-02-25
| | | | | | | | | | | | | | | | | | | | | When this assertion was installed (in commit d2f60a3ab), I thought it was only for catching server logic errors that caused accesses to catalogs that were undergoing index rebuilds. However, it will also fire in case of a user-defined index expression that attempts to access its own table. We occasionally see reports of people trying to do that, and typically getting unintelligible low-level errors as a result. We can provide a more on-point message by making this a regular runtime check. While at it, adjust the similar error check in systable_beginscan_ordered to use the same message text. That one is (probably) not reachable without a coding bug, but we might as well use a translatable message if we have one. Per bug #18363 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18363-e3598a5a572d0699@postgresql.org
* Doc: fix minor typos in two ECPG function descriptions.Tom Lane2024-02-25
| | | | | | Noted by Aidar Imamov. Discussion: https://postgr.es/m/170869935022.643.3709087848818148291@wrigleys.postgresql.org
* Avoid dangling-pointer problem with partitionwise joins under GEQO.Tom Lane2024-02-23
| | | | | | | | | | | | | | | | build_child_join_sjinfo creates a derived SpecialJoinInfo in the short-lived GEQO context, but afterwards the semi_rhs_exprs from that may be used in a UniquePath for a child base relation. This breaks the expectation that all base-relation-level structures are in the planning-lifespan context, leading to use of a dangling pointer with probable ensuing crash later on in create_unique_plan. To fix, copy the expression trees when making a UniquePath. Per bug #18360 from Alexander Lakhin. This has been broken since partitionwise joins were added, so back-patch to all supported branches. Discussion: https://postgr.es/m/18360-a23caf3157f34e62@postgresql.org
* Doc: improve explanation of type interval, especially extract().Tom Lane2024-02-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The explanation of interval's behavior in datatype.sgml wasn't wrong exactly, but it was unclear, partly because it buried the lede about there being three internal fields. Rearrange and wordsmith for more clarity. The discussion of extract() claimed that input of type date was handled by casting, but actually there's been a separate SQL function taking date for a very long time. Also, it was mostly silent about how interval inputs are handled, but there are several field types for which it seems useful to be specific. Improve discussion of justify_days()/justify_hours() too. In passing, remove vertical space in some groups of examples, as there was little consistency about whether to have such space or not. (I only did this within the datetime functions section; there are some related inconsistencies elsewhere.) Per discussion of bug #18348 from Michael Bondarenko. There may be some code changes coming out of that discussion too, but we likely won't back-patch them. This docs-only patch seems useful to back-patch, though I only carried it back to v13 because it didn't apply easily in v12. Discussion: https://postgr.es/m/18348-b097a3587dfde8a4@postgresql.org
* Doc: correct minor error in back-branch release notes.Tom Lane2024-02-20
| | | | | | | | | | | Commits 1b2c6b756 et al affected the core BRIN "bloom" opclasses, not contrib/bloom. This only corrected a bad assertion so it's not too significant to end users, but since we documented it we should do so accurately. Spotted by Takatsuka Haruka. Discussion: https://postgr.es/m/18353-926aa99cfe58aa78@postgresql.org
* Fix incorrect pruning of NULL partition for boolean IS NOT clausesDavid Rowley2024-02-20
| | | | | | | | | | | | | | | | | | | | | | | | Partition pruning wrongly assumed that, for a table partitioned on a boolean column, a clause in the form "boolcol IS NOT false" and "boolcol IS NOT true" could be inverted to correspondingly become "boolcol IS true" and "boolcol IS false". These are not equivalent as the NOT version matches the opposite boolean value *and* NULLs. This incorrect assumption meant that partition pruning pruned away partitions that could contain NULL values. Here we fix this by correctly not pruning partitions which could store NULLs. To be affected by this, the table must be partitioned by a NULLable boolean column and queries would have to contain "boolcol IS NOT false" or "boolcol IS NOT true". This could result in queries filtering out NULL values with a LIST partitioned table and "ERROR: invalid strategy number 0" for RANGE and HASH partitioned tables. Reported-by: Alexander Lakhin Bug: #18344 Discussion: https://postgr.es/m/18344-8d3f00bada6d09c6@postgresql.org Backpatch-through: 12
* Doc: fix typo in SECURITY LABEL synopsis.Tom Lane2024-02-19
| | | | | | | | One case missed its trailing "|". Reported by Tim Needham. Discussion: https://postgr.es/m/170833547220.3279712.700702770281879175@wrigleys.postgresql.org
* ecpg: Fix zero-termination of string generated by intoasc()Michael Paquier2024-02-19
| | | | | | | | | | | | | | | | | intoasc(), a wrapper for PGTYPESinterval_to_asc that converts an interval to its textual representation, used a plain memcpy() when copying its result. This could miss a zero-termination in the result string, leading to an incorrect result. The routines in informix.c do not provide the length of their result buffer, which would allow a replacement of strcpy() to safer strlcpy() calls, but this requires an ABI breakage and that cannot happen in back-branches. Author: Oleg Tselebrovskiy Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/bf47888585149f83b276861a1662f7e4@postgrespro.ru Backpatch-through: 12
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-03 18:07:19 +0000