aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix memory leak in Memoize codeDavid Rowley2023-10-05
| | | | | | | | | | Ensure we switch to the per-tuple memory context to prevent any memory leaks of detoasted Datums in MemoizeHash_hash() and MemoizeHash_equal(). Reported-by: Orlov Aleksej Author: Orlov Aleksej, David Rowley Discussion: https://postgr.es/m/83281eed63c74e4f940317186372abfd%40cft.ru Backpatch-through: 14, where Memoize was added
* Avoid memory size overflow when allocating backend activity bufferMichael Paquier2023-10-03
| | | | | | | | | | | | | | | | | | | | | | The code in charge of copying the contents of PgBackendStatus to local memory could fail on memory allocation because of an overflow on the amount of memory to use. The overflow can happen when combining a high value track_activity_query_size (max at 1MB) with a large max_connections, when both multiplied get higher than INT32_MAX as both parameters treated as signed integers. This could for example trigger with the following functions, all calling pgstat_read_current_status(): - pg_stat_get_backend_subxact() - pg_stat_get_backend_idset() - pg_stat_get_progress_info() - pg_stat_get_activity() - pg_stat_get_db_numbackends() The change to use MemoryContextAllocHuge() has been introduced in 8d0ddccec636, so backpatch down to 12. Author: Jakub Wartak Discussion: https://postgr.es/m/CAKZiRmw8QSNVw2qNK-dznsatQqz+9DkCquxP0GHbbv1jMkGHMA@mail.gmail.com Backpatch-through: 12
* Fail hard on out-of-memory failures in xlogreader.cMichael Paquier2023-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit changes the WAL reader routines so as a FATAL for the backend or exit(FAILURE) for the frontend is triggered if an allocation for a WAL record decode fails in walreader.c, rather than treating this case as bogus data, which would be equivalent to the end of WAL. The key is to avoid palloc_extended(MCXT_ALLOC_NO_OOM) in walreader.c, relying on plain palloc() calls. The previous behavior could make WAL replay finish too early than it should. For example, crash recovery finishing earlier may corrupt clusters because not all the WAL available locally was replayed to ensure a consistent state. Out-of-memory failures would show up randomly depending on the memory pressure on the host, but one simple case would be to generate a large record, then replay this record after downsizing a host, as Ethan Mertz originally reported. This relies on bae868caf222, as the WAL reader routines now do the memory allocation required for a record only once its header has been fully read and validated, making xl_tot_len trustable. Making the WAL reader react differently on out-of-memory or bogus record data would require ABI changes, so this is the safest choice for stable branches. Also, it is worth noting that 3f1ce973467a has been using a plain palloc() in this code for some time now. Thanks to Noah Misch and Thomas Munro for the discussion. Like the other commit, backpatch down to 12, leaving out v11 that will be EOL'd soon. The behavior of considering a failed allocation as bogus data comes originally from 0ffe11abd3a0, where the record length retrieved from its header was not entirely trustable. Reported-by: Ethan Mertz Discussion: https://postgr.es/m/ZRKKdI5-RRlta3aF@paquier.xyz Backpatch-through: 12
* Flush WAL stats in bgwriterHeikki Linnakangas2023-10-02
| | | | | | | | | | | bgwriter can write out WAL, but did not flush the WAL pgstat counters, so the writes were not seen in pg_stat_wal. Back-patch to v14, where pg_stat_wal was introduced. Author: Nazir Bilal Yavuz Reviewed-by: Matthias van de Meent, Kyotaro Horiguchi Discussion: https://www.postgresql.org/message-id/CAN55FZ2FPYngovZstr%3D3w1KSEHe6toiZwrurbhspfkXe5UDocg%40mail.gmail.com
* Fix datalen calculation in tsvectorrecv().Tom Lane2023-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After receiving position data for a lexeme, tsvectorrecv() advanced its "datalen" value by (npos+1)*sizeof(WordEntry) where the correct calculation is (npos+1)*sizeof(WordEntryPos). This accidentally failed to render the constructed tsvector invalid, but it did result in leaving some wasted space approximately equal to the space consumed by the position data. That could have several bad effects: * Disk space is wasted if the received tsvector is stored into a table as-is. * A legal tsvector could get rejected with "maximum total lexeme length exceeded" if the extra space pushes it over the MAXSTRPOS limit. * In edge cases, the finished tsvector could be assigned a length larger than the allocated size of its palloc chunk, conceivably leading to SIGSEGV when the tsvector gets copied somewhere else. The odds of a field failure of this sort seem low, though valgrind testing could probably have found this. While we're here, let's express the calculation as "sizeof(uint16) + npos * sizeof(WordEntryPos)" to avoid the type pun implicit in the "npos + 1" formulation. It's not wrong given that WordEntryPos had better be 2 bytes to avoid padding problems, but it seems clearer this way. Report and patch by Denis Erokhin. Back-patch to all supported versions. Discussion: https://postgr.es/m/009801d9f2d9$f29730c0$d7c59240$@datagile.ru
* In COPY FROM, fail cleanly when unsupported encoding conversion is needed.Tom Lane2023-10-01
| | | | | | | | | | | | In recent releases, such cases fail with "cache lookup failed for function 0" rather than complaining that the conversion function doesn't exist as prior versions did. Seems to be a consequence of sloppy refactoring in commit f82de5c46. Add the missing error check. Per report from Pierre Fortin. Back-patch to v14 where the oversight crept in. Discussion: https://postgr.es/m/20230929163739.3bea46e5.pfortin@pfortin.com
* Fix briefly showing old progress stats for ANALYZE on inherited tables.Heikki Linnakangas2023-09-30
| | | | | | | | | | | | | | | | | | | ANALYZE on a table with inheritance children analyzes all the child tables in a loop. When stepping to next child table, it updated the child rel ID value in the command progress stats, but did not reset the 'sample_blks_total' and 'sample_blks_scanned' counters. acquire_sample_rows() updates 'sample_blks_total' as soon as the scan starts and 'sample_blks_scanned' after processing the first block, but until then, pg_stat_progress_analyze would display a bogus combination of the new child table relid with old counter values from the previously processed child table. Fix by resetting 'sample_blks_total' and 'sample_blks_scanned' to zero at the same time that 'current_child_table_relid' is updated. Backpatch to v13, where pg_stat_progress_analyze view was introduced. Reported-by: Justin Pryzby Discussion: https://www.postgresql.org/message-id/20230122162345.GP13860%40telsasoft.com
* Fix EvalPlanQual rechecking during MERGE.Dean Rasheed2023-09-30
| | | | | | | | | | | | | | | | | | Under some circumstances, concurrent MERGE operations could lead to inconsistent results, that varied according the plan chosen. This was caused by a lack of rowmarks on the source relation, which meant that EvalPlanQual rechecking was not guaranteed to return the same source tuples when re-running the join query. Fix by ensuring that preprocess_rowmarks() sets up PlanRowMarks for all non-target relations used in MERGE, in the same way that it does for UPDATE and DELETE. Per bug #18103. Back-patch to v15, where MERGE was introduced. Dean Rasheed, reviewed by Richard Guo. Discussion: https://postgr.es/m/18103-c4386baab8e355e3%40postgresql.org
* Fix btmarkpos/btrestrpos array key wraparound bug.Peter Geoghegan2023-09-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | nbtree's mark/restore processing failed to correctly handle an edge case involving array key advancement and related search-type scan key state. Scans with ScalarArrayScalarArrayOpExpr quals requiring mark/restore processing (for a merge join) could incorrectly conclude that an affected array/scan key must not have advanced during the time between marking and restoring the scan's position. As a result of all this, array key handling within btrestrpos could skip a required call to _bt_preprocess_keys(). This confusion allowed later primitive index scans to overlook tuples matching the true current array keys. The scan's search-type scan keys would still have spurious values corresponding to the final array element(s) -- not values matching the first/now-current array element(s). To fix, remember that "array key wraparound" has taken place during the ongoing btrescan in a flag variable stored in the scan's state, and use that information at the point where btrestrpos decides if another call to _bt_preprocess_keys is required. Oversight in commit 70bc5833, which taught nbtree to handle array keys during mark/restore processing, but missed this subtlety. That commit was itself a bug fix for an issue in commit 9e8da0f7, which taught nbtree to handle ScalarArrayOpExpr quals natively. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-WzkgP3DDRJxw6DgjCxo-cu-DKrvjEv_ArkP2ctBJatDCYg@mail.gmail.com Backpatch: 11- (all supported branches).
* Fix checking of index expressions in CompareIndexInfo().Tom Lane2023-09-28
| | | | | | | | | | | | | | | | | | | | | | | This code was sloppy about comparison of index columns that are expressions. It didn't reliably reject cases where one index has an expression where the other has a plain column, and it could index off the start of the attmap array, leading to a Valgrind complaint (though an actual crash seems unlikely). I'm not sure that the expression-vs-column sloppiness leads to any visible problem in practice, because the subsequent comparison of the two expression lists would reject cases where the indexes have different numbers of expressions overall. Maybe we could falsely match indexes having the same expressions in different column positions, but it'd require unlucky contents of the word before the attmap array. It's not too surprising that no problem has been reported from the field. Nonetheless, this code is clearly wrong. Per bug #18135 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18135-532f4a755e71e4d2@postgresql.org
* Add missing TidRangePath handling in print_path()David Rowley2023-09-29
| | | | | | | | | | | | | Tid Range scans were added back in bb437f995. That commit forgot to add handling for TidRangePaths in print_path(). Only people building with OPTIMIZER_DEBUG might have noticed this, which likely is the reason it's taken 4 years for anyone to notice. Author: Andrey Lepikhov Reported-by: Andrey Lepikhov Discussion: https://postgr.es/m/379082d6-1b6a-4cd6-9ecf-7157d8c08635@postgrespro.ru Backpatch-through: 14, where bb437f995 was introduced
* Fix typo in src/backend/access/transam/README.Etsuro Fujita2023-09-28
|
* Fix another bug in parent page splitting during GiST index build.Heikki Linnakangas2023-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Yet another bug in the ilk of commits a7ee7c851 and 741b88435. In 741b88435, we took care to clear the memorized location of the downlink when we split the parent page, because splitting the parent page can move the downlink. But we missed that even *updating* a tuple on the parent can move it, because updating a tuple on a gist page is implemented as a delete+insert, so the updated tuple gets moved to the end of the page. This commit fixes the bug in two different ways (belt and suspenders): 1. Clear the downlink when we update a tuple on the parent page, even if it's not split. This the same approach as in commits a7ee7c851 and 741b88435. I also noticed that gistFindCorrectParent did not clear the 'downlinkoffnum' when it stepped to the right sibling. Fix that too, as it seems like a clear bug even though I haven't been able to find a test case to hit that. 2. Change gistFindCorrectParent so that it treats 'downlinkoffnum' merely as a hint. It now always first checks if the downlink is still at that location, and if not, it scans the page like before. That's more robust if there are still more cases where we fail to clear 'downlinkoffnum' that we haven't yet uncovered. With this, it's no longer necessary to meticulously clear 'downlinkoffnum', so this makes the previous fixes unnecessary, but I didn't revert them because it still seems nice to clear it when we know that the downlink has moved. Also add the test case using the same test data that Alexander posted. I tried to reduce it to a smaller test, and I also tried to reproduce this with different test data, but I was not able to, so let's just include what we have. Backpatch to v12, like the previous fixes. Reported-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/18129-caca016eaf0c3702@postgresql.org
* Fix behavior of "force" in pgstat_report_wal()Michael Paquier2023-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As implemented in 5891c7a8ed8f, setting "force" to true in pgstat_report_wal() causes the routine to not wait for the pgstat shmem lock if it cannot be acquired, in which case the WAL and I/O statistics finish by not being flushed. The origin of the confusion comes from pgstat_flush_wal() and pgstat_flush_io(), that use "nowait" as sole argument. The I/O stats are new in v16. This is the opposite behavior of what has been used in pgstat_report_stat(), where "force" is the opposite of "nowait". In this case, when "force" is true, the routine sets "nowait" to false, which would cause the routine to wait for the pgstat shmem lock, ensuring that the stats are always flushed. When "force" is false, "nowait" is set to true, and the stats would only not be flushed if the pgstat shmem lock can be acquired, returning immediately without flushing the stats if the lock cannot be acquired. This commit changes pgstat_report_wal() so as "force" has the same behavior as in pgstat_report_stat(). There are currently three callers of pgstat_report_wal(): - Two in the checkpointer where force=true during a shutdown and the main checkpointer loop. Now the code behaves so as the stats are always flushed. - One in the main loop of the bgwriter, where force=false. Now the code behaves so as the stats would not be flushed if the pgstat shmem lock could not be acquired. Before this commit, some stats on WAL and I/O could have been lost after a shutdown, for example. Reported-by: Ryoga Yoshida Author: Ryoga Yoshida, Michael Paquier Discussion: https://postgr.es/m/f87a4d7be70530606b864fd1df91718c@oss.nttdata.com Backpatch-through: 15
* Fix edge-case for xl_tot_len broken by bae868ca.Thomas Munro2023-09-26
| | | | | | | | | | | | | | | | | bae868ca removed a check that was still needed. If you had an xl_tot_len at the end of a page that was too small for a record header, but not big enough to span onto the next page, we'd immediately perform the CRC check using a bogus large length. Because of arbitrary coding differences between the CRC implementations on different platforms, nothing very bad happened on common modern systems. On systems using the _sb8.c fallback we could segfault. Restore that check, add a new assertion and supply a test for that case. Back-patch to 12, like bae868ca. Tested-by: Tom Lane <tgl@sss.pgh.pa.us> Tested-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGLCkTT7zYjzOxuLGahBdQ%3DMcF%3Dz5ZvrjSOnW4EDhVjT-g%40mail.gmail.com
* Collect dependency information for parsed CallStmts.Tom Lane2023-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Parse analysis of a CallStmt will inject mutable information, for instance the OID of the called procedure, so that subsequent DDL may create a need to re-parse the CALL. We failed to detect this for CALLs in plpgsql routines, because no dependency information was collected when putting a CallStmt into the plan cache. That could lead to misbehavior or strange errors such as "cache lookup failed". Before commit ee895a655, the issue would only manifest for CALLs appearing in atomic contexts, because we re-planned non-atomic CALLs every time through anyway. It is now apparent that extract_query_dependencies() probably needs a special case for every utility statement type for which stmt_requires_parse_analysis() returns true. I wanted to add something like Assert(!stmt_requires_parse_analysis(...)) when falling out of extract_query_dependencies_walker without doing anything, but there are API issues as well as a more fundamental point: stmt_requires_parse_analysis is supposed to be applied to raw parser output, so it'd be cheating to assume it will give the correct answer for post-parse-analysis trees. I contented myself with adding a comment. Per bug #18131 from Christian Stork. Back-patch to all supported branches. Discussion: https://postgr.es/m/18131-576854e79c5cd264@postgresql.org
* Limit to_tsvector_byid's initial array allocation to something sane.Tom Lane2023-09-25
| | | | | | | | | | | | The initial estimate of the number of distinct ParsedWords is just that: an estimate. Don't let it exceed what palloc is willing to allocate. If in fact we need more entries, we'll eventually fail trying to enlarge the array. But if we don't, this allows success on inputs that currently draw "invalid memory alloc request size". Per bug #18080 from Uwe Binder. Back-patch to all supported branches. Discussion: https://postgr.es/m/18080-d5c5e58fef8c99b7@postgresql.org
* Don't trust unvalidated xl_tot_len.Thomas Munro2023-09-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | xl_tot_len comes first in a WAL record. Usually we don't trust it to be the true length until we've validated the record header. If the record header was split across two pages, previously we wouldn't do the validation until after we'd already tried to allocate enough memory to hold the record, which was bad because it might actually be garbage bytes from a recycled WAL file, so we could try to allocate a lot of memory. Release 15 made it worse. Since 70b4f82a4b5, we'd at least generate an end-of-WAL condition if the garbage 4 byte value happened to be > 1GB, but we'd still try to allocate up to 1GB of memory bogusly otherwise. That was an improvement, but unfortunately release 15 tries to allocate another object before that, so you could get a FATAL error and recovery could fail. We can fix both variants of the problem more fundamentally using pre-existing page-level validation, if we just re-order some logic. The new order of operations in the split-header case defers all memory allocation based on xl_tot_len until we've read the following page. At that point we know that its first few bytes are not recycled data, by checking its xlp_pageaddr, and that its xlp_rem_len agrees with xl_tot_len on the preceding page. That is strong evidence that xl_tot_len was truly the start of a record that was logged. This problem was most likely to occur on a standby, because walreceiver.c recycles WAL files without zeroing out trailing regions of each page. We could fix that too, but that wouldn't protect us from rare crash scenarios where the trailing zeroes don't make it to disk. With reliable xl_tot_len validation in place, the ancient policy of considering malloc failure to indicate corruption at end-of-WAL seems quite surprising, but changing that is left for later work. Also included is a new TAP test to exercise various cases of end-of-WAL detection by writing contrived data into the WAL from Perl. Back-patch to 12. We decided not to put this change into the final release of 11. Author: Thomas Munro <thomas.munro@gmail.com> Author: Michael Paquier <michael@paquier.xyz> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> (the idea, not the code) Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Sergei Kornilov <sk@zsrv.org> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/17928-aa92416a70ff44a2%40postgresql.org
* Avoid potential pfree on NULL on OpenSSL errorsDaniel Gustafsson2023-09-22
| | | | | | | | | | | | Guard against the pointer being NULL before pfreeing upon an error returned from OpenSSL. Also handle errors from X509_NAME_print_ex which also can return -1 on memory allocation errors. Backpatch down to v15 where the code was added. Author: Sergey Shinderuk <s.shinderuk@postgrespro.ru> Discussion: https://postgr.es/m/8db5374d-32e0-6abb-d402-40762511eff2@postgrespro.ru Backpatch-through: v15
* Fix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions.Tom Lane2023-09-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In older branches, COMMIT/ROLLBACK AND CHAIN failed to propagate the current transaction's properties to the new transaction if there was any open subtransaction (unreleased savepoint). Instead, some previous transaction's properties would be restored. This is because the "if (s->chain)" check in CommitTransactionCommand examined the wrong instance of the "chain" flag and falsely concluded that it didn't need to save transaction properties. Our regression tests would have noticed this, except they used identical transaction properties for multiple tests in a row, so that the faulty behavior was not distinguishable from correct behavior. Commit 12d768e70 fixed the problem in v15 and later, but only rather accidentally, because I removed the "if (s->chain)" test to avoid a compiler warning, while not realizing that the warning was flagging a real bug. In v14 and before, remove the if-test and save transaction properties unconditionally; just as in the newer branches, that's not expensive enough to justify thinking harder. Add the comment and extra regression test to v15 and later to forestall any future recurrence, but there's no live bug in those branches. Patch by me, per bug #18118 from Liu Xiang. Back-patch to v12 where the AND CHAIN feature was added. Discussion: https://postgr.es/m/18118-4b72fcbb903aace6@postgresql.org
* Update comment about set_join_pathlist_hook().Etsuro Fujita2023-09-21
| | | | | | | | | | | | | | | | The comment introduced by commit e7cb7ee14 was a bit too terse, which could lead to extensions doing different things within the hook function than we intend to allow. Extend the comment to explain what they can do within the hook function. Back-patch to all supported branches. In passing, I rephrased a nearby comment that I recently added to the back branches. Reviewed by David Rowley and Andrei Lepikhov. Discussion: https://postgr.es/m/CAPmGK15SBPA1nr3Aqsdm%2BYyS-ay0Ayo2BRYQ8_A2To9eLqwopQ%40mail.gmail.com
* Fix GiST README's explanation of the NSN cross-check.Heikki Linnakangas2023-09-19
| | | | | | | | The text got the condition backwards, it's "NSN > LSN", not "NSN < LSN". While we're at it, expand it a little for clarity. Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/4cb46e18-e688-524a-0f73-b1f03ed5d6ee@iki.fi
* Don't crash if cursor_to_xmlschema is used on a non-data-returning Portal.Tom Lane2023-09-18
| | | | | | | | | | | | | | | | cursor_to_xmlschema() assumed that any Portal must have a tupDesc, which is not so. Add a defensive check. It's plausible that this mistake occurred because of the rather poorly chosen name of the lookup function SPI_cursor_find(), which in such cases is returning something that isn't very much like a cursor. Add some documentation to try to forestall future errors of the same ilk. Report and patch by Boyu Yang (docs changes by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/dd343010-c637-434c-a8cb-418f53bda3b8.yangboyu.yby@alibaba-inc.com
* Track nesting depth correctly when drilling down into RECORD Vars.Tom Lane2023-09-15
| | | | | | | | | | | | | | | | | | expandRecordVariable() failed to adjust the parse nesting structure correctly when recursing to inspect an outer-level Var. This could result in assertion failures or core dumps in corner cases. Likewise, get_name_for_var_field() failed to adjust the deparse namespace stack correctly when recursing to inspect an outer-level Var. In this case the likely result was a "bogus varno" error while deparsing a view. Per bug #18077 from Jingzhou Fu. Back-patch to all supported branches. Richard Guo, with some adjustments by me Discussion: https://postgr.es/m/18077-b9db97c6e0ab45d8@postgresql.org
* Revert "Improve error message on snapshot import in snapmgr.c"Michael Paquier2023-09-14
| | | | | | | | | | This reverts commit a0d87bcd9b57, following a remark from Andres Frend that the new error can be triggered with an incorrect SET TRANSACTION SNAPSHOT command without being really helpful for the user as it uses the internal file name. Discussion: https://postgr.es/m/20230914020724.hlks7vunitvtbbz4@awork3.anarazel.de Backpatch-through: 11
* Improve error message on snapshot import in snapmgr.cMichael Paquier2023-09-14
| | | | | | | | | | | | | | | | | When a snapshot file fails to be read in ImportSnapshot(), it would issue an ERROR as "invalid snapshot identifier" when opening a stream for it in read-only mode. This error message is reworded to be the same as all the other messages used in this case on failure, which is useful when debugging this area. Thinko introduced by bb446b689b66 where snapshot imports have been added. A backpatch down to 11 is done as this can improve any work related to snapshot imports in older branches. Author: Bharath Rupireddy Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CALj2ACWmr=3KdxDkm8h7Zn1XxBoF6hdzq8WQyMn2y1OL5RYFrg@mail.gmail.com Backpatch-through: 11
* Fix incorrect logic in plan dependency recordingDavid Rowley2023-09-14
| | | | | | | | | | | | | | | Both 50e17ad28 and 29f45e299 mistakenly tried to record a plan dependency on a function but mistakenly inverted the OidIsValid test. This meant that we'd record a dependency only when the function's Oid was InvalidOid. Clearly this was meant to *not* record the dependency in that case. 50e17ad28 made this mistake first, then in v15 29f45e299 copied the same mistake. Reported-by: Tom Lane Backpatch-through: 14, where 50e17ad28 first made this mistake Discussion: https://postgr.es/m/2277537.1694301772@sss.pgh.pa.us
* Fix exception safety bug in typcache.c.Thomas Munro2023-09-13
| | | | | | | | | | | | | | | | If an out-of-memory error was thrown at an unfortunate time, ensure_record_cache_typmod_slot_exists() could leak memory and leave behind a global state that produced an infinite loop on the next call. Fix by merging RecordCacheArray and RecordIdentifierArray into a single array. With only one allocation or re-allocation, there is no intermediate state. Back-patch to all supported releases. Reported-by: "James Pang (chaolpan)" <chaolpan@cisco.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/PH0PR11MB519113E738814BDDA702EDADD6EFA%40PH0PR11MB5191.namprd11.prod.outlook.com
* Fix uninitialized access to InitialRunningXacts during decoding after ERROR.Amit Kapila2023-09-12
| | | | | | | | | | | | | | | | | | | | The transactions and subtransactions array that was allocated under snapshot builder memory context and recorded during decoding was not cleared in case of errors. This can result in an assertion failure if we attempt to retry logical decoding within the same session. To address this issue, we register a callback function under the snapshot builder memory context to clear the recorded transactions and subtransactions array along with the context. This problem doesn't exist in PG16 and HEAD as instead of using InitialRunningXacts, we added the list of transaction IDs and sub-transaction IDs, that have modified catalogs and are running during snapshot serialization, to the serialized snapshot (see commit 7f13ac8123). Author: Hou Zhijie Reviewed-by: Amit Kapila Backpatch-through: 11 Discussion: http://postgr.es/m/18055-ab3beed9f4b7b7d6@postgresql.org
* Fix out-of-bound read in gtsvector_picksplit()Michael Paquier2023-09-04
| | | | | | | | | | | | | | This could lead to an imprecise choice when splitting an index page of a GiST index on a tsvector, deciding which entries should remain on the old page and which entries should move to a new page. This is wrong since tsearch2 has been moved into core with commit 140d4ebcb46e, so backpatch all the way down. This error has been spotted by valgrind. Author: Alexander Lakhin Discussion: https://postgr.es/m/17950-6c80a8d2b94ec695@postgresql.org Backpatch-through: 11
* Fix handling of shared statistics with dropped databasesMichael Paquier2023-09-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Dropping a database while a connection is attempted on it was able to lead to the presence of valid database entries in shared statistics. The issue is that MyDatabaseId was getting set too early than it should, as, if the connection attempted on the dropped database fails when renamed or dropped, the shutdown callback of the shared statistics would finish by re-inserting a correct entry related to the database already dropped. As analyzed by the bug reporters, this issue could lead to phantom entries in the database list maintained by the autovacuum launcher (in rebuild_database_list()) if the database dropped was part of the database list when it was still valid. After the database was dropped, it would remain the highest on the list of databases to considered by the autovacuum worker as things to process. This would prevent autovacuum jobs to happen on all the other databases still present. The commit fixes this issue by delaying setting MyDatabaseId until the database existence has been re-checked with the second scan on pg_database after getting a shared lock on it, and by switching pgstat_update_dbstats() so as nothing happens if MyDatabaseId is not valid. Issue introduced by 5891c7a8ed8f, so backpatch down to 15. Reported-by: Will Mortensen, Jacob Speidel Analyzed-by: Will Mortensen, Jacob Speidel Author: Andres Freund Discussion: https://postgr.es/m/17973-bca1f7d5c14f601e@postgresql.org Backpatch-through: 15
* Avoid possible overflow with ltsGetFreeBlock() in logtape.cMichael Paquier2023-08-30
| | | | | | | | | | | | | | | | | | nFreeBlocks, defined as a long, stores the number of free blocks in a logical tape. ltsGetFreeBlock() has been using an int to store the value of nFreeBlocks, which could lead to overflows on platforms where long and int are not the same size (in short everything except Windows where long is 4 bytes). The problematic intermediate variable is switched to be a long instead of an int. Issue introduced by c02fdc9223015, so backpatch down to 13. Author: Ranier vilela Reviewed-by: Peter Geoghegan, David Rowley Discussion: https://postgr.es/m/CAEudQApLDWCBR_xmwNjGBrDo+f+S4E87x3s7-+hoaKqYdtC4JQ@mail.gmail.com Backpatch-through: 13
* Initialize ListenSocket array earlier.Heikki Linnakangas2023-08-29
| | | | | | | | | | | | | | | | | | | After commit b0bea38705, syslogger prints 63 warnings about failing to close a listen socket at postmaster startup. That's because the syslogger process forks before the ListenSockets array is initialized, so ClosePostmasterPorts() calls "close(0)" 64 times. The first call succeeds, because fd 0 is stdin. This has been like this since commit 9a86f03b4e in version 13, which moved the SysLogger_Start() call to before initializing ListenSockets. We just didn't notice until commit b0bea38705 added the LOG message. Reported by Michael Paquier and Jeff Janes. Author: Michael Paquier Discussion: https://www.postgresql.org/message-id/ZOvvuQe0rdj2slA9%40paquier.xyz Discussion: https://www.postgresql.org/message-id/ZO0fgDwVw2SUJiZx@paquier.xyz#482670177eb4eaf4c9f03c1eed963e5f Backpatch-through: 13
* Avoid unnecessary plancache revalidation of utility statements.Tom Lane2023-08-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Revalidation of a plancache entry (after a cache invalidation event) requires acquiring a snapshot. Normally that is harmless, but not if the cached statement is one that needs to run without acquiring a snapshot. We were already aware of that for TransactionStmts, but for some reason hadn't extrapolated to the other statements that PlannedStmtRequiresSnapshot() knows mustn't set a snapshot. This can lead to unexpected failures of commands such as SET TRANSACTION ISOLATION LEVEL. We can fix it in the same way, by excluding those command types from revalidation. However, we can do even better than that: there is no need to revalidate for any statement type for which parse analysis, rewrite, and plan steps do nothing interesting, which is nearly all utility commands. To mechanize this, invent a parser function stmt_requires_parse_analysis() that tells whether parse analysis does anything beyond wrapping a CMD_UTILITY Query around the raw parse tree. If that's what it does, then rewrite and plan will just skip the Query, so that it is not possible for the same raw parse tree to produce a different plan tree after cache invalidation. stmt_requires_parse_analysis() is basically equivalent to the existing function analyze_requires_snapshot(), except that for obscure reasons that function omits ReturnStmt and CallStmt. It is unclear whether those were oversights or intentional. I have not been able to demonstrate a bug from not acquiring a snapshot while analyzing these commands, but at best it seems mighty fragile. It seems safer to acquire a snapshot for parse analysis of these commands too, which allows making stmt_requires_parse_analysis and analyze_requires_snapshot equivalent. In passing this fixes a second bug, which is that ResetPlanCache would exclude ReturnStmts and CallStmts from revalidation. That's surely *not* safe, since they contain parsable expressions. Per bug #18059 from Pavel Kulakov. Back-patch to all supported branches. Discussion: https://postgr.es/m/18059-79c692f036b25346@postgresql.org
* Cache by-reference missing values in a long lived contextAndrew Dunstan2023-08-22
| | | | | | | | | | | | | | | | Attribute missing values might be needed past the lifetime of the tuple descriptors from which they are extracted. To avoid possibly using pointers for by-reference values which might thus be left dangling, we cache a datumCopy'd version of the datum in the TopMemoryContext. Since we first search for the value this only needs to be done once per session for any such value. Original complaint from Tom Lane, idea for mitigation by Andrew Dunstan, tweaked by Tom Lane. Backpatch to version 11 where missing values were introduced. Discussion: https://postgr.es/m/1306569.1687978174@sss.pgh.pa.us
* Fix pg_stat_reset_single_table_counters() for shared relationsMichael Paquier2023-08-21
| | | | | | | | | | | | This commit fixes the function of $subject for shared relations. This feature has been added by e042678. Unfortunately, this new behavior got removed by 5891c7a when moving statistics to shared memory. Reported-by: Mitsuru Hinata Author: Masahiro Ikeda Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada Discussion: https://postgr.es/m/7cc69f863d9b1bc677544e3accd0e4b4@oss.nttdata.com Backpatch-through: 15
* Invalidate smgr_targblock in smgrrelease().Thomas Munro2023-08-17
| | | | | | | | | | In rare circumstances involving relfilenode reuse, it might have been possible for smgr_targblock to finish up pointing past the end. Oversight in b74e94dc. Back-patch to 15. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CA%2BhUKGJ8NTvqLHz6dqbQnt2c8XCki4r2QvXjBQcXpVwxTY_pvA%40mail.gmail.com
* Recalculate search_path after ALTER ROLE.Jeff Davis2023-08-07
| | | | | | | | | Renaming a role can affect the meaning of the special string $user, so must cause search_path to be recalculated. Discussion: https://postgr.es/m/186761d32c0255debbdf50b6310b581b9c973e6c.camel@j-davis.com Reviewed-by: Nathan Bossart, Michael Paquier Backpatch-through: 11
* Reject substituting extension schemas or owners matching ["$'\].Noah Misch2023-08-07
| | | | | | | | | | | | | | | | | | | Substituting such values in extension scripts facilitated SQL injection when @extowner@, @extschema@, or @extschema:...@ appeared inside a quoting construct (dollar quoting, '', or ""). No bundled extension was vulnerable. Vulnerable uses do appear in a documentation example and in non-bundled extensions. Hence, the attack prerequisite was an administrator having installed files of a vulnerable, trusted, non-bundled extension. Subject to that prerequisite, this enabled an attacker having database-level CREATE privilege to execute arbitrary code as the bootstrap superuser. By blocking this attack in the core server, there's no need to modify individual extensions. Back-patch to v11 (all supported versions). Reported by Micah Gate, Valerie Woolard, Tim Carey-Smith, and Christoph Berg. Security: CVE-2023-39417
* Translation updatesPeter Eisentraut2023-08-07
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 1168da8e78b0511c8bfa99ceb08e848fbaa7e8f2
* Don't Memoize lateral joins with volatile join conditionsDavid Rowley2023-08-07
| | | | | | | | | | | | | | | | | | The use of Memoize was already disabled in normal joins when the join conditions had volatile functions per the code in match_opclause_to_indexcol(). Ordinarily, the parameterization for the inner side of a nested loop will be an Index Scan or at least eventually lead to an index scan (perhaps nested several joins deep). However, for lateral joins, that's not the case and seq scans can be parameterized too, so we can't rely on match_opclause_to_indexcol(). Here we explicitly check the parameterization for volatile functions and don't consider the generation of a Memoize path when such functions are present. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs49nHFnHbpepLsv_yF3qkpCS4BdB-v8HoJVv8_=Oat0u_w@mail.gmail.com Backpatch-through: 14, where Memoize was introduced
* Fix RLS policy usage in MERGE.Dean Rasheed2023-08-07
| | | | | | | | | | | | | | | | | | | | | | | | | If MERGE executes an UPDATE action on a table with row-level security, the code incorrectly applied the WITH CHECK clauses from the target table's INSERT policies to new rows, instead of the clauses from the table's UPDATE policies. In addition, it failed to check new rows against the target table's SELECT policies, if SELECT permissions were required (likely to always be the case). In addition, if MERGE executes a DO NOTHING action for matched rows, the code incorrectly applied the USING clauses from the target table's DELETE policies to existing target tuples. These policies were applied as checks that would throw an error, if they did not pass. Fix this, so that a MERGE UPDATE action applies the same RLS policies as a plain UPDATE query with a WHERE clause, and a DO NOTHING action does not apply any RLS checks (other than adding clauses from SELECT policies to the join). Back-patch to v15, where MERGE was introduced. Dean Rasheed, reviewed by Stephen Frost. Security: CVE-2023-39418
* Fix ReorderBufferCheckMemoryLimit() comment.Masahiko Sawada2023-08-02
| | | | | | | | | | Commit 7259736a6 updated the comment but it was not correct since ReorderBufferLargestStreamableTopTXN() returns only top-level transactions. Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAD21AoA9XB7OR86BqvrCe2dMYX%2BZv3-BvVmjF%3DGY2z6jN-kqjg%40mail.gmail.com Backpatch-through: 14
* Fix overly strict Assert in jsonpath codeDavid Rowley2023-08-02
| | | | | | | | | | | | | This was failing for queries which try to get the .type() of a jpiLikeRegex. For example: select jsonb_path_query('["string", "string"]', '($[0] like_regex ".{7}").type()'); Reported-by: Alexander Kozhemyakin Bug: #18035 Discussion: https://postgr.es/m/18035-64af5cdcb5adf2a9@postgresql.org Backpatch-through: 12, where SQL/JSON path was added.
* Disallow replacing joins with scans in problematic cases.Etsuro Fujita2023-07-28
| | | | | | | | | | | | | | | | | | | | | | | | Commit e7cb7ee14, which introduced the infrastructure for FDWs and custom scan providers to replace joins with scans, failed to add support handling of pseudoconstant quals assigned to replaced joins in createplan.c, leading to an incorrect plan without a gating Result node when postgres_fdw replaced a join with such a qual. To fix, we could add the support by 1) modifying the ForeignPath and CustomPath structs to store the list of RestrictInfo nodes to apply to the join, as in JoinPaths, if they represent foreign and custom scans replacing a join with a scan, and by 2) modifying create_scan_plan() in createplan.c to use that list in that case, instead of the baserestrictinfo list, to get pseudoconstant quals assigned to the join; but #1 would cause an ABI break. So fix by modifying the infrastructure to just disallow replacing joins with such quals. Back-patch to all supported branches. Reported by Nishant Sharma. Patch by me, reviewed by Nishant Sharma and Richard Guo. Discussion: https://postgr.es/m/CADrsxdbcN1vejBaf8a%2BQhrZY5PXL-04mCd4GDu6qm6FigDZd6Q%40mail.gmail.com
* Raise fixed token-length limit in hba.c.Tom Lane2023-07-27
| | | | | | | | | | | | | | Historically, hba.c limited tokens in the authentication configuration files (pg_hba.conf and pg_ident.conf) to less than 256 bytes. We have seen a few reports of this limit causing problems; notably, for moderately-complex LDAP configurations. Increase the limit to 10240 bytes as a low-risk stop-gap solution. In v13 and earlier, this also requires raising MAX_LINE, the limit on overall line length. I'm hesitant to make this code consume too much stack space, so I only raised that to 20480 bytes. Discussion: https://postgr.es/m/1588937.1690221208@sss.pgh.pa.us
* Fix the display of UNKNOWN message type in apply worker.Amit Kapila2023-07-25
| | | | | | | | | | | | | | We include the message type while displaying an error context in the apply worker. Now, while retrieving the message type string if the message type is unknown we throw an error that will hide the original error. So, instead, we need to simply return the string indicating an unknown message type. Reported-by: Ashutosh Bapat Author: Euler Taveira, Amit Kapila Reviewed-by: Ashutosh Bapat Backpatch-through: 15 Discussion: https://postgr.es/m/CAExHW5suAEDW-mBZt_qu4RVxWZ1vL54-L+ci2zreYWebpzxYsA@mail.gmail.com
* Guard against null plan pointer in CachedPlanIsSimplyValid().Tom Lane2023-07-20
| | | | | | | | | | | | | If both the passed-in plan pointer and plansource->gplan are NULL, CachedPlanIsSimplyValid would think that the plan pointer is possibly-valid and try to dereference it. For the one extant call site in plpgsql, this situation doesn't normally happen which is why we've not noticed. However, it appears to be possible if the previous use of the cached plan failed, as per report from Justin Pryzby. Add an extra check to prevent crashing. Back-patch to v13 where this code was added. Discussion: https://postgr.es/m/ZLlV+STFz1l/WhAQ@telsasoft.com
* Fix indentation in twophase.cMichael Paquier2023-07-18
| | | | | | | | | This has been missed in cb0cca1, noticed before buildfarm member koel has been able to complain while poking at a different patch. Like the other commit, backpatch all the way down to limit the odds of merge conflicts. Backpatch-through: 11
* Fix recovery of 2PC transaction during crash recoveryMichael Paquier2023-07-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A crash in the middle of a checkpoint with some two-phase state data already flushed to disk by this checkpoint could cause a follow-up crash recovery to recover twice the same transaction, once from what has been found in pg_twophase/ at the beginning of recovery and a second time when replaying its corresponding record. This would lead to FATAL failures in the startup process during recovery, where the same transaction would have a state recovered twice instead of once: LOG: recovering prepared transaction 731 from shared memory LOG: recovering prepared transaction 731 from shared memory FATAL: lock ExclusiveLock on object 731/0/0 is already held This issue is fixed by skipping the addition of any 2PC state coming from a record whose equivalent 2PC state file has already been loaded in TwoPhaseState at the beginning of recovery by restoreTwoPhaseData(), which is OK as long as the system has not reached a consistent state. The timing to get a messed up recovery processing is very racy, and would very unlikely happen. The thread that has reported the issue has demonstrated the bug using injection points to force a PANIC in the middle of a checkpoint. Issue introduced in 728bd99, so backpatch all the way down. Reported-by: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com> Author: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com> Author: Michael Paquier Discussion: https://postgr.es/m/109e6994-b971-48cb-84f6-829646f18b4c.mengjuan.cmj@alibaba-inc.com Backpatch-through: 11
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-07 03:46:37 +0000