aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Force testing of query jumbling in 027_stream_regress.plMichael Paquier2023-03-03
| | | | | | | | | | | | | | | | | | | | | Coverage of the query jumbling code has always relied on the queries included in the regression tests of pg_stat_statements. This has its limitations, as a lot of query patterns have never really stressed the query jumbling code. The situation got a bit worse since the query jumbling has been added in the backend core code (5fd9dfa), hence new nodes that should be included in the jumbling could easily be missed, resulting in failures in pg_stat_statements or any modules that require query ID computations. Forcing a load of pg_stat_statements in 027_stream_regress.pl ensures that nodes are never missed in the computations, without having to rely on a buildfarm member for this check. Before this commit, the line coverage of queryjumblefuncs.funcs.c was around 48.5%, now up to 94.6% just by running 027_stream_regress.pl. A basic check is added to show that pg_stat_statements reports are generated after the main regression test suite is finished. Discussion: https://postgr.es/m/Y+nD9LN70w+8eaG9@paquier.xyz
* Harden new test case against force_parallel_mode = regress.Tom Lane2023-03-02
| | | | | Per buildfarm: worker processes can't see a role created in the current transaction.
* Show "internal name" not "source code" in psql's \df+ command.Tom Lane2023-03-02
| | | | | | | | | | | | | | Our previous habit of showing the full function body is really pretty unfriendly for tabular viewing of functions, and now that we have \sf and \ef commands there seems no good reason why \df+ has to do it. It still seems to make sense to show prosrc for internal and C-language functions, since in those cases prosrc is just the C function name; but then let's rename the column to "Internal name" which is a more accurate descriptor. Isaac Morland Discussion: https://postgr.es/m/CAMsGm5eqKc6J1=Lwn=ZONG=6ZDYWRQ4cgZQLqMuZGB1aVt_JBg@mail.gmail.com
* Don't leak descriptors into subprograms.Thomas Munro2023-03-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Open long-lived data and WAL file descriptors with O_CLOEXEC. This flag was introduced by SUSv4 (POSIX.1-2008), and by now all of our target Unix systems have it. Our open() implementation for Windows already had that behavior, so provide a dummy O_CLOEXEC flag on that platform. For now, callers of open() and the "thin" wrappers in fd.c that deal in raw descriptors need to pass in O_CLOEXEC explicitly if desired. This commit does that for WAL files, and automatically for everything accessed via VFDs including SMgrRelation and BufFile. (With more discussion we might decide to turn it on automatically for the thin open()-wrappers too to avoid risk of missing places that need it, but these are typically used for short-lived descriptors where we don't expect to fork/exec, and it's remotely possible that extensions could be using these APIs and passing descriptors to subprograms deliberately, so that hasn't been done here.) Do the same for sockets and the postmaster pipe with FD_CLOEXEC. (Later commits might use modern interfaces to remove these extra fcntl() calls and more where possible, but we'll need them as a fallback for a couple of systems, so do it that way in this initial commit.) With this change, subprograms executed for archiving, copying etc will no longer have access to the server's descriptors, other than the ones that we decide to pass down. Reviewed-by: Andres Freund <andres@anarazel.de> (earlier version) Discussion: https://postgr.es/m/CA%2BhUKGKb6FsAdQWcRL35KJsftv%2B9zXqQbzwkfRf1i0J2e57%2BhQ%40mail.gmail.com
* Remove local optimizations of empty Bitmapsets into null pointers.Tom Lane2023-03-02
| | | | | | | | These are all dead code now that it's done centrally. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Require empty Bitmapsets to be represented as NULL.Tom Lane2023-03-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When I designed the Bitmapset module, I set things up so that an empty Bitmapset could be represented either by a NULL pointer, or by an allocated object all of whose bits are zero. I've recently come to the conclusion that that was a bad idea and we should instead have a convention like the longstanding invariant for Lists, whereby an empty list is represented by NIL and nothing else. To do this, we need to fix bms_intersect, bms_difference, and a couple of other functions to check for having produced an empty result; but then we can replace bms_is_empty(a) by a simple "a == NULL" test. This is very likely a (marginal) win performance-wise, because we call bms_is_empty many more times than those other functions put together. However, the real reason to do it is that we have various places that have hand-implemented a rule about "this Bitmapset variable must be exactly NULL if empty", so that they can use checks-for-null in place of bms_is_empty calls in particularly hot code paths. That is a really fragile, mistake-prone way to do things, and I'm surprised that we've seldom been bitten by it. It's not well documented at all which variables have this property, so you can't readily tell which code might be violating those conventions. By making the convention universal, we can eliminate a subtle source of bugs. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Mop up some undue familiarity with the innards of Bitmapsets.Tom Lane2023-03-02
| | | | | | | | | | | | | | | | | | | | | nodeAppend.c used non-nullness of appendstate->as_valid_subplans as a state flag to indicate whether it'd done ExecFindMatchingSubPlans (or some sufficient approximation to that). This was pretty questionable even in the beginning, since it wouldn't really work right if there are no valid subplans. It got more questionable after commit 27e1f1456 added logic that could reduce as_valid_subplans to an empty set: at that point we were depending on unspecified behavior of bms_del_members, namely that it'd not return an empty set as NULL. It's about to start doing that, which breaks this logic entirely. Hence, add a separate boolean flag to signal whether as_valid_subplans has been computed. Also fix a previously-cosmetic bug in nodeAgg.c, wherein it ignored the return value of bms_del_member instead of updating its pointer. Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Remove bms_first_member().Tom Lane2023-03-02
| | | | | | | | | | | | | | This function has been semi-deprecated ever since we invented bms_next_member(). Its habit of scribbling on the input bitmapset isn't great, plus for sufficiently large bitmapsets it would take O(N^2) time to complete a loop. Now we have the additional problem that reducing the input to empty while leaving it still accessible would violate a planned invariant. So let's just get rid of it, after updating the few extant callers to use bms_next_member(). Patch by me; thanks to Nathan Bossart and Richard Guo for review. Discussion: https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
* Mark options as deprecated in usage outputDaniel Gustafsson2023-03-02
| | | | | | | | | Some deprecated options were not marked as such in usage output. This does so across the installed binaries in an attempt to provide consistent markup for this. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/062C6A8A-A4E8-4F52-9E31-45F0C9E9915E@yesql.se
* Fix outdated references to guc.cDaniel Gustafsson2023-03-02
| | | | | | | | | Commit 0a20ff54f split out the GUC variables from guc.c into a new file guc_tables.c. This updates comments referencing guc.c regarding variables which are now in guc_tables.c. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/6B50C70C-8C1F-4F9A-A7C0-EEAFCC032406@yesql.se
* Make some xlogreader messages more accuratePeter Eisentraut2023-03-02
| | | | | | | | | | | | When you have some invalid WAL, you often get a message like "wanted 24, got 0". This is a bit incorrect, since it really wanted *at least* 24, not exactly 24. This updates the messages to that effect, and also adds that detail to one message where it was available but not printed. Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Jeevan Ladhe <jeevanladhe.os@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/726d782b-5e45-0c3e-d775-6686afe9aa83%40enterprisedb.com
* Avoid fetching one past the end of translate()'s "to" parameter.Tom Lane2023-03-01
| | | | | | | | | | | | | | | | | This is usually harmless, but if you were very unlucky it could provoke a segfault due to the "to" string being right up against the end of memory. Found via valgrind testing (so we might've found it earlier, except that our regression tests lacked any exercise of translate()'s deletion feature). Fix by switching the order of the test-for-end-of-string and advance-pointer steps. While here, compute "to_ptr + tolen" just once. (Smarter compilers might figure that out for themselves, but let's just make sure.) Report and fix by Daniil Anisimov, in bug #17816. Discussion: https://postgr.es/m/17816-70f3d2764e88a108@postgresql.org
* Improve wording in pg_dump compression docsTomas Vondra2023-03-01
| | | | | | | | A couple minor corrections in pg_dump comments and docs, related to the recently introduced compression API. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20230227044910.GO1653@telsasoft.com
* Fix condition in pg_dump TAP testTomas Vondra2023-03-01
| | | | | | | | The condition checking compression support was parenthesized incorrectly after adding lz4, so fix that. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20230227044910.GO1653@telsasoft.com
* meson: Add equivalent of configure --disable-rpath optionPeter Eisentraut2023-03-01
| | | | Discussion: https://www.postgresql.org/message-id/flat/33e957e6-4b4e-b0ed-1cc1-6335a24543ff%40enterprisedb.com
* Suppress more compiler warnings in new pgstats code.Tom Lane2023-02-28
| | | | | | | | Per buildfarm, we didn't get rid of quite all of the -Wtautological-constant-out-of-range-compare warnings in pgstat_io.c. Discussion: https://postgr.es/m/20520.1677435600@sss.pgh.pa.us
* Fix logic buglets in pg_dump's flagInhAttrs().Tom Lane2023-02-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As it stands, flagInhAttrs() can make changes in table properties that change decisions made at other tables during other iterations of its loop. This is a pretty bad idea, since we visit the tables in OID order which is not necessarily related to inheritance relationships. So far as I can tell, the consequences are just cosmetic: we might dump DEFAULT or GENERATED expressions that we don't really need to because they match properties of the parent. Nonetheless, it's buggy, and somebody might someday add functionality here that fails less benignly when the traversal order varies. One issue is that when we decide we needn't dump a particular GENERATED expression, we physically unlink the struct for it, so that it will now look like the table has no such expression, causing the wrong choice to be made at any child visited later. We can improve that by instead clearing the dobj.dump flag, and taking care to check that flag when it comes time to dump the expression or not. The other problem is that if we decide we need to fake up a DEFAULT NULL clause to override a default that would otherwise get inherited, we modify the data structure in the reverse fashion, creating an attrdefs entry where there hadn't been one. It's harder to avoid doing that, but since the backend won't report a plain "DEFAULT NULL" property we can modify the code to recognize ones we just added. Add some commentary to perhaps forestall future mistakes of the same ilk. Since the effects of this seem only cosmetic, no back-patch. Discussion: https://postgr.es/m/1506298.1676323579@sss.pgh.pa.us
* Remove unnecessary and problematic collate.windows.win1252 testsAndrew Dunstan2023-02-28
| | | | | | | | | | Some windows instances can't handle setting lc_time to a non BCP 47 locale, and the removed tests in any case don't really make lots of sense here. Juan José Santamaría Flecha Discussion: https://postgr.es/m/237b255b-e063-a82e-66e1-c63a12bf9664@dunslane.net
* Fix expected output of xml_2.outMichael Paquier2023-02-28
| | | | | | | | Per buildfarm members snakefly, parula and prion, that reflect the results coming from the latest versions of libxml2. Oversight in b8da37b in the shape of an incorrect copy-paste. The CI was green, but it does not stress this expected output.
* Rework pg_input_error_message(), now renamed pg_input_error_info()Michael Paquier2023-02-28
| | | | | | | | | | | | | | | | | | | pg_input_error_info() is now a SQL function able to return a row with more than just the error message generated for incorrect data type inputs when these are able to handle soft failures, returning more contents of ErrorData, as of: - The error message (same as before). - The error detail, if set. - The error hint, if set. - SQL error code. All the regression tests that relied on pg_input_error_message() are updated to reflect the effects of the rename. Per discussion with Tom Lane and Andrew Dunstan. Author: Nathan Bossart Discussion: https://postgr.es/m/139a68e1-bd1f-a9a7-b5fe-0be9845c6311@dunslane.net
* Suppress compiler warnings in new pgstats code.Tom Lane2023-02-27
| | | | | | | | | | | | | | | | | | | | | Some clang versions whine about comparing an enum variable to a value outside the range of the enum, on the grounds that the result must be constant. In the cases we fix here, the loops will terminate only if the enum variable can in fact hold a value one beyond its declared range. While that's very likely to always be true for these enum types, it still seems like a poor coding practice to assume it; so use "int" loop variables instead to silence the warnings. (This matches what we've done in other places, for example loops over the range of ForkNumber.) While at it, let's drop the XXX_FIRST macros for these enums and just write zeroes for the loop start values. The apparent flexibility seems rather illusory given that iterating up to one-less-than- the-number-of-values is only correct for a zero-based range. Melanie Plageman Discussion: https://postgr.es/m/20520.1677435600@sss.pgh.pa.us
* Update types in smgr APIPeter Eisentraut2023-02-27
| | | | | | | | Change data buffer to void *, from char *, and add const where appropriate. This makes it match the File API (see also 2d4f1ba6cfc2f0a977f1c30bda9848041343e248) and stdio. Discussion: https://www.postgresql.org/message-id/flat/11dda853-bb5b-59ba-a746-e168b1ce4bdb%40enterprisedb.com
* Change xl_hash_vacuum_one_page.ntuples from int to uint16.Amit Kapila2023-02-27
| | | | | | | | | | | This will create two bytes of padding space in xl_hash_vacuum_one_page which can be used for future patches. This makes the datatype of xl_hash_vacuum_one_page.ntuples same as gistxlogDelete.ntodelete which is advisable as both are used for the same purpose. Author: Bertrand Drouvot Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/b0e20c40-cb7a-fc1c-c607-2a78dac5021e@gmail.com
* Silence more compiler warnings introduced by d87d548cd0.Tom Lane2023-02-26
| | | | | | Per buildfarm, there are still a couple of functions where we get warnings from compilers that don't know that elog(ERROR) doesn't return.
* Don't force SQL_ASCII/no-locale for installcheck in vcregress.plAndrew Dunstan2023-02-26
| | | | | | | | It's been this way for a very long time, but it appears to have been masking an issue that only manifests with different settings. Therefore, run the tests in the installation's default encoding/locale. Backpatch to all live branches.
* Fix MULTIEXPR_SUBLINK with partitioned target tables, yet again.Tom Lane2023-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We already tried to fix this in commits 3f7323cbb et al (and follow-on fixes), but now it emerges that there are still unfixed cases; moreover, these cases affect all branches not only pre-v14. I thought we had eliminated all cases of making multiple clones of an UPDATE's target list when we nuked inheritance_planner. But it turns out we still do that in some partitioned-UPDATE cases, notably including INSERT ... ON CONFLICT UPDATE, because ExecInitPartitionInfo thinks it's okay to clone and modify the parent's targetlist. This fix is based on a suggestion from Andres Freund: let's stop abusing the ParamExecData.execPlan mechanism, which was only ever meant to handle initplans, and instead solve the execution timing problem by having the expression compiler move MULTIEXPR_SUBLINK steps to the front of their expression step lists. This is feasible because (a) all branches still in support compile the entire targetlist of an UPDATE into a single ExprState, and (b) we know that all MULTIEXPR_SUBLINKs do need to be evaluated --- none could be buried inside a CASE, for example. There is a minor semantics change concerning the order of execution of the MULTIEXPR's subquery versus other parts of the parent targetlist, but that seems like something we can get away with. By doing that, we no longer need to worry about whether different clones of a MULTIEXPR_SUBLINK share output Params; their usage of that data structure won't overlap. Per bug #17800 from Alexander Lakhin. Back-patch to all supported branches. In v13 and earlier, we can revert 3f7323cbb and follow-on fixes; however, I chose to keep the SubPlan.subLinkId field added in ccbb54c72. We don't need that anymore in the core code, but it's cheap enough to fill, and removing a plan node field in a minor release seems like it'd be asking for trouble. Andres Freund and Tom Lane Discussion: https://postgr.es/m/17800-ff90866b3906c964@postgresql.org
* Fix mishandling of OLD/NEW references in subqueries in rule actions.Dean Rasheed2023-02-25
| | | | | | | | | | | | | | | | | If a rule action contains a subquery that refers to columns from OLD or NEW, then those are really lateral references, and the planner will complain if it sees such things in a subquery that isn't marked as lateral. However, at rule-definition time, the user isn't required to mark the subquery with LATERAL, and so it can fail when the rule is used. Fix this by marking such subqueries as lateral in the rewriter, at the point where they're used. Dean Rasheed and Tom Lane, per report from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/5e09da43-aaba-7ea7-0a51-a2eb981b058b%40gmail.com
* Silence compiler warnings introduced by d87d548cd0.Jeff Davis2023-02-24
| | | | | Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20230224002029.GQ1653@telsasoft.com
* Disallow NULLS NOT DISTINCT indexes for primary keysDaniel Gustafsson2023-02-24
| | | | | | | | | | | | | A unique index which is created with non-distinct NULLS cannot be used for backing a primary key constraint. Make sure to disallow such table alterations and teach pg_dump to drop the non-distinct NULLS clause on indexes where this has been set. Bug: 17720 Reported-by: Reiner Peterke <zedaardv@drizzle.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/17720-dab8ee0fa85d316d@postgresql.org
* pg_dump: Remove move "blob" terminologyDaniel Gustafsson2023-02-24
| | | | | | | | | | Commit e9960732a9618 accidentally introduced the blob terminology in error messages which had previously been altered by commit 35ce24c33 from "blob" to "LO". This reverts back to "LO". Reported-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20230224.163127.68506240520261483.horikyota.ntt@gmail.com Discussion: https://www.postgresql.org/message-id/flat/868a381f-4650-9460-1726-1ffd39a270b4%40enterprisedb.com
* Fix incorrect format placeholdersPeter Eisentraut2023-02-24
|
* Don't repeatedly register cache callbacks in pgoutput plugin.Tom Lane2023-02-23
| | | | | | | | | | | | | | Multiple cycles of starting up and shutting down the plugin within a single session would eventually lead to "out of relcache_callback_list slots", because pgoutput_startup blindly re-registered its cache callbacks each time. Fix it to register them only once, as all other users of cache callbacks already take care to do. This has been broken all along, so back-patch to all supported branches. Shi Yu Discussion: https://postgr.es/m/OSZPR01MB631004A78D743D68921FFAD3FDA79@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Add LZ4 compression to pg_dumpTomas Vondra2023-02-23
| | | | | | | | | | | | Expand pg_dump's compression streaming and file APIs to support the lz4 algorithm. The newly added compress_lz4.{c,h} files cover all the functionality of the aforementioned APIs. Minor changes were necessary in various pg_backup_* files, where code for the 'lz4' file suffix has been added, as well as pg_dump's compression option parsing. Author: Georgios Kokolatos Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Shi Yu, Tomas Vondra Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
* Remove unnecessary #ifdef USE_ICU and branch.Jeff Davis2023-02-23
| | | | | | | | Now that the provider-independent API pg_strnxfrm() is available, we no longer need the special cases for ICU in hashfunc.c and varchar.c. Reviewed-by: Peter Eisentraut, Peter Geoghegan Discussion: https://postgr.es/m/a581136455c940d7bd0ff482d3a2bd51af25a94f.camel%40j-davis.com
* Refactor to introduce pg_locale_deterministic().Jeff Davis2023-02-23
| | | | | | | | Avoids the need of callers to test for NULL, and also avoids the need to access the pg_locale_t structure directly. Reviewed-by: Peter Eisentraut, Peter Geoghegan Discussion: https://postgr.es/m/a581136455c940d7bd0ff482d3a2bd51af25a94f.camel%40j-davis.com
* Refactor to add pg_strcoll(), pg_strxfrm(), and variants.Jeff Davis2023-02-23
| | | | | | | | | | | | | Offers a generally better separation of responsibilities for collation code. Also, a step towards multi-lib ICU, which should be based on a clean separation of the routines required for collation providers. Callers with NUL-terminated strings should call pg_strcoll() or pg_strxfrm(); callers with strings and their length should call the variants pg_strncoll() or pg_strnxfrm(). Reviewed-by: Peter Eisentraut, Peter Geoghegan Discussion: https://postgr.es/m/a581136455c940d7bd0ff482d3a2bd51af25a94f.camel%40j-davis.com
* Introduce a generic pg_dump compression APITomas Vondra2023-02-23
| | | | | | | | | | | | | | | | | Switch pg_dump to use the Compression API, implemented by bf9aa490db. The CompressFileHandle replaces the cfp* family of functions with a struct of callbacks for accessing (compressed) files. This allows adding new compression methods simply by introducing a new struct instance with appropriate implementation of the callbacks. Archives compressed using custom compression methods store an identifier of the compression algorithm in their header instead of the compression level. The header version is bumped. Author: Georgios Kokolatos Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Tomas Vondra Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
* Fix mis-handling of outer join quals generated by EquivalenceClasses.Tom Lane2023-02-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's possible, in admittedly-rather-contrived cases, for an eclass to generate a derived "join" qual that constrains the post-outer-join value(s) of some RHS variable(s) without mentioning the LHS at all. While the mechanisms were set up to work for this, we fell foul of the "get_common_eclass_indexes" filter installed by commit 3373c7155: it could decide that such an eclass wasn't relevant to the join, so that the required qual clause wouldn't get emitted there or anywhere else. To fix, apply get_common_eclass_indexes only at inner joins, where its rule is still valid. At an outer join, fall back to examining all eclasses that mention either input (or the OJ relid, though it should be impossible for an eclass to mention that without mentioning either input). Perhaps we can improve on that later, but the cost/benefit of adding more complexity to skip some irrelevant eclasses is dubious. To allow cheaply distinguishing outer from inner joins, pass the ojrelid to generate_join_implied_equalities as a separate argument. This also allows cleaning up some sloppiness that had crept into the definition of its join_relids argument, and it allows accurate calculation of nominal_join_relids for a child outer join. (The latter oversight seems not to have been a live bug, but it certainly could have caused problems in future.) Also fix what might be a live bug in check_index_predicates: it was being sloppy about what it passed to generate_join_implied_equalities. Per report from Richard Guo. Discussion: https://postgr.es/m/CAMbWs4-DsTBfOvXuw64GdFss2=M5cwtEhY=0DCS7t2gT7P6hSA@mail.gmail.com
* Prepare pg_dump internals for additional compression methodsTomas Vondra2023-02-23
| | | | | | | | | | | | | | | Commit bf9aa490db introduced a compression API in compress_io.{c,h} to make reuse easier, and allow adding more compression algorithms. However, pg_backup_archiver.c was not switched to this API and continued to call the compression directly. This commit teaches pg_backup_archiver.c about the compression API, so that it can benefit from bf9aa490db (simpler code, easier addition of new compression methods). Author: Georgios Kokolatos Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Tomas Vondra Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
* pg_rewind: Fix determining TLI when server was just promoted.Heikki Linnakangas2023-02-23
| | | | | | | | | | | | | | | | | | | | | | | If the source server was just promoted, and it hasn't written the checkpoint record yet, pg_rewind considered the server to be still on the old timeline. Because of that, it would claim incorrectly that no rewind is required. Fix that by looking at minRecoveryPointTLI in the control file in addition to the ThisTimeLineID on the checkpoint. This has been a known issue since forever, and we had worked around it in the regression tests by issuing a checkpoint after each promotion, before running pg_rewind. But that was always quite hacky, so better to fix this properly. This doesn't add any new tests for this, but removes the previously-added workarounds from the existing tests, so that they should occasionally hit this codepath again. This is arguably a bug fix, but don't backpatch because we haven't really treated it as a bug so far. Also, the patch didn't apply cleanly to v13 and below. I'm sure sure it could be made to work on v13, but doesn't seem worth the risk and effort. Reviewed-by: Kyotaro Horiguchi, Ibrar Ahmed, Aleksander Alekseev Discussion: https://www.postgresql.org/message-id/9f568c97-87fe-a716-bd39-65299b8a60f4%40iki.fi
* Fix multi-row DEFAULT handling for INSERT ... SELECT rules.Dean Rasheed2023-02-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Given an updatable view with a DO ALSO INSERT ... SELECT rule, a multi-row INSERT ... VALUES query on the view fails if the VALUES list contains any DEFAULTs that are not replaced by view defaults. This manifests as an "unrecognized node type" error, or an Assert failure, in an assert-enabled build. The reason is that when RewriteQuery() attempts to replace the remaining DEFAULT items with NULLs in any product queries, using rewriteValuesRTEToNulls(), it assumes that the VALUES RTE is located at the same rangetable index in each product query. However, if the product query is an INSERT ... SELECT, then the VALUES RTE is actually in the SELECT part of that query (at the same index), rather than the top-level product query itself. Fix, by descending to the SELECT in such cases. Note that we can't simply use getInsertSelectQuery() for this, since that expects to be given a raw rule action with OLD and NEW placeholder entries, so we duplicate its logic instead. While at it, beef up the checks in getInsertSelectQuery() by checking that the jointree->fromlist node is indeed a RangeTblRef, and that the RTE it points to has rtekind == RTE_SUBQUERY. Per bug #17803, from Alexander Lakhin. Back-patch to all supported branches. Dean Rasheed, reviewed by Tom Lane. Discussion: https://postgr.es/m/17803-53c63ed4ecb4eac6%40postgresql.org
* Consider a failed process as a failed test in pg_regressDaniel Gustafsson2023-02-23
| | | | | | | | | | | Commit 55de145d1cf added reporting of child process failures, but the test suite is still allowed to pass even if the process failed. Since regress tests are higher level tests, a false positive is more likely in this case so report failed test processes as failed tests. Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/82C46B5E-1821-4039-82C2-56BCA5992989@yesql.se Discussion: https://postgr.es/m/20221122235636.4frx7hjterq6bmls@awork3.anarazel.de
* Add static assertion ensuring sizeof(ExprEvalStep) <= 64 bytesAndres Freund2023-02-22
| | | | | | | | | | | | This was previously only documented in a comment. Given the size of the struct, it's not hard to miss that comment. As evidenced by the commits leading up to fe3caa14393, 67b26703b41. It's possible, but not likely, that we might have to weaken these assertions on a less commonly used architecture. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/295606.1677101684@sss.pgh.pa.us
* Check for unbounded authentication exchanges in libpq.Heikki Linnakangas2023-02-22
| | | | | | | | | | | | | | A couple of code paths in CONNECTION_AWAITING_RESPONSE will eagerly read bytes off a connection that should be closed. Don't let a misbehaving server chew up client resources here; a v2 error can't be infinitely long, and a v3 error should be bounded by its original message length. For the existing error_return cases, I added some additional error messages for symmetry with the new ones, and cleaned up some message rot. Author: Jacob Champion Discussion: https://www.postgresql.org/message-id/8e729daf-7d71-6965-9687-8bc0630599b3%40timescale.com
* Fix some issues with wrong placement of pseudo-constant quals.Tom Lane2023-02-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | initsplan.c figured that it could push Var-free qual clauses to the top of the current JoinDomain, which is okay in the abstract. But if the current domain is inside some outer join, and we later commute an inside-the-domain outer join with one outside it, we end up placing the pushed-up qual clause incorrectly. In distribute_qual_to_rels, avoid this by using the syntactic scope of the qual clause; with the exception that if we're in the top-level join domain we can still use the full query relid set, ensuring the resulting gating Result node goes to the top of the plan. (This is approximately as smart as the pre-v16 code was. Perhaps we can do better later, but it's not clear that such cases are worth a lot of sweat.) In process_implied_equality, we don't have a clear notion of syntactic scope, but we do have the results of SpecialJoinInfo construction. Thumb through those and remove any lower outer joins that might get commuted to above the join domain. Again, we can make an exception for the top-level join domain. It'd be possible to work harder here (for example, by keeping outer joins that aren't shown as potentially commutable), but I'm going to stop here for the moment. This issue has convinced me that the current representation of join domains probably needs further refinement, so I'm disinclined to write inessential dependent logic just yet. In passing, tighten the qualscope passed to process_implied_equality by generate_base_implied_equalities_no_const; there's no need for it to be larger than the rel we are currently considering. Tom Lane and Richard Guo, per report from Tender Wang. Discussion: https://postgr.es/m/CAHewXNk9eJ35ru5xATWioTV4+xZPHptjy9etdcNPjUfY9RQ+uQ@mail.gmail.com
* Fix snapshot handling in logicalmsg_decodeTomas Vondra2023-02-22
| | | | | | | | | | | | | | | | | Whe decoding a transactional logical message, logicalmsg_decode called SnapBuildGetOrBuildSnapshot. But we may not have a consistent snapshot yet at that point. We don't actually need the snapshot in this case (during replay we'll have the snapshot from the transaction), so in practice this is harmless. But in assert-enabled build this crashes. Fixed by requesting the snapshot only in non-transactional case, where we are guaranteed to have SNAPBUILD_CONSISTENT. Backpatch to 11. The issue exists since 9.6. Backpatch-through: 11 Reviewed-by: Andres Freund Discussion: https://postgr.es/m/84d60912-6eab-9b84-5de3-41765a5449e8@enterprisedb.com
* Add missing support for the latest SPI status codes.Dean Rasheed2023-02-22
| | | | | | | | | | | | | | | | | | | SPI_result_code_string() was missing support for SPI_OK_TD_REGISTER, and in v15 and later, it was missing support for SPI_OK_MERGE, as was pltcl_process_SPI_result(). The last of those would trigger an error if a MERGE was executed from PL/Tcl. The others seem fairly innocuous, but worth fixing. Back-patch to all supported branches. Before v15, this is just adding SPI_OK_TD_REGISTER to SPI_result_code_string(), which is unlikely to be seen by anyone, but seems worth doing for completeness. Reviewed by Tom Lane. Discussion: https://postgr.es/m/CAEZATCUg8V%2BK%2BGcafOPqymxk84Y_prXgfe64PDoopjLFH6Z0Aw%40mail.gmail.com https://postgr.es/m/CAEZATCUMe%2B_KedPMM9AxKqm%3DSZogSxjUcrMe%2BsakusZh3BFcQw%40mail.gmail.com
* Fix Assert failure for MERGE into a partitioned table with RLS.Dean Rasheed2023-02-22
| | | | | | | | | | | In ExecInitPartitionInfo(), the Assert when building the WITH CHECK OPTION list for the new partition assumed that the command would be an INSERT or UPDATE, but it can also be a MERGE. This can be triggered by a MERGE into a partitioned table with RLS checks to enforce. Fix, and back-patch to v15, where MERGE was introduced. Discussion: https://postgr.es/m/CAEZATCWWFtQmW67F3XTyMU5Am10Oxa_b8oe0x%2BNu5Mo%2BCdRErg%40mail.gmail.com
* Remove newly added asserts from pg_bitutils.hJohn Naylor2023-02-22
| | | | | | | | | | These were valuable during development, but are unlikely to tell us anything going forward. This reverts 204b0cbec and adjusts the content of 677319746 to more closely match the more-readable original style. Per review from Tom Lane Discussion: https://www.postgresql.org/message-id/3567481.1676906261%40sss.pgh.pa.us
* Fix MERGE command tag for cross-partition updates.Dean Rasheed2023-02-22
| | | | | | | | | | | This ensures that the row count in the command tag for a MERGE is correctly computed. Previously, if MERGE updated a partitioned table, the row count would be incorrect if any row was moved to a different partition, since such updates were counted twice. Back-patch to v15, where MERGE was introduced. Discussion: https://postgr.es/m/CAEZATCWRMG7XX2QEsVL1LswmNo2d_YG8tKTLkpD3=Lp644S7rg@mail.gmail.com