aboutsummaryrefslogtreecommitdiff
path: root/contrib
Commit message (Collapse)AuthorAge
* Backport BackgroundPsql perl test moduleHeikki Linnakangas2024-06-27
| | | | | | | | | | | | | | | Backport the new BackgroundPsql modules and the constructor functions, background_psql() and interactive_psql, to all supported branches. That makes it easier to backpatch tests that use it. BackgroundPsql was introduced in version 16. On version 16, this commit backports just the new timeout argument from master (commit 334f512f45). On older branches, the whole facility. This includes the change to `use warnings FATAL => 'all'`, which we haven't otherwise backported, but it seems good to keep the file identical across branches. Discussion: https://www.postgresql.org/message-id/b7c64f20-ea01-4f15-9088-0cd6832af149@iki.fi
* Make postgres_fdw request remote time zone 'GMT' not 'UTC'.Tom Lane2024-04-21
| | | | | | | | | | | | | | | | | | This should have the same results for all practical purposes. The advantage of selecting 'GMT' is that it's guaranteed to work even when the remote system's timezone database is missing entries, because pg_tzset() hard-wires handling of that, at least in 9.2 and later. (It seems like it would be a good idea to similarly hard-wire correct handling of 'UTC', but that'll be a little more invasive than I want to consider back-patching. Leave that for another day when we're not in feature freeze.) Per trouble report from Adnan Dautovic. Back-patch to all supported branches. Discussion: https://postgr.es/m/465248.1712211585@sss.pgh.pa.us
* xml2: Replace deprecated routines with recommended onesMichael Paquier2024-04-16
| | | | | | | | | | | | | | | | | | | | | | | | | Some functions are used in the tree and are currently marked as deprecated by upstream. This commit refreshes the code to use the recommended functions, leading to the following changes: - xmlSubstituteEntitiesDefault() is gone, and needs to be replaced with XML_PARSE_NOENT for the paths doing the parsing. - xmlParseMemory() -> xmlReadMemory(). These functions, as well as more functions setting global states, have been officially marked as deprecated by upstream in August 2022. Their replacements exist since the 2001-ish area, as far as I have checked, so that should be safe. This has been originally applied as 65c5864d7fac without a backpatch, and this has come up as well when working on 400928b83. Per request from Tom Lane, for new buildfarm member indri that is able to see deprecation warnings with xmlSubstituteEntitiesDefault() in 16 and older stable branches. Author: Dmitry Koval Discussion: https://postgr.es/m/18274-98d16bc03520665f@postgresql.org Discussion: https://postgr.es/m/1012981.1713222862@sss.pgh.pa.us Bakpatch-through: 12
* amcheck: Normalize index tuples containing uncompressed varlenaAlexander Korotkov2024-03-24
| | | | | | | | | | | | | It might happen that the varlena value wasn't compressed by index_form_tuple() due to current storage parameters. If compression is currently enabled, we need to compress such values to match index tuple coming from the heap. Backpatch to all supported versions. Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru Author: Andrey Borodin Reviewed-by: Alexander Lakhin, Michael Zhilin, Jian He, Alexander Korotkov Backpatch-through: 12
* amcheck: Support for different header sizes of short varlena datumAlexander Korotkov2024-03-23
| | | | | | | | | | | | | In the heap, tuples may contain short varlena datum with both 1B header and 4B headers. But the corresponding index tuple should always have such varlena's with 1B headers. So, for fingerprinting, we need to convert. Backpatch to all supported versions. Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru Author: Michael Zhilin Reviewed-by: Alexander Lakhin, Andrey Borodin, Jian He, Alexander Korotkov Backpatch-through: 12
* Fix 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
* pgcrypto: Fix check for buffer sizeDaniel Gustafsson2024-01-30
| | | | | | | | | | | | | | The code copying the PGP block into the temp buffer failed to account for the extra 2 bytes in the buffer which are needed for the prefix. If the block was oversized, subsequent checks of the prefix would have exceeded the buffer size. Since the block sizes are hardcoded in the list of supported ciphers it can be verified that there is no live bug here. Backpatch all the way for consistency though, as this bug is old. Author: Mikhail Gribkov <youzhick@gmail.com> Discussion: https://postgr.es/m/CAMEv5_uWvcMCMdRFDsJLz2Q8g16HEa9xWyfrkr+FYMMFJhawOw@mail.gmail.com Backpatch-through: v12
* Fix incompatibilities with libxml2 >= 2.12.0.Tom Lane2024-01-29
| | | | | | | | | | | | | | | | | | | | | | libxml2 changed the required signature of error handler callbacks to make the passed xmlError struct "const". This is causing build failures on buildfarm member caiman, and no doubt will start showing up in the field quite soon. Add a version check to adjust the declaration of xml_errorHandler() according to LIBXML_VERSION. 2.12.x also produces deprecation warnings for contrib/xml2/xpath.c's assignment to xmlLoadExtDtdDefaultValue. I see no good reason for that to still be there, seeing that we disabled external DTDs (at a lower level) years ago for security reasons. Let's just remove it. Back-patch to all supported branches, since they might all get built with newer libxml2 once it gets a bit more popular. (The back branches produce another deprecation warning about xpath.c's use of xmlSubstituteEntitiesDefault(). We ought to consider whether to back-patch all or part of commit 65c5864d7 to silence that. It's less urgent though, since it won't break the buildfarm.) Discussion: https://postgr.es/m/1389505.1706382262@sss.pgh.pa.us
* Fix integer-overflow problem in intarray's g_int_decompress().Tom Lane2024-01-07
| | | | | | | | | | An array element equal to INT_MAX gave this code indigestion, causing an infinite loop that surely ended in SIGSEGV. We fixed some nearby problems awhile ago (cf 757c5182f) but missed this. Report and diagnosis by Alexander Lakhin (bug #18273); patch by me Discussion: https://postgr.es/m/18273-9a832d1da122600c@postgresql.org
* pageinspect: Fix failure with hash_bitmap_info() for partitioned indexesMichael Paquier2023-12-19
| | | | | | | | | | | | | This function reads directly a page from a relation, relying on index_open() to open the index to read from. Unfortunately, this would crash when using partitioned indexes, as these can be opened with index_open() but they have no physical pages. Alexander has fixed the module, while I have written the test. Author: Alexander Lakhin, Michael Paquier Discussion: https://postgr.es/m/18246-f4d9ff7cb3af77e6@postgresql.org Backpatch-through: 12
* pgstattuple: Fix failure with pgstathashindex() for partitioned indexesMichael Paquier2023-12-19
| | | | | | | | | | | | | | | | As coded, the function relied on index_open() when opening an index relation, allowing partitioned indexes to be processed by pgstathashindex(). This was leading to a "could not open file" error because partitioned indexes have no physical files, or to a crash with an assertion failure (like on HEAD). This issue is fixed by applying the same checks as the other stat functions for indexes, with a lookup at both RELKIND_INDEX and the index AM expected. Author: Alexander Lakhin Discussion: https://postgr.es/m/18246-f4d9ff7cb3af77e6@postgresql.org Backpatch-through: 12
* Adjust the order of the prechecks in pgrowlocks()David Rowley2023-10-31
| | | | | | | | | | | | | | | | | | 4b8266415 added a precheck to pgrowlocks() to ensure the given object's pg_class.relam is HEAP_TABLE_AM_OID, however, that check was put before another check which was checking if the given object was a partitioned table. Since the pg_class.relam is always InvalidOid for partitioned tables, if pgrowlocks() was called passing a partitioned table, then the "only heap AM is supported" error would be raised instead of the intended error about the given object being a partitioned table. Here we simply move the pg_class.relam check to after the check that verifies that we are in fact working with a normal (non-partitioned) table. Reported-by: jian he Discussion: https://postgr.es/m/CACJufxFaSp_WguFCf0X98951zFVX+dXFnF1mxAb-G3g1HiHOow@mail.gmail.com Backpatch-through: 12, where 4b8266415 was introduced.
* Diagnose !indisvalid in more SQL functions.Noah Misch2023-10-30
| | | | | | | | | | | | | pgstatindex failed with ERRCODE_DATA_CORRUPTED, of the "can't-happen" class XX. The other functions succeeded on an empty index; they might have malfunctioned if the failed index build left torn I/O or other complex state. Report an ERROR in statistics functions pgstatindex, pgstatginindex, pgstathashindex, and pgstattuple. Report DEBUG1 and skip all index I/O in maintenance functions brin_desummarize_range, brin_summarize_new_values, brin_summarize_range, and gin_clean_pending_list. Back-patch to v11 (all supported versions). Discussion: https://postgr.es/m/20231001195309.a3@google.com
* amcheck: Distinguish interrupted page deletion from corruption.Noah Misch2023-10-30
| | | | | | | | | | | | | This prevents false-positive reports about "the first child of leftmost target page is not leftmost of its level", "block %u is not leftmost" and "left link/right link pair". They appeared if amcheck ran before VACUUM cleaned things, after a cluster exited recovery between the first-stage and second-stage WAL records of a deletion. Back-patch to v11 (all supported versions). Reviewed by Peter Geoghegan. Discussion: https://postgr.es/m/20231005025232.c7.nmisch@google.com
* btree_gin: Fix calculation of leftmost interval value.Dean Rasheed2023-10-29
| | | | | | | | | | | | | | | | | | Formerly, the value computed by leftmostvalue_interval() was a long way short of the minimum possible interval value. As a result, an index scan on a GIN index on an interval column with < or <= operators would miss large negative interval values. Fix by setting all fields of the leftmost interval to their minimum values, ensuring that the result is less than any other possible interval. Since this only affects index searches, no index rebuild is necessary. Back-patch to all supported branches. Dean Rasheed, reviewed by Heikki Linnakangas. Discussion: https://postgr.es/m/CAEZATCV80%2BgOfF8ehNUUfaKBZgZMDfCfL-g1HhWGb6kC3rpDfw%40mail.gmail.com
* postgres_fdw: Fix test for parameterized foreign scan.Etsuro Fujita2023-08-30
| | | | | | | | | Commit e4106b252 should have updated this test, but did not; back-patch to all supported branches. Reviewed by Richard Guo. Discussion: http://postgr.es/m/CAPmGK15nR0NXLSCKQAcqbZbTzrzd5MozowWnTnGfPkayndF43Q%40mail.gmail.com
* 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
* Remove unnecessary pfree() in g_intbig_compress().Tom Lane2023-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GiST compress functions (like all GiST opclass functions) are supposed to be called in short-lived memory contexts, so that minor memory leaks in them are not of concern, and indeed explicit pfree's are likely slightly counterproductive. But this one in g_intbig_compress() is more than slightly counterproductive, because it's guarded by "if (in != DatumGetArrayTypeP(entry->key))" which means that if this test succeeds, we've detoasted the datum twice. (And to add insult to injury, the extra detoast result is leaked.) Let's just drop the whole stanza, relying on the GiST temporary context mechanism to clean up in good time. The analogous bit in g_int_compress() is if (r != (ArrayType *) DatumGetPointer(entry->key)) pfree(r); which doesn't have the gratuitous-detoast problem so I left it alone. Perhaps there is a case for removing unnecessary pfree's more widely, but I'm not sure if it's worth the code churn. The potential extra decompress seems expensive enough to justify calling this a (minor) performance bug and back-patching. Konstantin Knizhnik, Matthias van de Meent, Tom Lane Discussion: https://postgr.es/m/CAEze2Wi86=DxErfvf+SCB2UKmU2amKOF60BKuJOX=w-RojRn0A@mail.gmail.com
* intarray: Prevent out-of-bound memory reads with gist__int_opsMichael Paquier2023-06-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As gist__int_ops stands in intarray, it is possible to store GiST entries for leaf pages that can cause corruptions when decompressed. Leaf nodes are stored as decompressed all the time by the compression method, and the decompression method should map with that, retrieving the contents of the page without doing any decompression. However, the code authorized the insertion of leaf page data with a higher number of array items than what can be supported, generating a NOTICE message to inform about this matter (199 for a 8k page, for reference). When calling the decompression method, a decompression would be attempted on this leaf node item but the contents should be retrieved as they are. The NOTICE message generated when dealing with the compression of a leaf page and too many elements in the input array for gist__int_ops has been introduced by 08ee64e, removing the marker stored in the array to track if this is actually a leaf node. However, it also missed the fact that the decompression path should do nothing for a leaf page. Hence, as the code stand, a too-large array would be stored as uncompressed but the decompression path would attempt a decompression rather that retrieving the contents as they are. This leads to various problems. First, even if 08ee64e tried to address that, it is possible to do out-of-bound chunk writes with a large input array, with the backend informing about that with WARNINGs. On decompression, retrieving the stored leaf data would lead to incorrect memory reads, leading to crashes or even worse. Perhaps somebody would be interested in expanding the number of array items that can be handled in a leaf page for this operator in the future, which would require revisiting the choice done in 08ee64e, but based on the lack of reports about this problem since 2005 it does not look so. For now, this commit prevents the insertion of data for leaf pages when using more array items that the code can handle on decompression, switching the NOTICE message to an ERROR. If one wishes to use more array items, gist__intbig_ops is an optional choice. While on it, use ERRCODE_PROGRAM_LIMIT_EXCEEDED as error code when a limit is reached, because that's what the module is facing in such cases. Author: Ankit Kumar Pandey, Alexander Lakhin Reviewed-by: Richard Guo, Michael Paquier Discussion: https://postgr.es/m/796b65c3-57b7-bddf-b0d5-a8afafb8b627@gmail.com Discussion: https://postgr.es/m/17888-f72930e6b5ce8c14@postgresql.org Backpatch-through: 11
* hstore: Tighten key/value parsing check for whitespacesMichael Paquier2023-06-12
| | | | | | | | | | | | | | | | | isspace() can be locale-sensitive depending on the platform, causing hstore to consider as whitespaces characters it should not see as such. For example, U+0105, being decoded as 0xC4 0x85 in UTF-8, would be discarded from the input given. This problem is similar to 9ae2661, though it was missed that hstore can also manipulate non-ASCII inputs, so replace the existing isspace() calls with scanner_isspace(). This problem exists for a long time, so backpatch all the way down. Author: Evan Jones Discussion: https://postgr.es/m/CA+HWA9awUW0+RV_gO9r1ABZwGoZxPztcJxPy8vMFSTbTfi4jig@mail.gmail.com Backpatch-through: 11
* Ensure Soundex difference() function handles empty input sanely.Tom Lane2023-05-16
| | | | | | | | | | | | | | | | fuzzystrmatch's difference() function assumes that _soundex() always initializes its output buffer fully. This was not so for the case of a string containing no alphabetic characters, resulting in unstable output and Valgrind complaints. Fix by using memset() to fill the whole buffer in the early-exit case. Also make some cosmetic improvements (I didn't care for the random switches between "instr[0]" and "*instr" notation). Report and diagnosis by Alexander Lakhin (bug #17935). Back-patch to all supported branches. Discussion: https://postgr.es/m/17935-b99316aa79c18513@postgresql.org
* Adjust sepgsql expected output for 681d9e462 et al.Tom Lane2023-05-08
| | | | Security: CVE-2023-2454
* In hstore_plpython, avoid crashing when return value isn't a mapping.Tom Lane2023-04-27
| | | | | | | | | | | | | | | | | | Python 3 changed the behavior of PyMapping_Check(), breaking the test in plpython_to_hstore() that verifies whether a function result to be transformed is acceptable. A backwards-compatible fix is to first verify that the object doesn't pass PySequence_Check(). Perhaps accidentally, our other uses of PyMapping_Check() already follow uses of PySequence_Check(), so that no other bugs were created by this change. Per bug #17908 from Alexander Lakhin. Back-patch to all supported branches. Dmitry Dolgov and Tom Lane Discussion: https://postgr.es/m/17908-3f19a125d56a11d6@postgresql.org
* Fix misbehavior in contrib/pg_trgm with an unsatisfiable regex.Tom Lane2023-03-11
| | | | | | | | | | | | | | | | | If the regex compiler can see that a regex is unsatisfiable (for example, '$foo') then it may emit an NFA having no arcs. pg_trgm's packGraph function did the wrong thing in this case; it would access off the end of a work array, and with bad luck could produce a corrupted output data structure causing more problems later. This could end with wrong answers or crashes in queries using a pg_trgm GIN or GiST index with such a regex. Fix by not trying to de-duplicate if there aren't at least 2 arcs. Per bug #17830 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17830-57ff5f89bdb02b09@postgresql.org
* Fix contrib/seg to be more wary of long input numbers.Tom Lane2022-12-21
| | | | | | | | | | | | | | | | | | | | | seg stores the number of significant digits in an input number in a "char" field. If char is signed, and the input is more than 127 digits long, the count can read out as negative causing seg_out() to print garbage (or, if you're really unlucky, even crash). To fix, clamp the digit count to be not more than FLT_DIG. (In theory this loses some information about what the original input was, but it doesn't seem like useful information; it would not survive dump/restore in any case.) Also, in case there are stored values of the seg type containing bad data, add a clamp in seg_out's restore() subroutine. Per bug #17725 from Robins Tharakan. It's been like this forever, so back-patch to all supported branches. Discussion: https://postgr.es/m/17725-0a09313b67fbe86e@postgresql.org
* postgres_fdw: Fix assertion in estimate_path_cost_size().Etsuro Fujita2022-12-15
| | | | | | | | | | | | | | | | | | Commit 08d2d58a2 added this assertion assuming that the retrieved_rows estimate for a foreign relation is set to at least one row in estimate_path_cost_size(), but the assumption isn't correct because if the relation is a foreign table with tuples=0, the estimate would be set to 0 in there when using local stats, and if the query's WHERE clause has a NULL condition, the estimate could be set to 0 in there when using remote estimates. (Note: even in the latter case the assertion could be reachable when costing foreign final paths.) Repair. Per bug #17713 from Robins Tharakan. This patch was already applied to v13 or later; apply it to v12 as well where the aforementioned commit was added. Thanks to Richard Guo for investigation. Discussion: http://postgr.es/m/17713-92cce66de7e81c04%40postgresql.org Discussion: http://postgr.es/m/CAEP4nAza%2B0fTCLkgkKYux3JDo3tUBTQORehP%2BaCxSNURpSFpHw%40mail.gmail.com
* Replace RelationOpenSmgr() with RelationGetSmgr().Tom Lane2022-11-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a back-patch of the v15-era commit f10f0ae42 into older supported branches. The idea is to design out bugs in which an ill-timed relcache flush clears rel->rd_smgr partway through some code sequence that wasn't expecting that. We had another report today of a corner case that reliably crashes v14 under debug_discard_caches (nee CLOBBER_CACHE_ALWAYS), and therefore would crash once in a blue moon in the field. We're unlikely to get rid of all such code paths unless we adopt the more rigorous coding rules instituted by f10f0ae42. Therefore, even though this is a bit invasive, it's time to back-patch. Some comfort can be taken in the fact that f10f0ae42 has been in v15 for 16 months without problems. I left the RelationOpenSmgr macro present in the back branches, even though no core code should use it anymore, in order to not break third-party extensions in minor releases. Such extensions might opt to start using RelationGetSmgr instead, to reduce their code differential between v15 and earlier branches. This carries a hazard of failing to compile against headers from existing minor releases. However, once compiled the extension should work fine even with such releases, because RelationGetSmgr is a "static inline" function so it creates no link-time dependency. So depending on distribution practices, that might be an OK tradeoff. Per report from Spyridon Dimitrios Agathos. Original patch by Amul Sul. Discussion: https://postgr.es/m/CAFM5RaqdgyusQvmWkyPYaWMwoK5gigdtW-7HcgHgOeAw7mqJ_Q@mail.gmail.com Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
* Fix compilation warnings with libselinux 3.1 in contrib/sepgsql/Michael Paquier2022-11-09
| | | | | | | | | | | | | | | | | | | | | | Upstream SELinux has recently marked security_context_t as officially deprecated, causing warnings with -Wdeprecated-declarations. This is considered as legacy code for some time now by upstream as security_context_t got removed from most of the code tree during the development of 2.3 back in 2014. This removes all the references to security_context_t in sepgsql/ to be consistent with SELinux, fixing the warnings. Note that this does not impact the minimum version of libselinux supported. This has been applied first as 1f32136 for 14~, but no other branches got the call. This is in line with the recent project policy to have no warnings in branches where builds should still be supported (9.2~ as of today). Per discussion with Tom Lane and Álvaro Herrera. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/20200813012735.GC11663@paquier.xyz Discussion: https://postgr.es/m/20221103181028.raqta27jcuypor4l@alvherre.pgsql Backpatch-through: 9.2
* pg_stat_statements: fetch stmt location/length before it disappears.Tom Lane2022-11-01
| | | | | | | | | | | | | | | | | | | | When executing a utility statement, we must fetch everything we need out of the PlannedStmt data structure before calling standard_ProcessUtility. In certain cases (possibly only ROLLBACK in extended query protocol), that data structure will get freed during command execution. The situation is probably often harmless in production builds, but in debug builds we intentionally overwrite the freed memory with garbage, leading to picking up garbage values of statement location and length, typically causing an assertion failure later in pg_stat_statements. In non-debug builds, if something did go wrong it would likely lead to storing garbage for the query string. Report and fix by zhaoqigui (with cosmetic adjustments by me). It's an old problem, so back-patch to all supported versions. Discussion: https://postgr.es/m/17663-a344fd0675f92128@postgresql.org Discussion: https://postgr.es/m/1667307420050.56657@hundsun.com
* Fix assertion failures while processing NEW_CID record in logical decoding.Amit Kapila2022-10-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | When the logical decoding restarts from NEW_CID, since there is no association between the top transaction and its subtransaction, both are created as top transactions and have the same LSN. This caused the assertion failure in AssertTXNLsnOrder(). This patch skips the assertion check until we reach the LSN at which we start decoding the contents of the transaction, specifically start_decoding_at LSN in SnapBuild. This is okay because we don't guarantee to make the association between top transaction and subtransaction until we try to decode the actual contents of transaction. The ordering of the records prior to the start_decoding_at LSN should have been checked before the restart. The other assertion failure is due to the reason that we forgot to track that we have considered top-level transaction id in the list of catalog changing transactions that were committed when one of its subtransactions is marked as containing catalog change. Reported-by: Tomas Vondra, Osumi Takamichi Author: Masahiko Sawada, Kuroda Hayato Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada Backpatch-through: 10 Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com
* postgres_fdw: Avoid 'variable not found in subplan target list' error.Etsuro Fujita2022-09-14
| | | | | | | | | | | | | | | | | | | | | | The tlist of the EvalPlanQual outer plan for a ForeignScan node is adjusted to produce a tuple whose descriptor matches the scan tuple slot for the ForeignScan node. But in the case where the outer plan contains an extra Sort node, if the new tlist contained columns required only for evaluating PlaceHolderVars or columns required only for evaluating local conditions, this would cause setrefs.c to fail with the error. The cause of this is that when creating the outer plan by injecting the Sort node into an alternative local join plan that could emit such extra columns as well, we fail to arrange for the outer plan to propagate them up through the Sort node, causing setrefs.c to fail to match up them in the new tlist to what is available from the outer plan. Repair. Per report from Alexander Pyhalov. Richard Guo and Etsuro Fujita, reviewed by Alexander Pyhalov and Tom Lane. Backpatch to all supported versions. Discussion: http://postgr.es/m/cfb17bf6dfdf876467bd5ef533852d18%40postgrespro.ru
* Reject bogus output from uuid_create(3).Tom Lane2022-09-09
| | | | | | | | | | | | | | | | | | | | | | When using the BSD UUID functions, contrib/uuid-ossp expects uuid_create() to produce a version-1 UUID. FreeBSD still does so, but in recent NetBSD releases that function produces a version-4 (random) UUID instead. That's not acceptable for our purposes: if the user wanted v4 she would have asked for v4, not v1. Hence, check the version digit and complain if it's not '1'. Also drop the documentation's claim that the NetBSD implementation is usable. It might be, depending on which OS version you're using, but we're not going to get into that kind of detail. (Maybe someday we should ditch all these external libraries and just write our own UUID code, but today is not that day.) Nazir Bilal Yavuz, with cosmetic adjustments and docs by me. Backpatch to all supported versions. Discussion: https://postgr.es/m/3848059.1661038772@sss.pgh.pa.us Discussion: https://postgr.es/m/17358-89806e7420797025@postgresql.org
* Fix catalog lookup with the wrong snapshot during logical decoding.Amit Kapila2022-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we relied on HEAP2_NEW_CID records and XACT_INVALIDATION records to know if the transaction has modified the catalog, and that information is not serialized to snapshot. Therefore, after the restart, if the logical decoding decodes only the commit record of the transaction that has actually modified a catalog, we will miss adding its XID to the snapshot. Thus, we will end up looking at catalogs with the wrong snapshot. To fix this problem, this changes the snapshot builder so that it remembers the last-running-xacts list of the decoded RUNNING_XACTS record after restoring the previously serialized snapshot. Then, we mark the transaction as containing catalog changes if it's in the list of initial running transactions and its commit record has XACT_XINFO_HAS_INVALS. To avoid ABI breakage, we store the array of the initial running transactions in the static variables InitialRunningXacts and NInitialRunningXacts, instead of storing those in SnapBuild or ReorderBuffer. This approach has a false positive; we could end up adding the transaction that didn't change catalog to the snapshot since we cannot distinguish whether the transaction has catalog changes only by checking the COMMIT record. It doesn't have the information on which (sub) transaction has catalog changes, and XACT_XINFO_HAS_INVALS doesn't necessarily indicate that the transaction has catalog change. But that won't be a problem since we use snapshot built during decoding only to read system catalogs. On the master branch, we took a more future-proof approach by writing catalog modifying transactions to the serialized snapshot which avoids the above false positive. But we cannot backpatch it because of a change in the SnapBuild. Reported-by: Mike Oh Author: Masahiko Sawada Reviewed-by: Amit Kapila, Shi yu, Takamichi Osumi, Kyotaro Horiguchi, Bertrand Drouvot, Ahsan Hadi Backpatch-through: 10 Discussion: https://postgr.es/m/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.com
* Be more wary about 32-bit integer overflow in pg_stat_statements.Tom Lane2022-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We've heard a couple of reports of people having trouble with multi-gigabyte-sized query-texts files. It occurred to me that on 32-bit platforms, there could be an issue with integer overflow of calculations associated with the total query text size. Address that with several changes: 1. Limit pg_stat_statements.max to INT_MAX / 2 not INT_MAX. The hashtable code will bound it to that anyway unless "long" is 64 bits. We still need overflow guards on its use, but this helps. 2. Add a check to prevent extending the query-texts file to more than MaxAllocHugeSize. If it got that big, qtext_load_file would certainly fail, so there's not much point in allowing it. Without this, we'd need to consider whether extent, query_offset, and related variables shouldn't be off_t not size_t. 3. Adjust the comparisons in need_gc_qtexts() to be done in 64-bit arithmetic on all platforms. It appears possible that under duress those multiplications could overflow 32 bits, yielding a false conclusion that we need to garbage-collect the texts file, which could lead to repeatedly garbage-collecting after every hash table insertion. Per report from Bruno da Silva. I'm not convinced that these issues fully explain his problem; there may be some other bug that's contributing to the query-texts file becoming so large in the first place. But it did get that big, so #2 is a reasonable defense, and #3 could explain the reported performance difficulties. (See also commit 8bbe4cbd9, which addressed some related bugs. The second Discussion: link is the thread that led up to that.) This issue is old, and is primarily a problem for old platforms, so back-patch. Discussion: https://postgr.es/m/CAB+Nuk93fL1Q9eLOCotvLP07g7RAv4vbdrkm0cVQohDVMpAb9A@mail.gmail.com Discussion: https://postgr.es/m/5601D354.5000703@BlueTreble.com
* postgres_fdw: set search_path to 'pg_catalog' while deparsing constants.Tom Lane2022-07-17
| | | | | | | | | | | | | The motivation for this is to ensure successful transmission of the values of constants of regconfig and other reg* types. The remote will be reading them with search_path = 'pg_catalog', so schema qualification is necessary when referencing objects in other schemas. Per bug #17483 from Emmanuel Quincerot. Back-patch to all supported versions. (There's some other stuff to do here, but it's less back-patchable.) Discussion: https://postgr.es/m/1423433.1652722406@sss.pgh.pa.us
* CREATE INDEX: use the original userid for more ACL checks.Noah Misch2022-06-25
| | | | | | | | | | | | | Commit a117cebd638dd02e5c2e791c25e43745f233111b used the original userid for ACL checks located directly in DefineIndex(), but it still adopted the table owner userid for more ACL checks than intended. That broke dump/reload of indexes that refer to an operator class, collation, or exclusion operator in a schema other than "public" or "pg_catalog". Back-patch to v10 (all supported versions), like the earlier commit. Nathan Bossart and Noah Misch Discussion: https://postgr.es/m/f8a4105f076544c180a87ef0c4822352@stmuk.bayern.de
* Silence compiler warnings from some older compilers.Tom Lane2022-06-01
| | | | | | | | | | | | Since a117cebd6, some older gcc versions issue "variable may be used uninitialized in this function" complaints for brin_summarize_range. Silence that using the same coding pattern as in bt_index_check_internal; arguably, a117cebd6 had too narrow a view of which compilers might give trouble. Nathan Bossart and Tom Lane. Back-patch as the previous commit was. Discussion: https://postgr.es/m/20220601163537.GA2331988@nathanxps13
* Make relation-enumerating operations be security-restricted operations.Noah Misch2022-05-09
| | | | | | | | | | | | | | | | | | When a feature enumerates relations and runs functions associated with all found relations, the feature's user shall not need to trust every user having permission to create objects. BRIN-specific functionality in autovacuum neglected to account for this, as did pg_amcheck and CLUSTER. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. CREATE INDEX (not a relation-enumerating operation) and REINDEX protected themselves too late. This change extends to the non-enumerating amcheck interface. Back-patch to v10 (all supported versions). Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2022-1552
* Fix back-patch of "Under has_wal_read_bug, skip .../001_wal.pl."Noah Misch2022-05-07
| | | | | Per buildfarm members tadarida, snapper, and kittiwake. Back-patch to v10 (all supported versions).
* Under has_wal_read_bug, skip contrib/bloom/t/001_wal.pl.Noah Misch2022-05-07
| | | | | | | Per buildfarm members snapper and kittiwake. Back-patch to v10 (all supported versions). Discussion: https://postgr.es/m/20220116210241.GC756210@rfd.leadboat.com
* pageinspect: Fix handling of all-zero pagesMichael Paquier2022-04-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Getting from get_raw_page() an all-zero page is considered as a valid case by the buffer manager and it can happen for example when finding a corrupted page with zero_damaged_pages enabled (using zero_damaged_pages to look at corrupted pages happens), or after a crash when a relation file is extended before any WAL for its new data is generated (before a vacuum or autovacuum job comes in to do some cleanup). However, all the functions of pageinspect, as of the index AMs (except hash that has its own idea of new pages), heap, the FSM or the page header have never worked with all-zero pages, causing various crashes when going through the page internals. This commit changes all the pageinspect functions to be compliant with all-zero pages, where the choice is made to return NULL or no rows for SRFs when finding a new page. get_raw_page() still works the same way, returning a batch of zeros in the bytea of the page retrieved. A hard error could be used but NULL, while more invasive, is useful when scanning relation files in full to get a batch of results for a single relation in one query. Tests are added for all the code paths impacted. Reported-by: Daria Lepikhova Author: Michael Paquier Discussion: https://postgr.es/m/561e187b-3549-c8d5-03f5-525c14e65bd0@postgrespro.ru Backpatch-through: 10
* Fix postgres_fdw to check shippability of sort clauses properly.Tom Lane2022-03-31
| | | | | | | | | | | | | | | | | | | | | | | | | postgres_fdw would push ORDER BY clauses to the remote side without verifying that the sort operator is safe to ship. Moreover, it failed to print a suitable USING clause if the sort operator isn't default for the sort expression's type. The net result of this is that the remote sort might not have anywhere near the semantics we expect, which'd be disastrous for locally-performed merge joins in particular. We addressed similar issues in the context of ORDER BY within an aggregate function call in commit 7012b132d, but failed to notice that query-level ORDER BY was broken. Thus, much of the necessary logic already existed, but it requires refactoring to be usable in both cases. Back-patch to all supported branches. In HEAD only, remove the core code's copy of find_em_expr_for_rel, which is no longer used and really should never have been pushed into equivclass.c in the first place. Ronan Dunklau, per report from David Rowley; reviews by David Rowley, Ranier Vilela, and myself Discussion: https://postgr.es/m/CAApHDvr4OeC2DBVY--zVP83-K=bYrTD7F8SZDhN4g+pj2f2S-A@mail.gmail.com
* pageinspect: Add more sanity checks to prevent out-of-bound readsMichael Paquier2022-03-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A couple of code paths use the special area on the page passed by the function caller, expecting to find some data in it. However, feeding an incorrect page can lead to out-of-bound reads when trying to access the page special area (like a heap page that has no special area, leading PageGetSpecialPointer() to grab a pointer outside the allocated page). The functions used for hash and btree indexes have some protection already against that, while some other functions using a relation OID as argument would make sure that the access method involved is correct, but functions taking in input a raw page without knowing the relation the page is attached to would run into problems. This commit improves the set of checks used in the code paths of BRIN, btree (including one check if a leaf page is found with a non-zero level), GIN and GiST to verify that the page given in input has a special area size that fits with each access method, which is done though PageGetSpecialSize(), becore calling PageGetSpecialPointer(). The scope of the checks done is limited to work with pages that one would pass after getting a block with get_raw_page(), as it is possible to craft byteas that could bypass existing code paths. Having too many checks would also impact the usability of pageinspect, as the existing code is very useful to look at the content details in a corrupted page, so the focus is really to avoid out-of-bound reads as this is never a good thing even with functions whose execution is limited to superusers. The safest approach could be to rework the functions so as these fetch a block using a relation OID and a block number, but there are also cases where using a raw page is useful. Tests are added to cover all the code paths that needed such checks, and an error message for hash indexes is reworded to fit better with what this commit adds. Reported-By: Alexander Lakhin Author: Julien Rouhaud, Michael Paquier Discussion: https://postgr.es/m/16527-ef7606186f0610a1@postgresql.org Discussion: https://postgr.es/m/561e187b-3549-c8d5-03f5-525c14e65bd0@postgrespro.ru Backpatch-through: 10
* pageinspect: Fix memory context allocation of page in brin_revmap_data()Michael Paquier2022-03-16
| | | | | | | | | | | | | | This caused the function to fail, as the aligned copy of the raw page given by the function caller was not saved in the correct memory context, which needs to be multi_call_memory_ctx in this case. Issue introduced by 076f4d9. Per buildfarm members sifika, mylodon and longfin. I have reproduced that locally with macos. Discussion: https://postgr.es/m/YjFPOtfCW6yLXUeM@paquier.xyz Backpatch-through: 10
* pageinspect: Fix handling of page sizes and AM typesMichael Paquier2022-03-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit fixes a set of issues related to the use of the SQL functions in this module when the caller is able to pass down raw page data as input argument: - The page size check was fuzzy in a couple of places, sometimes looking after only a sub-range, but what we are looking for is an exact match on BLCKSZ. After considering a few options here, I have settled down to do a generalization of get_page_from_raw(). Most of the SQL functions already used that, and this is not strictly required if not accessing an 8-byte-wide value from a raw page, but this feels safer in the long run for alignment-picky environment, particularly if a code path begins to access such values. This also reduces the number of strings that need to be translated. - The BRIN function brin_page_items() uses a Relation but it did not check the access method of the opened index, potentially leading to crashes. All the other functions in need of a Relation already did that. - Some code paths could fail on elog(), but we should to use ereport() for failures that can be triggered by the user. Tests are added to stress all the cases that are fixed as of this commit, with some junk raw pages (\set VERBOSITY ensures that this works across all page sizes) and unexpected index types when functions open relations. Author: Michael Paquier, Justin Prysby Discussion: https://postgr.es/m/20220218030020.GA1137@telsasoft.com Backpatch-through: 10
* Introduce PG_TEST_TIMEOUT_DEFAULT for TAP suite non-elapsing timeouts.Noah Misch2022-03-04
| | | | | | | | | | | | Slow hosts may avoid load-induced, spurious failures by setting environment variable PG_TEST_TIMEOUT_DEFAULT to some number of seconds greater than 180. Developers may see faster failures by setting that environment variable to some lesser number of seconds. In tests, write $PostgreSQL::Test::Utils::timeout_default wherever the convention has been to write 180. This change raises the default for some briefer timeouts. Back-patch to v10 (all supported versions). Discussion: https://postgr.es/m/20220218052842.GA3627003@rfd.leadboat.com
* Clean up assorted failures under clang's -fsanitize=undefined checks.Tom Lane2022-03-03
| | | | | | | | | | | | | | | | | | | | | | Most of these are cases where we could call memcpy() or other libc functions with a NULL pointer and a zero count, which is forbidden by POSIX even though every production version of libc allows it. We've fixed such things before in a piecemeal way, but apparently never made an effort to try to get them all. I don't claim that this patch does so either, but it gets every failure I observe in check-world, using clang 12.0.1 on current RHEL8. numeric.c has a different issue that the sanitizer doesn't like: "ln(-1.0)" will compute log10(0) and then try to assign the resulting -Inf to an integer variable. We don't actually use the result in such a case, so there's no live bug. Back-patch to all supported branches, with the idea that we might start running a buildfarm member that tests this case. This includes back-patching c1132aae3 (Check the size in COPY_POINTER_FIELD), which previously silenced some of these issues in copyfuncs.c. Discussion: https://postgr.es/m/CALNJ-vT9r0DSsAOw9OXVJFxLENoVS_68kJ5x0p44atoYH+H4dg@mail.gmail.com
* WAL log unchanged toasted replica identity key attributes.Amit Kapila2022-02-14
| | | | | | | | | | | | | | | Currently, during UPDATE, the unchanged replica identity key attributes are not logged separately because they are getting logged as part of the new tuple. But if they are stored externally then the untoasted values are not getting logged as part of the new tuple and logical replication won't be able to replicate such UPDATEs. So we need to log such attributes as part of the old_key_tuple during UPDATE. Reported-by: Haiying Tang Author: Dilip Kumar and Amit Kapila Reviewed-by: Alvaro Herrera, Haiying Tang, Andres Freund Backpatch-through: 10 Discussion: https://postgr.es/m/OS0PR01MB611342D0A92D4F4BF26C0F47FB229@OS0PR01MB6113.jpnprd01.prod.outlook.com
* Use Test::Builder::todo_start(), replacing $::TODO.Noah Misch2022-02-09
| | | | | | | | | | Some pre-2017 Test::More versions need perfect $Test::Builder::Level maintenance to find the variable. Buildfarm member snapper reported an overall failure that the file intended to hide via the TODO construct. That trouble was reachable in v11 and v10. For later branches, this serves as defense in depth. Back-patch to v10 (all supported versions). Discussion: https://postgr.es/m/20220202055556.GB2745933@rfd.leadboat.com
* On sparc64+ext4, suppress test failures from known WAL read failure.Noah Misch2022-01-26
| | | | | | | | | | Buildfarm members kittiwake, tadarida and snapper began to fail frequently when commits 3cd9c3b921977272e6650a5efbeade4203c4bca2 and f47ed79cc8a0cfa154dc7f01faaf59822552363f added tests of concurrency, but the problem was reachable before those commits. Back-patch to v10 (all supported versions). Discussion: https://postgr.es/m/20220116210241.GC756210@rfd.leadboat.com