Rel 2 stable cherry pick - #1904
Open
chenjinbao1989 wants to merge 1181 commits into
Open
Conversation
Per report from rsindlin Discussion: https://postgr.es/m/167907221210.1803488.5939223864945604536@wrigleys.postgresql.org
When probing the Memoize cache to check if the current cache key values exist in the cache, we perform an evaluation of the expressions making up the cache key before probing the hash table for those values. This operation could leak memory as it is possible that the cache key is an expression which requires allocation of memory, as was the case in bug 17844. Here we fix this by correctly switching to the per tuple context before evaluating the cache expressions so that the memory is freed next time the per tuple context is reset. Bug: 17844 Reported-by: Alexey Ermakov Discussion: https://postgr.es/m/17844-d2f6f9e75a622bed@postgresql.org Backpatch-through: 14, where Memoize was introduced
When calculating distance in brin_minmax_multi_distance_inet(), the netmask was applied incorrectly. This results in (seemingly) incorrect ordering of values, triggering an assert. For builds without asserts this is mostly harmless - we may merge other ranges, possibly resulting in slightly less efficient index. But it's still correct and the greedy algorithm doesn't guarantee optimality anyway. Backpatch to 14, where minmax-multi indexes were introduced. Reported by Dmitry Dolgov, investigation and fix by me. Reported-by: Dmitry Dolgov Backpatch-through: 14 Discussion: https://postgr.es/m/17774-c6f3e36dd4471e67@postgresql.org
With unlucky timing and parallel_leader_participation=off (not the default), PHJ could attempt to access per-batch shared state just as it was being freed. There was code intended to prevent that by checking for a cleared pointer, but it was racy. Fix, by introducing an extra barrier phase. The new phase PHJ_BUILD_RUNNING means that it's safe to access the per-batch state to find a batch to help with, and PHJ_BUILD_DONE means that it is too late. The last to detach will free the array of per-batch state as before, but now it will also atomically advance the phase, so that late attachers can avoid the hazard. This mirrors the way per-batch hash tables are freed (see phases PHJ_BATCH_PROBING and PHJ_BATCH_DONE). An earlier attempt to fix this (commit 3b8981b, later reverted) missed one special case. When the inner side is empty (the "empty inner optimization), the build barrier would only make it to PHJ_BUILD_HASHING_INNER phase before workers attempted to detach from the hashtable. In that case, fast-forward the build barrier to PHJ_BUILD_RUNNING before proceeding, so that our later assertions hold and we can still negotiate who is cleaning up. Revealed by build farm failures, where BarrierAttach() failed a sanity check assertion, because the memory had been clobbered by dsa_free(). In non-assert builds, the result could be a segmentation fault. Back-patch to all supported releases. Author: Thomas Munro <thomas.munro@gmail.com> Author: Melanie Plageman <melanieplageman@gmail.com> Reported-by: Michael Paquier <michael@paquier.xyz> Reported-by: David Geier <geidav.pg@gmail.com> Tested-by: David Geier <geidav.pg@gmail.com> Discussion: https://postgr.es/m/20200929061142.GA29096%40paquier.xyz
We fail to apply updates and deletes when the REPLICA IDENTITY FULL is used for the table having dropped columns. We didn't use to ignore dropped columns while doing tuple comparison among the tuples from the publisher and subscriber during apply of updates and deletes. Author: Onder Kalaci, Shi yu Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CACawEhVQC9WoofunvXg12aXtbqKnEgWxoRx3+v8q32AWYsdpGg@mail.gmail.com
This commit adds some documentation about two monitoring functions: - pg_stat_get_xact_blocks_fetched() - pg_stat_get_xact_blocks_hit() The description of these functions has been removed in ddfc2d9, later simplified by 5f2b089, assuming that all the functions whose descriptions were removed are used in system views. Unfortunately, some of them were are not used in any system views, so they lacked documentation. This gap exists in the docs for a long time, so backpatch all the way down. Reported-by: Michael Paquier Author: Bertrand Drouvot Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/ZBeeH5UoNkTPrwHO@paquier.xyz Backpatch-through: 11
We fail to apply updates and deletes when the REPLICA IDENTITY FULL is used for the table having generated columns. We didn't use to ignore generated columns while doing tuple comparison among the tuples from the publisher and subscriber during apply of updates and deletes. Author: Onder Kalaci Reviewed-by: Shi yu, Amit Kapila Backpatch-through: 12 Discussion: https://postgr.es/m/CACawEhVQC9WoofunvXg12aXtbqKnEgWxoRx3+v8q32AWYsdpGg@mail.gmail.com
In such cases, get_xid_status() doesn't set its output parameter (the third argument), so we shouldn't fall through to code which will test the value of that parameter. There are five existing calls to get_xid_status(), three of which seem to already handle this case properly. This commit tries to fix the other two. If we're checking xmin and find that it is invalid (i.e. 0) just report that as corruption, similar to what's already done in the three cases that seem correct. If we're checking xmax and find that's invalid, that's fine: it just means that the tuple hasn't been updated or deleted. Thanks to Andres Freund and valgrind for finding this problem, and also to Andres for having a look at the patch. This bug seems to go all the way back to where verify_heapam was first introduced, but wasn't detected until recently, possibly because of the new test cases added for update chain verification. Back-patch to v14, where this code showed up. Discussion: http://postgr.es/m/CA+TgmoZAYzQZqyUparXy_ks3OEOfLD9-bEXt8N-2tS1qghX9gQ@mail.gmail.com
The nested-arrays code path in ExecEvalArrayExpr() used palloc to allocate the result array, whereas every other array-creating function has used palloc0 since 18c0b4e. This mostly works, but unused bits past the end of the nulls bitmap may end up undefined. That causes valgrind complaints with -DWRITE_READ_PARSE_PLAN_TREES, and could cause planner misbehavior as cited in 18c0b4e. There seems no very good reason why we should strive to avoid palloc0 in just this one case, so fix it the easy way with s/palloc/palloc0/. While looking at that I noted that we also failed to check for overflow of "nbytes" and "nitems" while summing the sizes of the sub-arrays, potentially allowing a crash due to undersized output allocation. For "nbytes", follow the policy used by other array-munging code of checking for overflow after each addition. (As elsewhere, the last addition of the array's overhead space doesn't need an extra check, since palloc itself will catch a value between 1Gb and 2Gb.) For "nitems", there's no very good reason to sum the inputs at all, since we can perfectly well use ArrayGetNItems' result instead of ignoring it. Per discussion of this bug, also remove redundant zeroing of the nulls bitmap in array_set_element and array_set_slice. Patch by Alexander Lakhin and myself, per bug #17858 from Alexander Lakhin; thanks also to Richard Guo. These bugs are a dozen years old, so back-patch to all supported branches. Discussion: https://postgr.es/m/17858-8fd287fd3663d051@postgresql.org
find_composite_type_dependencies() ignored indexes, which is a poor decision because an expression index could have a stored column of a composite (or other container) type even when the underlying table does not. Teach it to detect such cases and error out. We have to work a bit harder than for other relations because the pg_depend entry won't identify the specific index column of concern, but it's not much new code. This does not address bug #17872's original complaint that dropping a column in such a type might lead to violations of the uniqueness property that a unique index is supposed to ensure. That seems of much less concern to me because it won't lead to crashes. Per bug #17872 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17872-d0fbb799dc3fd85d@postgresql.org
Homebrew changed the prefix for Apple Silicon based machines, so our advice for XML_CATALOG_FILES needs to mention both. More info on the Homebrew change can be found at: Homebrew/brew#9177 This is backpatch of commits 4c8d654 and 5a91c79, the latter which contained a small fix based on a report from Dagfinn Ilmari Mannsåker. Author: Julien Rouhaud <julien.rouhaud@free.fr> Discussion: https://postgr.es/m/20230327082441.h7pa2vqiobbyo7rd@jrouhaud
Commit e88754a caused that case to be reported as corruption, but Peter Geoghegan pointed out that it can legitimately happen in the case of a speculative insertion that aborts, so we'd better not flag it as corruption after all. Back-patch to v14, like the commit that introduced the issue. Discussion: http://postgr.es/m/CAH2-WzmEabzcPTxSY-NXKH6Qt3FkAPYHGQSe2PtvGgj17ZQkCw@mail.gmail.com
gistBuildCallback tried to fetch the size of an index tuple that might have already been freed by gistProcessEmptyingQueue. While this seems to usually be harmless in production builds, in principle it could result in a SIGSEGV, or more likely a bogus value for indtuplesSize leading to poor page-split decisions later in the build. The memory management here is confusing and could stand to be refactored, but for the moment it seems to be enough to fetch the tuple size sooner. AFAICT the indtuples[Size] totals aren't used in between these places; even if they were, the updated values shouldn't be any worse to use. So just move the incrementing of the totals up. It's not very clear why our valgrind-using buildfarm animals haven't noticed this problem, because the relevant code path does seem to be exercised according to the code coverage report. I think the reason that we didn't fix this bug after the first report is that I'd wanted to try to understand that better. However, now that it's been re-discovered let's just be pragmatic and fix it already. Original report by Alexander Lakhin (bug #16329), later rediscovered by Egor Chindyaskin (bug #17874). Patch by Alexander Lakhin (commentary by Pavel Borisov and me). Back-patch to all supported branches. Discussion: https://postgr.es/m/16329-7a6aa9b6fa1118a1@postgresql.org Discussion: https://postgr.es/m/17874-63ca6c7ce42d2103@postgresql.org
When calling generateSerialExtraStmts(), we would pass in the constraint->options. In some cases, generateSerialExtraStmts() would modify the referenced List to remove elements from it, but doing so is invalid without assigning the list back to all variables that point to it. In the particular reported problem case, the List became empty, in which cases it became NIL, but the passed in constraint->options didn't get to find out about that and was left pointing to free'd memory. To fix this, just perform a list_copy() inside generateSerialExtraStmts(). We could just do a list_copy() just before we perform the delete from the list, however, that seems less robust. Let's make sure the generated CreateSeqStmt gets a completely different copy of the list to be safe. Bug: #17879 Reported-by: Fei Changhong Diagnosed-by: Fei Changhong Discussion: https://postgr.es/m/17879-b7dfb5debee58ff5@postgresql.org Backpatch-through: 11, all supported versions
The totalrows/totaldeadrows outputs were left uninitialized in cases where we find no analyzable child tables of a partitioned table. This could lead to setting the partitioned table's pg_class.reltuples value to garbage. It's not clear that that would have any very bad effects in practice, but fix it anyway because it's making valgrind unhappy. Reported and diagnosed by Alexander Lakhin (bug #17880). Discussion: https://postgr.es/m/17880-9282037c923d856e@postgresql.org
Up through v11 it was sensible to use the "oid" system column as a foreign key column, but since that was removed there's no visible usefulness in making any of the remaining system columns a foreign key. Moreover, since the TupleTableSlot rewrites in v12, such cases actively fail because of implicit assumptions that only user columns appear in foreign keys. The lack of complaints about that seems like good evidence that no one is trying to do it. Hence, rather than trying to repair those assumptions (of which there are at least two, maybe more), let's just forbid the case up front. Per this patch, a system column in either the referenced or referencing side of a foreign key will draw this error; however, putting one in the referenced side would have failed later anyway, since we don't allow unique indexes to be made on system columns. Per bug #17877 from Alexander Lakhin. Back-patch to v12; the case still appears to work in v11, so we shouldn't break it there. Discussion: https://postgr.es/m/17877-4bcc658e33df6de1@postgresql.org
The explanation describing the dependency to system read() calls for these two functions has been removed in ddfc2d9. And after more discussion about d69c404, we have concluded that adding more details makes them easier to understand. While on it, use the term "block read requests" (maybe found in cache) rather than "buffers fetched" and "buffer hits". Per discussion with Melanie Plageman, Kyotaro Horiguchi, Bertrand Drouvot and myself. Discussion: https://postgr.es/m/CAAKRu_ZmdiScT4q83OAbfmR5AH-L5zWya3SXjaxiJvhCob-e2A@mail.gmail.com Backpatch-through: 11
Since 8b9e964, the messages for failed permissions checks report "table" where appropriate, rather than "relation". Backpatch to all supported branches
tsquery's GETQUERY() macro is only safe to apply to a tsquery that is known non-empty; otherwise it gives a pointer to garbage. Before commit 5a617d7, ts_headline() avoided this pitfall, but only in a very indirect, nonobvious way. (hlCover could not reach its TS_execute call, because if the query contains no lexemes then hlFirstIndex would surely return -1.) After that commit, it fell into the trap, resulting in weird errors such as "unrecognized operator" and/or valgrind complaints. In HEAD, fix this by not calling TS_execute_locations() at all for an empty query. In the back branches, add a defensive check to hlCover() --- that's not fixing any live bug, but I judge the code a bit too fragile as-is. Also, both mark_hl_fragments() and mark_hl_words() were careless about the possibility of empty search text: in the cases where no match has been found, they'd end up telling mark_fragment() to mark from word indexes 0 to 0 inclusive, even when there is no word 0. This is harmless since we over-allocated the prs->words array, but it does annoy valgrind. Fix so that the end index is -1 and thus mark_fragment() will do nothing in such cases. Bottom line is that this fixes a live bug in HEAD, but in the back branches it's only getting rid of a valgrind nitpick. Back-patch anyway. Per report from Alexander Lakhin. Discussion: https://postgr.es/m/c27f642d-020b-01ff-ae61-086af287c4fd@gmail.com
The tests added by commits 029dea8 et al turn out to produce different output under -DRANDOMIZE_ALLOCATED_MEMORY. This is not a bug exactly: that flag causes coerce_type() to invoke the input function twice when coercing an unknown-type literal to a specific type. So you get tsqueryin's bleat about an empty tsquery twice. Revise the test query to avoid that. Discussion: https://postgr.es/m/20230406213813.uep7plg6lvcywujo@awork3.anarazel.de
In our Kerberos test suite, there isn't much need to worry about the normal canonicalization that Kerberos provides by looking up the reverse DNS for the IP address connected to, and in some cases it can actively cause problems (eg: a captive portal wifi where the normally not resolvable localhost address used ends up being resolved anyway, and not to the domain we are using for testing, causing the entire regression test to fail with errors about not being able to get a TGT for the remote realm for cross-realm trust). Therefore, disable it by adding rdns = false into the krb5.conf that's generated for the test. Reviewed-By: Heikki Linnakangas Discussion: https://postgr.es/m/Y/QD2zDkDYQA1GQt@tamriel.snowman.net
Similar to 8dff2f2, this disables DNS lookups by the Kerberos library to look up the KDC and the realm while the Kerberos tests are running. In some environments, these lookups can take a long time and end up timing out and causing tests to fail. Further, since this isn't really our domain, we shouldn't be sending out these DNS requests during our tests.
EXTRACT(EPOCH), EXTRACT(SECOND), and some related cases print more trailing zeroes than they used to. This behavior change happened with commit a2da77c (Change return type of EXTRACT to numeric), and it was intentional according to the commit log: - Return values when extracting fields with possibly fractional values, such as second and epoch, now have the full scale that the value has internally (so, for example, '1.000000' instead of just '1'). It's been like that for two releases now, so while I suggested changing this back, it's probably better to adjust the documentation examples. Per bug #17866 from Евгений Жужнев. Back-patch to v14 where the change came in. Discussion: https://postgr.es/m/17866-18eb70095b1594e2@postgresql.org
The tables in "71.3. Extensibility" listing the support functions for bloom and minmax-multi opclasses should include the associated options function. While this isn't quite as required as the rest, you need it for full functionality of the opclass. Back-patch to v14 where these functions were added.
Calling fseek() or ftello() on a handle to a non-seeking device such as a pipe or a communications device is not supported. Unfortunately, MSVC's flavor of these routines, _fseeki64() and _ftelli64(), do not return an error when given a pipe as handle. Some of the logic of pg_dump and restore relies on these routines to check if a handle is seekable, causing failures when passing the contents of pg_dump to pg_restore through a pipe, for example. This commit introduces wrappers for fseeko() and ftello() on MSVC so as any callers are able to properly detect the cases of non-seekable handles. This relies mainly on GetFileType(), sharing a bit of code with the MSVC port for fstat(). The code in charge of getting a file type is refactored into a new file called win32common.c, shared by win32stat.c and the new win32fseek.c. It includes the MSVC ports for fseeko() and ftello(). Like 765f5df, this is backpatched down to 14, where the fstat() implementation for MSVC is able to understand about files larger than 4GB in size. Using a TAP test for that is proving to be tricky as IPC::Run handles the pipes by itself, still I have been able to check the fix manually. Reported-by: Daniel Watzinger Author: Juan José Santamaría Flecha, Michael Paquier Discussion: https://postgr.es/m/CAC+AXB26a4EmxM2suXxPpJaGrqAdxracd7hskLg-zxtPB50h7A@mail.gmail.com Backpatch-through: 14
Starting with OpenSSL 1.1.0 there is no need to call PQinitOpenSSL or PQinitSSL to avoid duplicate initialization of OpenSSL. Add a note to the documentation to explain this. Backpatch to all supported versions as older OpenSSL versions are equally likely to be used for all branches. Reported-by: Sebastien Flaesch <sebastien.flaesch@4js.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/DBAP191MB12895BFFEC4B5FE0460D0F2FB0459@DBAP191MB1289.EURP191.PROD.OUTLOOK.COM Backpatch-through: 11, all supported versions
If the last few pages in the specified range are empty (all zero), then log_newpage_range() could try to emit an empty WAL record containing no FPIs. This at least upsets an Assert in ReserveXLogInsertLocation, and might perhaps have bad real-world consequences in non-assert builds. This has been broken since log_newpage_range() was introduced, but the case was hard if not impossible to hit before commit 3d6a984 decided it was okay to leave VM and FSM pages intentionally zero. Nonetheless, it seems prudent to back-patch. log_newpage_range() was added in v12 but later back-patched, so this affects all supported branches. Matthias van de Meent, per report from Justin Pryzby Discussion: https://postgr.es/m/ZD1daibg4RF50IOj@telsasoft.com
When compiled with -C ORACLE, ecpg_get_data() had a one-off issue where it would incorrectly store the null terminator byte to str[-1] when varcharsize is 0, which is something that can happen when using SQLDA. This would eat 1 byte from the previous field stored, corrupting the results generated. All the callers of ecpg_get_data() estimate and allocate enough storage for the data received, and the fix of this commit relies on this assumption. Note that this maps to the case where no padding or truncation is required. This issue has been introduced by 3b7ab43 with the Oracle compatibility option, so backpatch down to v11. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20230410.173500.440060475837236886.horikyota.ntt@gmail.com Backpatch-through: 11
We've long used "--strip-unneeded" for shared libraries but plain "-x" for static libraries when stripping symbols with GNU strip. There doesn't seem to be any really good reason for that though, since --strip-unneeded produces smaller output (as "-x" alone does not remove debug symbols). Moreover it seems that llvm-strip, although it identifies as GNU strip, misbehaves when given "-x" for this purpose. It's unclear whether that's intentional or a bug in llvm-strip, but in any case it seems like changing to use --strip-unneeded in all cases should be a win. Note that this doesn't change our behavior when dealing with non-GNU strip. Per gripes from Ed Maste and Palle Girgensohn. Back-patch, in case anyone wants to use llvm-strip with stable branches. Discussion: https://postgr.es/m/17898-5308d09543463266@postgresql.org Discussion: https://postgr.es/m/20230420153338.bbj2g5jiyy3afhjz@awork3.anarazel.de
We need to call them only when validate == true. Backpatch to 13, where opclass options were introduced. Reported-by: Tom Lane Discussion: https://postgr.es/m/2656633.1681831542%40sss.pgh.pa.us Reviewed-by: Tom Lane, Pavel Borisov Backpatch-through: 13
When GPHOME is reached via a symlink whose target is stored as a relative path (e.g. /opt/database -> database-2.1.0), the previous logic ran a bare `readlink` on the script's directory and accepted the link target verbatim. `readlink` returns the symlink's target string as-is; for a relative target this leaves GPHOME as a relative path, so PYTHONPATH becomes `database-2.1.0/lib/python` and gpstart fails with ModuleNotFoundError: No module named 'gppylib' the moment the shell's cwd is anything other than the symlink's parent. Replace the symlink-vs-not branch with a single `pwd -P`, which resolves every symlink component and returns the canonical absolute path regardless of how the user reached the directory. This also removes the unnecessary `[ -L ... ]` test and the dependency on GNU readlink semantics.
…buffer CException is ~255KB due to stack_[DEFAULT_STACK_MAX_SIZE]. When Raise(CException ex, ...) passes by value, two copies (510KB) are placed on the stack, causing stack overflow in backtrace() -> dlopen(). Fix: - Change Raise/ReRaise to pass CException by reference instead of value - Reduce DEFAULT_STACK_MAX_SIZE to 1/4 (255KB -> 63KB) - Callers now construct a local CException and pass it by reference
This omission allowed table owners to create statistics in any schema, potentially leading to unexpected naming conflicts. For ALTER TABLE commands that require re-creating statistics objects, skip this check in case the user has since lost CREATE on the schema. The addition of a second parameter to CreateStatistics() breaks ABI compatibility, but we are unaware of any impacted third-party code. Reported-by: Jelte Fennema-Nio <postgres@jeltef.nl> Author: Jelte Fennema-Nio <postgres@jeltef.nl> Co-authored-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Security: CVE-2025-12817 Backpatch-through: 13
Several functions could overflow their size calculations, when presented with very large inputs from remote and/or untrusted locations, and then allocate buffers that were too small to hold the intended contents. Switch from int to size_t where appropriate, and check for overflow conditions when the inputs could have plausibly originated outside of the libpq trust boundary. (Overflows from within the trust boundary are still possible, but these will be fixed separately.) A version of add_size() is ported from the backend to assist with code that performs more complicated concatenation. Reported-by: Aleksey Solovev (Positive Technologies) Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Security: CVE-2025-12818 Backpatch-through: 13
pgp_pub_decrypt_bytea() was missing a safeguard for the session key length read from the message data, that can be given in input of pgp_pub_decrypt_bytea(). This can result in the possibility of a buffer overflow for the session key data, when the length specified is longer than PGP_MAX_KEY, which is the maximum size of the buffer where the session data is copied to. A script able to rebuild the message and key data that can trigger the overflow is included in this commit, based on some contents provided by the reporter, heavily editted by me. A SQL test is added, based on the data generated by the script. Reported-by: Team Xint Code as part of zeroday.cloud Author: Michael Paquier <michael@paquier.xyz> Reviewed-by: Noah Misch <noah@leadboat.com> Security: CVE-2026-2005 Backpatch-through: 14
While EUC_CN supports only 1- and 2-byte sequences (CS0, CS1), the mb<->wchar conversion functions allow 3-byte sequences beginning SS2, SS3. Change pg_encoding_max_length() to return 3, not 2, to close a hypothesized buffer overrun if a corrupted string is converted to wchar and back again in a newly allocated buffer. We might reconsider that in master (ie harmonizing in a different direction), but this change seems better for the back-branches. Also change pg_euccn_mblen() to report SS2 and SS3 characters as having length 3 (following the example of EUC_KR). Even though such characters would not pass verification, it's remotely possible that invalid bytes could be used to compute a buffer size for use in wchar conversion. Security: CVE-2026-2006 Backpatch-through: 14 Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
When converting multibyte to pg_wchar, the UTF-8 implementation would silently ignore an incomplete final character, while the other implementations would cast a single byte to pg_wchar, and then repeat for the remaining byte sequence. While it didn't overrun the buffer, it was surely garbage output. Make all encodings behave like the UTF-8 implementation. A later change for master only will convert this to an error, but we choose not to back-patch that behavior change on the off-chance that someone is relying on the existing UTF-8 behavior. Security: CVE-2026-2006 Backpatch-through: 14 Author: Thomas Munro <thomas.munro@gmail.com> Reported-by: Noah Misch <noah@leadboat.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
These data types are represented like full-fledged arrays, but functions that deal specifically with these types assume that the array is 1-dimensional and contains no nulls. However, there are cast pathways that allow general oid[] or int2[] arrays to be cast to these types, allowing these expectations to be violated. This can be exploited to cause server memory disclosure or SIGSEGV. Fix by installing explicit checks in functions that accept these types. Reported-by: Altan Birler <altan.birler@tum.de> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Security: CVE-2026-2003 Backpatch-through: 14
An upcoming patch requires this cache so that it can track updates in the pg_extension catalog. So far though, the EXTENSIONOID cache only exists in v18 and up (see 490f869). We can add it in older branches without an ABI break, if we are careful not to disturb the numbering of existing syscache IDs. In v16 and before, that just requires adding the new ID at the end of the hand-assigned enum list, ignoring our convention about alphabetizing the IDs. But in v17, genbki.pl enforces alphabetical order of the IDs listed in MAKE_SYSCACHE macros. We can fake it out by calling the new cache ZEXTENSIONOID. Note that adding a syscache does change the required contents of the relcache init file (pg_internal.init). But that isn't problematic since we blow those away at postmaster start for other reasons. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Security: CVE-2026-2004 Backpatch-through: 14-17
Selectivity estimators come in two flavors: those that make specific assumptions about the data types they are working with, and those that don't. Most of the built-in estimators are of the latter kind and are meant to be safely attachable to any operator. If the operator does not behave as the estimator expects, you might get a poor estimate, but it won't crash. However, estimators that do make datatype assumptions can malfunction if they are attached to the wrong operator, since then the data they get from pg_statistic may not be of the type they expect. This can rise to the level of a security problem, even permitting arbitrary code execution by a user who has the ability to create SQL objects. To close this hole, establish a rule that built-in estimators are required to protect themselves against being called on the wrong type of data. It does not seem practical however to expect estimators in extensions to reach a similar level of security, at least not in the near term. Therefore, also establish a rule that superuser privilege is required to attach a non-built-in estimator to an operator. We expect that this restriction will have little negative impact on extensions, since estimators generally have to be written in C and thus superuser privilege is required to create them in the first place. This commit changes the privilege checks in CREATE/ALTER OPERATOR to enforce the rule about superuser privilege, and fixes a couple of built-in estimators that were making datatype assumptions without sufficiently checking that they're valid. Reported-by: Daniel Firer as part of zeroday.cloud Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Security: CVE-2026-2004 Backpatch-through: 14
While the preceding commit prevented such attachments from occurring in future, this one aims to prevent further abuse of any already- created operator that exposes _int_matchsel to the wrong data types. (No other contrib module has a vulnerable selectivity estimator.) We need only check that the Const we've found in the query is indeed of the type we expect (query_int), but there's a difficulty: as an extension type, query_int doesn't have a fixed OID that we could hard-code into the estimator. Therefore, the bulk of this patch consists of infrastructure to let an extension function securely look up the OID of a datatype belonging to the same extension. (Extension authors have requested such functionality before, so we anticipate that this code will have additional non-security uses, and may soon be extended to allow looking up other kinds of SQL objects.) This is done by first finding the extension that owns the calling function (there can be only one), and then thumbing through the objects owned by that extension to find a type that has the desired name. This is relatively expensive, especially for large extensions, so a simple cache is put in front of these lookups. Reported-by: Daniel Firer as part of zeroday.cloud Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Security: CVE-2026-2004 Backpatch-through: 14
RAT check expection added for PG16 section as it is effitinely comes from PG16 backport
liburing is the Linux-only userspace API for the io_uring kernel
interface (Linux 5.1+). It is not available on macOS or *BSD.
PAX already has a sync fallback path (SyncFastIO using pread(2)) and
LocalFile::ReadBatch picks between the two via IOUringFastIO::available().
This commit lets the build proceed without liburing:
* configure: AC_CHECK_LIB(uring) no longer aborts when missing; PAX
falls back to SyncFastIO at runtime.
* fast_io.h / fast_io.cc: wrap the IOUringFastIO class declaration
and methods with #ifdef __linux__ so they only exist where the
header is available.
* local_file_system.cc: gate the IOUringFastIO::available() branch
with #ifdef __linux__; non-Linux unconditionally uses SyncFastIO.
ic_udpifc.c uses the HZ macro (kernel timer frequency) for RTO calculations. On Linux glibc, HZ is exposed via <sys/param.h> (typically 100). macOS's <sys/param.h> does not define HZ. Define HZ=100 as a fallback when the platform headers don't provide it. The exact value only affects retransmit pacing constants (UDP_RTO_MIN, TIME_TICK); 100 matches the Linux default.
src/backend/gporca/gporca.mk forced -Werror -Wextra -Wpedantic on top of the project CXXFLAGS via 'override CXXFLAGS := ...'. clang reports many more warnings than gcc under -Wextra/-Wpedantic, which then become errors and break the macOS build (unused-but-set-variable, inconsistent-missing-override, ...). Keep -fno-omit-frame-pointer (used by ORCA's backtracing); drop the strict warning flags. The base project CXXFLAGS still includes -Wall and per-feature -Werror= flags.
With --enable-shared-postgres-backend (default) the libpostgres.so recipe filters out main/main.o, but other backend objects still reference symbols defined in main.o (progname, etc.). On Linux the default linker behaviour permits undefined references in a shared library; on macOS, ld -dynamiclib rejects them with 'Undefined symbols ... _progname'. Pass -Wl,-undefined,dynamic_lookup only when PORTNAME=darwin so those symbols resolve at load time against the postgres executable that loads libpostgres.so. The Linux behaviour is unchanged.
Several gcc / GNU-ld specific bits in the PAX cmake setup break under
Apple clang and Apple ld. Make them conditional on Linux.
* CMakeLists.txt: gate -no-pie / -Wl,--allow-multiple-definition /
-fno-access-control / -Wno-pmf-conversions (gcc-only) behind
if(NOT APPLE). Replace gcc-only warning disables (-Wno-clobbered,
-Wno-sized-deallocation, -Wno-parameter-name) with
-Wno-unknown-warning-option on APPLE, plus a set of
-Wno-error= demotions for clang-stricter diagnostics
(inconsistent-missing-override, overloaded-virtual,
sometimes-uninitialized, unused-private-field, format,
mismatched-tags, pessimizing-move, unused-but-set-variable,
deprecated-copy, unused-result).
* Makefile: pass -DBUILD_GTEST=OFF to the inner cmake (googletest's
own flags clash with clang). Drop SHLIB_LINK += -luuid on darwin
(uuid_* is in libSystem). Move the ifneq to after the include of
Makefile.global so PORTNAME is actually defined. Cope with cmake
naming the artefact libpax.so on every platform now (see below).
* src/cpp/CMakeLists.txt: skip the standalone libpaxformat target on
macOS. It links to backend symbols (write_stderr, ...) directly
and isn't needed to load PAX inside postgres.
* pax.cmake: on APPLE, build pax as a MODULE (Mach-O bundle, what
PG extensions are) instead of SHARED, and link with
-Wl,-undefined,dynamic_lookup -Wl,-bundle_loader,<postgres>. This
is the standard PG extension pattern; it guarantees that backend
globals (e.g. process_shared_preload_libraries_in_progress) have
one shared instance between postgres and pax.so. Also gate -luring
on Linux; pull abseil deps via pkg-config on macOS (Homebrew's
protobuf v22+ split them into separate libs); replace the
Linux-only $ORIGIN INSTALL_RPATH and -Wl,--enable-new-dtags with
@loader_path on macOS.
* pax_format.cmake: same Linux-only treatment for -luuid / -luring
and the abseil pkg-config.
Compile-time fixes to let PAX build with Apple clang against
Homebrew's modern protobuf (v22+).
* file_system.h: typedef off64_t = off_t on macOS. glibc exposes
off64_t for 32-bit programs opting into 64-bit file offsets;
macOS's off_t is already 64-bit so there is no separate symbol.
* pax_encoding_utils.{h,cc}: change BuildHistogram/ZigZagBuffers
signatures from int64_t* to PG's int64* (long). On macOS x86_64
int64_t is 'long long', distinct from 'long' for overload
resolution even though both are 64-bit. Using PG's int64
(consistently 'long' on every supported port) keeps callers from
PG-typed buffers (e.g. DataBuffer<int64>::StartT) working on both
platforms.
* pax_delta_encoding.cc: replace 'uint8_t bit_widths[var] = {0};'
(a gcc VLA-with-initializer extension that clang rejects) with
std::vector<uint8_t>.
* proto_wrappers.h: PG's c.h defines Min/Max macros and
xlog_internal.h defines IsPowerOf2; abseil (pulled in by modern
protobuf headers) declares identifiers with the same names.
#undef them around the protobuf include, then restore PG's
definitions afterwards.
* protobuf_stream.{h,cc}: protobuf v22 removed the
google::protobuf::int64 typedef. Use int64_t directly (which is
the type of the base ZeroCopy{Output,Input}Stream::ByteCount()
override anyway).
paxformat is the standalone PAX file reader meant to be linked by external tools. The previous commit 'macOS: PAX cmake portability' skipped it on APPLE because it references PG backend functions (write_stderr, xlog_check_consistency_hook, ...) that have no libpostgres.so to satisfy them on macOS. Build it after all by deferring those undefined symbols to load time with -Wl,-undefined,dynamic_lookup, just like Linux's default ld behaviour for shared libraries. The smoke-test executable paxformat_test uses the same flag and drops the explicit 'postgres' link library (there is no libpostgres.so to link against on macOS). paxformat is a regular dylib (SHARED) here — not a bundle — because the test executable links against it at link time.
After this commit, plain 'make' (without any CUSTOM_COPT= on the
command line) builds cleanly on macOS with Apple clang. Previously
users had to remember a long override.
The warning categories that needed demoting (darwin only):
-Wuninitialized — clang flags spots upstream gcc accepts.
-Wgnu-variable-sized-type-not-at-end
— clang-only; fires on PG catalog headers
like pg_task.h with inline struct-plus-
trailing-text declarations.
-Wunused-function — clang flags static functions never
referenced in the TU (a few exist in
currently-disabled code paths, e.g.
ic_udpifc.c).
-Wdeprecated-non-prototype
— clang-only; flags K&R-style 'foo()'
forward declarations. Where the
mismatch is a real bug we fix it
inline; this demotion covers the rest.
The block is gated to PORTNAME=darwin so the Linux gcc build path
is unchanged: -Werror remains in effect for all categories there.
The forward declaration at line 838 was
static void initSndBufferPool();
which under C's old K&R rules means 'function with unspecified
arguments' — but the actual definition takes a SendBufferPool *:
static void
initSndBufferPool(SendBufferPool *p)
and the single caller at line 3677 passes &snd_buffer_pool. gcc
silently accepts the mismatch; Apple clang correctly rejects it
under -Wdeprecated-non-prototype. C2x will reject it on every
compiler. Match the forward declaration to the definition.
Two related changes that together make PAX aux toasting match the
rest of the Cloudberry / Postgres tree:
1. src/backend/catalog/toasting.c
Drop the Cloudberry-only branch that routed TOAST for
pg_ext_aux parents back into pg_ext_aux:
else if (IsExtAuxNamespace(rel->rd_rel->relnamespace))
namespaceid = PG_EXTAUX_NAMESPACE;
PAX aux tables (pg_pax_blocks_<oid>) now get a normal TOAST
companion in pg_toast, the same as every other heap.
2. contrib/pax_storage/.../pax_aux_table.cc
Aux inherits the parent's persistence for PERMANENT / UNLOGGED
verbatim, but TEMP is clamped down to PERMANENT. Background:
the aux always lives in pg_ext_aux (not in pg_temp_<N>), so a
TEMP-persistence row in pg_ext_aux ends up mis-classified by
RELATION_IS_OTHER_TEMP — relcache.c sets rd_islocaltemp=false
because pg_ext_aux is not a temp namespace, and then
reindex_index() (or any catalog walk that touches the aux)
bails with "cannot reindex temporary tables of other
sessions". Clamping TEMP→PERMANENT avoids the mis-trigger;
the trade-off is that the aux of a TEMP PAX table outlives
the session, which is acceptable given the long-standing
FIXME in the same file ("temporary table in aux namespace is
not supported yet").
Resulting layout:
PERMANENT pax_tab aux in pg_ext_aux (p)
toast in pg_toast (p)
idx in pg_toast (p)
UNLOGGED u_pax aux in pg_ext_aux (u)
toast in pg_toast (u)
idx in pg_toast (u)
TEMP pax_tmp parent in pg_temp_<N> (t)
aux in pg_ext_aux (p) <-- clamped
toast in pg_toast (p)
idx in pg_toast (p)
Verified:
- CREATE TABLE / UNLOGGED / TEMP USING pax all produce the
layout above.
- INSERT round-trip works for all three persistence modes.
Add a gpcontrib debug extension that rejects full scans on partitioned ables when partition pruning is ineffective. The extension is built only with enable_debug_extensions=yes and supports Planner and ORCA plans, including Append, MergeAppend, PartitionSelector, and DynamicScan cases.
When a PAX table has minmax_columns that also support SUM statistics (e.g. int, bigint, numeric), repeated DELETE operations can crash a segment with SIGSEGV inside datumCopy() during the visibility-map statistics refresh path. Two bugs were identified in MicroPartitionStats::MergeRawInfo() and MergeTo(): 1. Wrong type metadata in FromValue(): The serialized SUM value was deserialized using the column physical type (typlen/typbyval) instead of the SUM aggregate return type (rettyplen/rettypbyval). For example, sum(bigint) returns numeric, so the stored SUM datum must be interpreted as numeric, not as int8. Using the wrong type metadata produces an invalid Datum that may crash in datumCopy(). 2. Swapped arguments in datumCopy(): The call passed (value, typlen, typbyval) but the wrapper signature is datumCopy(value, typByVal, typLen). This caused pass-by-value vs pass-by-reference confusion, leading to memory corruption. Add regression test (delete_sum_stats) covering DELETE + INSERT + DELETE on int, bigint, and numeric columns with minmax_columns enabled. Fixes apache#1767
With PAX relations now treated as AO-like in get_index_paths() (see preceding commit), a btree index over a PAX table no longer produces Index Scan / Index Only Scan plans -- only Bitmap Heap/Index Scan (or Seq Scan) paths survive. Regenerate the affected PAX regression expected outputs to match. Validated by running the PAX regress suite (parallel_schedule + greenplum_schedule) with default_table_access_method=pax; the refreshed files reflect the actual planner output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions