aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* pg_dump: Remove some dead codePeter Eisentraut2023-02-22
| | | | | | | | | Client-side tracking of atttypmod has been unused since 64f3524, when server-side format_type() started being used exclusively. So remove this dead code. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/144be239-c893-9361-704f-ac85b5b98d1a%40enterprisedb.com
* Fix small memory leak in psql's \bind commandMichael Paquier2023-02-22
| | | | | | | | | | | | psql_scan_slash_option() returns a malloc()'d result through a PQExpBuffer, and exec_command_bind() was doing an extra allocation of this option for no effect. Introduced in 5b66de3. Author: Kyotaro Horiguchi Reviewed-by: Corey Huinker Discussion: https://postgr.es/m/20230221.115555.89096938631423206.horikyota.ntt@gmail.com
* Fix corruption of templates after CREATE DATABASE .. STRATEGY WAL_LOGMichael Paquier2023-02-22
| | | | | | | | | | | | | | | | | | | | | | | | | | WAL_LOG does a scan of the template's pg_class to determine the set of relations that need to be copied from a template database to the new one. However, as coded in 9c08aea, this copy strategy would load the pages of pg_class without considering it as a permanent relation, causing the loaded pages to never be flushed when they should. Any modification of the template's pg_class, mostly through DDLs, would then be missed, causing corruptions. STRATEGY = WAL_LOG is the default over FILE_COPY since it has been introduced, so any changes done to pg_class on a database template would be gone. Updates of database templates should be a rare thing, so the impact of this bug should be hopefully limited. The pre-14 default strategy FILE_COPY is safe, and can be used as a workaround. Ryo Matsumura has found and analyzed the issue, and Nathan has written a test able to reproduce the failure (with few tweaks from me). Backpatch down to 15, where STRATEGY = WAL_LOG has been introduced. Author: Nathan Bossart, Ryo Matsumura Reviewed-by: Dilip Kumar, Michael Paquier Discussion: https://postgr.es/m/TYCPR01MB6868677E499C9AD5123084B5E8A39@TYCPR01MB6868.jpnprd01.prod.outlook.com Backpatch-through: 15
* Fix erroneous Valgrind markings in AllocSetRealloc.Tom Lane2023-02-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If asked to decrease the size of a large (>8K) palloc chunk, AllocSetRealloc could improperly change the Valgrind state of memory beyond the new end of the chunk: it would mark data UNDEFINED as far as the old end of the chunk after having done the realloc(3) call, thus tromping on the state of memory that no longer belongs to it. One would normally expect that memory to now be marked NOACCESS, so that this mislabeling might prevent detection of later errors. If realloc() had chosen to move the chunk someplace else (unlikely, but well within its rights) we could also mismark perfectly-valid DEFINED data as UNDEFINED, causing false-positive valgrind reports later. Also, any malloc bookkeeping data placed within this area might now be wrongly marked, causing additional problems. Fix by replacing relevant uses of "oldsize" with "Min(size, oldsize)". It's sufficient to mark as far as "size" when that's smaller, because whatever remains in the new chunk size will be marked NOACCESS below, and we expect realloc() to have taken care of marking the memory beyond the new official end of the chunk. While we're here, also rename the function's "oldsize" variable to "oldchksize" to more clearly explain what it actually holds, namely the distance to the end of the chunk (that is, requested size plus trailing padding). This is more consistent with the use of "size" and "chksize" to hold the new requested size and chunk size. Add a new variable "oldsize" in the one stanza where we're actually talking about the old requested size. Oversight in commit c477f3e44. Back-patch to all supported branches, as that was, just in case anybody wants to do valgrind testing on back branches. Karina Litskevich Discussion: https://postgr.es/m/CACiT8iaAET-fmzjjZLjaJC4zwSJmrFyL7LAdHwaYyjjQOQ4hcg@mail.gmail.com
* Remove obsolete coding for early macOS.Thomas Munro2023-02-22
| | | | | | | | | | | | | | | Commits 04cad8f7 and 0c088568 supported old macOS systems that didn't define O_CLOEXEC or O_DSYNC yet, but those arrived in macOS releases 10.7 and 10.6 (respectively), which themselves reached EOL around a decade ago. We've already made use of other POSIX features that early macOS vintages can't compile (for example commits 623cc673, d2e15083). A later commit will use O_CLOEXEC on POSIX systems so it would be strange to pretend here that it's optional, and we might as well give O_DSYNC the same treatment since the reference is also guarded by a test for a macOS-specific macro, and we know that current Macs have it. Discussion: https://postgr.es/m/CA%2BhUKGKb6FsAdQWcRL35KJsftv%2B9zXqQbzwkfRf1i0J2e57%2BhQ%40mail.gmail.com
* pgbench: Prepare commands in pipelines in advanceAlvaro Herrera2023-02-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | Failing to do so results in an error when a pgbench script tries to start a serializable transaction inside a pipeline, because by the time BEGIN ISOLATION LEVEL SERIALIZABLE is executed, we're already in a transaction that has acquired a snapshot, so the server rightfully complains. We can work around that by preparing all commands in the pipeline before actually starting the pipeline. This changes the existing code in two aspects: first, we now prepare each command individually at the point where that command is about to be executed; previously, we would prepare all commands in a script as soon as the first command of that script would be executed. It's hard to see that this would make much of a difference (particularly since it only affects the first time to execute each script in a client), but I didn't actually try to measure it. Secondly, we no longer use PQsendPrepare() in pipeline mode, but only PQprepare. There's no specific reason for this change other than no longer needing to do differently in pipeline mode. (Previously we had no choice, because in pipeline mode PQprepare could not be used.) Backpatch to 14, where pgbench got support for pipeline mode. Reported-by: Yugo NAGATA <nagata@sraoss.co.jp> Discussion: https://postgr.es/m/20210716153013.fc53b1c780b06fccc07a7f0d@sraoss.co.jp
* Detect overflow in timestamp[tz] subtraction.Tom Lane2023-02-20
| | | | | | | | | | It's possible to overflow the int64 microseconds field of the output interval when subtracting two timestamps. Detect that instead of silently returning a bogus result. Nick Babadzhanian Discussion: https://postgr.es/m/CABw73Uq2oJ3E+kYvvDuY04EkhhkChim2e-PaghBDjOmgUAMWGw@mail.gmail.com
* Fix parsing of ISO-8601 interval fields with exponential notation.Tom Lane2023-02-20
| | | | | | | | | | | | | | | | | | | | | | | | Historically we've accepted interval input like 'P.1e10D'. This is probably an accident of having used strtod() to do the parsing, rather than something anyone intended, but it's been that way for a long time. Commit e39f99046 broke this by trying to parse the integer and fractional parts separately, without accounting for the possibility of an exponent. In principle that coding allowed for precise conversions of field values wider than 15 decimal digits, but that does not seem like a goal worth sweating bullets for. So, rather than trying to manage an exponent on top of the existing complexity, let's just revert to the previous coding that used strtod() by itself. We can still improve on the old code to the extent of allowing the value to range up to 1.0e15 rather than only INT_MAX. (Allowing more than that risks creating problems due to precision loss: the converted fractional part might have absolute value more than 1. Perhaps that could be dealt with in some way, but it really does not seem worth additional effort.) Per bug #17795 from Alexander Lakhin. Back-patch to v15 where the faulty code came in. Discussion: https://postgr.es/m/17795-748d6db3ed95d313@postgresql.org
* Prevent join removal from removing the query's result relation.Tom Lane2023-02-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This was not something that required consideration before MERGE was invented; but MERGE builds a join tree that left-joins to the result relation, meaning that remove_useless_joins will consider removing it. That should generally be stopped by the query's use of output variables from the result relation. However, if the result relation is inherited (e.g. a partitioned table) then we don't add any row identity variables to the query until expand_inherited_rtentry, which happens after join removal. This was exposed as of commit 3c569049b, which made it possible to deduce that a partitioned table could contain at most one row matching a join key, enabling removal of the not-yet-expanded result relation. Ooops. To fix, let's just teach join_is_removable that the query result rel is never removable. It's a cheap enough test in any case, and it'll save some cycles that we'd otherwise expend in proving that it's not removable, even in the cases we got right. Back-patch to v15 where MERGE was added. Although I think the case cannot be reached in v15, this seems like cheap insurance. Per investigation of a report from Alexander Lakhin. Discussion: https://postgr.es/m/36bee393-b351-16ac-93b2-d46d83637e45@gmail.com
* Remove gratuitous assumptions about what make_modifytable can see.Tom Lane2023-02-20
| | | | | | | | | | | | | | | For no clearly good reason, make_modifytable assumed that it could not reach its get-the-FDW-info-the-hard-way path in MERGE. It's currently possible to demonstrate that assertion failing, which seems to be due to an upstream planner bug; but there's no good reason to do it like this at all. Let's apply the principle of separation of concerns and make the MERGE check separately, after getting or not getting the fdwroutine pointer. Per report from Alexander Lakhin. No test case, since I think the potential test condition will go away soon. Discussion: https://postgr.es/m/36bee393-b351-16ac-93b2-d46d83637e45@gmail.com
* Correctly set userid of subquery relations' child relsAlvaro Herrera2023-02-20
| | | | | | | | | | | | | | | | | The RelOptInfo->userid field (the user ID to check permissions as) of an "otherrel" relation was being copied from its parent relation, which is correct in most cases but wrong when the parent is a subquery. In that case, using the value from the RTEPermissionInfo of the child itself is the appropriate thing to do. Coming up with a test case where user-visible behavior changes proves hard enough, so we don't add one here. Bug introduced by a61b1f74823c, discovered by Amit while reviewing nearby code. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqE0WY_AhLnGtTsY7eYebG212XWbM-D8gr2A_ToOHyCywQ@mail.gmail.com
* Optimize generate_orderedappend_pathsDavid Rowley2023-02-20
| | | | | | | | | | | | | | | | | | In generate_orderedappend_paths(), when match_partition_order_desc was true, we would lcons() items to various lists in a loop over each live partition. When the number of live partitions was large, the lcons() could show up in profiles due to it having to perform memmove() to make way for the new list item. Here we adjust things so that we just perform the loop over the live partitions backwards when match_partition_order_desc is true. This allows us to simplify the logic in the loop. Now, as far as the guts of the loop knows, there's no difference between match_partition_order and match_partition_order_desc. We can just set match_partition_order to true so that we build the correct list of paths for the asc and desc case. Per idea from Andres Freund. Discussion: https://postgr.es/m/20230217002351.nyt4y5tdzg6hugdt@awork3.anarazel.de
* Add MSVC support for pg_leftmost_one_pos32() and friendsJohn Naylor2023-02-20
| | | | | | | | | | | | | | | To allow testing for general support for fast bitscan intrinsics, add symbols HAVE_BITSCAN_REVERSE and HAVE_BITSCAN_FORWARD. Also do related cleanup in AllocSetFreeIndex(): Previously, we tested for HAVE__BUILTIN_CLZ and copied the relevant internals of pg_leftmost_one_pos32(), with a special fallback that does less work than the general fallback for that function. Now that we have a more general test, we just call pg_leftmost_one_pos32() directly for platforms with intrinsic support. On gcc at least, there is no difference in the binary for non-assert builds. Discussion: https://www.postgresql.org/message-id/CAFBsxsEPc%2BFnX_0vmmQ5DHv60sk4rL_RZJ%2BMD6ei%3D76L0kFMvA%40mail.gmail.com
* Add assert checking to pg_leftmost_one_pos32() and friendsJohn Naylor2023-02-20
| | | | Discussion: https://www.postgresql.org/message-id/CAFBsxsEPc%2BFnX_0vmmQ5DHv60sk4rL_RZJ%2BMD6ei%3D76L0kFMvA%40mail.gmail.com
* Speedup and increase usability of set proc title functionsDavid Rowley2023-02-20
| | | | | | | | | | | | | | | | | | | | | | The setting of the process title could be seen on profiles of very fast-to-execute queries. In many locations where we call set_ps_display() we pass along a string constant, the length of which is known during compilation. Here we effectively rename set_ps_display() to set_ps_display_with_len() and then add a static inline function named set_ps_display() which calls strlen() on the given string. This allows the compiler to optimize away the strlen() call when dealing with call sites passing a string constant. We can then also use memcpy() instead of strlcpy() to copy the string into the destination buffer. That's significantly faster than strlcpy's byte-at-a-time way of copying. Here we also take measures to improve some code which was adjusting the process title to add a " waiting" suffix to it. Call sites which require this can now just call set_ps_display_suffix() to add or adjust the suffix and call set_ps_display_remove_suffix() to remove it again. Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CAApHDvocBvvk-0gWNA2Gohe+sv9fMcv+fK_G+siBKJrgDG4O7g@mail.gmail.com
* Fix handling of multi-column BRIN indexesTomas Vondra2023-02-19
| | | | | | | | | | | | | | When evaluating clauses on multiple scan keys of a multi-column BRIN index, we can stop processing as soon as we find a scan key eliminating the range, and the range should not be added to tbe bitmap. That's how it worked before 14, but since a681e3c107a the code treated the range as matching if it matched at least the last scan key. Backpatch to 14, where this code was introduced. Backpatch-through: 14 Discussion: https://postgr.es/m/ebc18613-125e-60df-7520-fcbe0f9274fc%40enterprisedb.com
* Print the correct aliases for DML target tables in ruleutils.Tom Lane2023-02-17
| | | | | | | | | | | | | | | | | | | | | | | | | ruleutils.c blindly printed the user-given alias (or nothing if there hadn't been one) for the target table of INSERT/UPDATE/DELETE queries. That works a large percentage of the time, but not always: for queries appearing in WITH, it's possible that we chose a different alias to avoid conflict with outer-scope names. Since the chosen alias would be used in any Var references to the target table, this'd lead to an inconsistent printout with consequences such as dump/restore failures. The correct logic for printing (or not) a relation alias was embedded in get_from_clause_item. Factor it out to a separate function so that we don't need a jointree node to use it. (Only a limited part of that function can be reached from these new call sites, but this seems like the cleanest non-duplicative factorization.) In passing, I got rid of a redundant "\d+ rules_src" step in rules.sql. Initial report from Jonathan Katz; thanks to Vignesh C for analysis. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/e947fa21-24b2-f922-375a-d4f763ef3e4b@postgresql.org Discussion: https://postgr.es/m/CALDaNm1MMntjmT_NJGp-Z=xbF02qHGAyuSHfYHias3TqQbPF2w@mail.gmail.com
* Fix incorrect format placeholderPeter Eisentraut2023-02-17
|
* Redesign archive modulesMichael Paquier2023-02-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A new callback named startup_cb, called shortly after a module is loaded, is added. This makes possible the initialization of any additional state data required by a module. This initial state data can be saved in a ArchiveModuleState, that is now passed down to all the callbacks that can be defined in a module. With this design, it is possible to have a per-module state, aimed at opening the door to the support of more than one archive module. The initialization of the callbacks is changed so as _PG_archive_module_init() does not anymore give in input a ArchiveModuleCallbacks that a module has to fill in with callback definitions. Instead, a module now needs to return a const ArchiveModuleCallbacks. All the structure and callback definitions of archive modules are moved into their own header, named archive_module.h, from pgarch.h. Command-based archiving follows the same line, with a new set of files named shell_archive.{c,h}. There are a few more items that are under discussion to improve the design of archive modules, like the fact that basic_archive calls sigsetjmp() by itself to define its own error handling flow. These will be adjusted later, the changes done here cover already a good portion of what has been discussed. Any modules created for v15 will need to be adjusted to this new design. Author: Nathan Bossart Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20230130194810.6fztfgbn32e7qarj@awork3.anarazel.de
* Remove obsolete platforms from ps_status.c.Thomas Munro2023-02-17
| | | | | | | | Time to remove various code, comments and configure/meson probes relating to ancient BSD, SunOS, GNU/Hurd, IRIX, NeXT and Unixware. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGJMNGUAqf27WbckYFrM-Mavy0RKJvocfJU%3DJ2XcAZyv%2Bw%40mail.gmail.com
* Fix check for child column generation status matching parent.Tom Lane2023-02-16
| | | | | | | | | | | | | | | | | In commit 8bf6ec3ba, I mistakenly supposed that MergeAttributes' loop over saved_schema was reprocessing column definitions that had already been checked earlier: there is a variant syntax for creating a child partition in which that's not true. So we need to duplicate the full check appearing further up. (Actually, I believe that the "if (restdef->identity)" part is not reachable, because we reject identity on partitions earlier. But it seems wise to keep the check, in case that's ever relaxed, and to keep this code in sync with the other instance.) Per report from Alexander Lakhin. Discussion: https://postgr.es/m/4a8200ca-8378-653e-38ed-b2e1f1611aa6@gmail.com
* pgindent: mention directory arguments in help textAndrew Dunstan2023-02-16
| | | | Shi Yu
* Remove duplicated comment in nodeModifyTable.cMichael Paquier2023-02-16
| | | | | Author: Amul Sul Discussion: https://postgr.es/m/CAAJ_b97badUU8_DHNoFCXZxF6YUk0Yb=53rrum168hd1haJgpQ@mail.gmail.com
* Add a new wait state and use it when sending data in the apply worker.Amit Kapila2023-02-16
| | | | | | | | | | | | | d9d7fe68d3 made use of an existing wait event when sending data from the apply worker, but we should have invented a new wait event since this is a new place to wait. This patch corrects the mistake by using a new wait event "LogicalApplySendData". Author: Hou Zhijie Reviewed-by: Peter Smith Discussion: https://postgr.es/m/CA+TgmobWzbr9H3yN3dLVckviEZKemPwd+XyCFKEgyZQZhgP66Q@mail.gmail.com
* Add description for new patterns supported in HBA and ident sample filesMichael Paquier2023-02-16
| | | | | | | | | | | | | | Support for regexps in database and role entries for pg_hba.conf has been added in 8fea8683, and efb6f4a has extended support of pg-user in pg_ident.conf, still both of them have missed a short description about the new patterns supported in their respective sample files. This commit closes the gap, by providing a short description of all the new features supported for each entry type. Reported-by: Pavel Luzanov Reviewed-by: Jelte Fennema, Pavel Luzanov Discussion: https://postgr.es/m/e495112d-8741-e651-64a2-ecb5728f1a56@postgrespro.ru
* Don't rely on uninitialized value in MERGE / DELETEAlvaro Herrera2023-02-15
| | | | | | | | | | | | On MERGE / WHEN MATCHED DELETE it's not possible to get cross-partition updates, so we don't initialize cpUpdateRetrySlot; however, the code was not careful to ignore the value in that case. Make it do so. Backpatch to 15. Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://postgr.es/m/17792-0f89452029662c36@postgresql.org
* Rename force_parallel_mode to debug_parallel_queryDavid Rowley2023-02-15
| | | | | | | | | | | | | | | | | | | | force_parallel_mode is meant to be used to allow us to exercise the parallel query infrastructure to ensure that it's working as we expect. It seems some users think this GUC is for forcing the query planner into picking a parallel plan regardless of the costs. A quick look at the documentation would have made them realize that they were wrong, but the GUC is likely too conveniently named which, evidently, seems to often result in users expecting that it forces the planner into usefully parallelizing queries. Here we rename the GUC to something which casual users are less likely to mistakenly think is what they need to make their query run more quickly. For now, the old name can still be used. We'll revisit if the old name mapping can be removed once the buildfarm configs are all updated. Reviewed-by: John Naylor Discussion: https://postgr.es/m/CAApHDvrsOi92_uA7PEaHZMH-S4Xv+MGhQWA+GrP8b1kjpS1HjQ@mail.gmail.com
* Fix handling of SCRAM-SHA-256's channel binding with RSA-PSS certificatesMichael Paquier2023-02-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | OpenSSL 1.1.1 and newer versions have added support for RSA-PSS certificates, which requires the use of a specific routine in OpenSSL to determine which hash function to use when compiling it when using channel binding in SCRAM-SHA-256. X509_get_signature_nid(), that is the original routine the channel binding code has relied on, is not able to determine which hash algorithm to use for such certificates. However, X509_get_signature_info(), new to OpenSSL 1.1.1, is able to do it. This commit switches the channel binding logic to rely on X509_get_signature_info() over X509_get_signature_nid(), which would be the choice when building with 1.1.1 or newer. The error could have been triggered on the client or the server, hence libpq and the backend need to have their related code paths patched. Note that attempting to load an RSA-PSS certificate with OpenSSL 1.1.0 or older leads to a failure due to an unsupported algorithm. The discovery of relying on X509_get_signature_info() comes from Jacob, the tests have been written by Heikki (with few tweaks from me), while I have bundled the whole together while adding the bits needed for MSVC and meson. This issue exists since channel binding exists, so backpatch all the way down. Some tests are added in 15~, triggered if compiling with OpenSSL 1.1.1 or newer, where the certificate and key files can easily be generated for RSA-PSS. Reported-by: Gunnar "Nick" Bluth Author: Jacob Champion, Heikki Linnakangas Discussion: https://postgr.es/m/17760-b6c61e752ec07060@postgresql.org Backpatch-through: 11
* Fix make_etags failure on Mac.Tatsuo Ishii2023-02-15
| | | | | | | | | | | | | | | | | Previously make_etags always ran make_ctags -e when make_etags was executed. However, because non-Exuberant ctags on Mac does not support -e option (and also on other platforms including old Linux), ctags failed. To avoid the failure change make_ctags so that if non-Exuberant ctags is used and ctags -e option is requested, run etags command instead. If etags command does not exist, make_ctags will fail. Also refactor make_ctags and tweak make_etags to emit proper usage message. Author: Fujii Masao Reviewed-by: Tatsuo Ishii Discussion: https://www.postgresql.org/message-id/369c13b9-8b0f-d6f9-58fc-61258ec8f713%40oss.nttdata.com
* Change argument type of pq_sendbytes from char * to void *Peter Eisentraut2023-02-14
| | | | | | | | This is a follow-up to 1f605b82ba66ece8b421b10d41094dd2e3c0c48b. It allows getting rid of further casts at call sites. Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://www.postgresql.org/message-id/783a4edb-84f9-6df2-7470-2ef5ccc6607a@enterprisedb.com
* When removing a relation from the query, drop its RelOptInfo.Tom Lane2023-02-13
| | | | | | | | | | | | | | | In commit b78f6264e I opined that it was "too risky" to delete a relation's RelOptInfo from the planner's data structures when we have realized that we don't need to join to it; so instead we just marked it as a dead relation. In hindsight that judgment seems flawed: any subsequent access to such a dead relation is arguably a bug in itself, so leaving the RelOptInfo present just helps to mask bugs. Let's delete it instead, allowing removal of the whole notion of a "dead relation". So far as the regression tests can find, this requires no other code changes, except for one Assert in equivclass.c that was very dubiously not complaining about access to a dead rel. Discussion: https://postgr.es/m/229905.1676062220@sss.pgh.pa.us
* Fix buggy recursion in flatten_rtes_walker().Tom Lane2023-02-13
| | | | | | | | | | Must save-and-restore the context we are modifying. Oversight in commit a61b1f748. Tender Wang Discussion: https://postgr.es/m/CAHewXNnnNySD_YcKNuFpQDV2gxWA7_YLWqHmYVcyoOYxn8kY2A@mail.gmail.com Discussion: https://postgr.es/m/20230212233711.GA1316@telsasoft.com
* Fix thinkos in have_unsafe_outer_join_ref; reduce to Assert check.Tom Lane2023-02-13
| | | | | | | | | | | | | | | | | Late in the development of commit 2489d76c4, I (tgl) incorrectly concluded that the new function have_unsafe_outer_join_ref couldn't ever reach its inner loop. That should be the case if the inner rel's parameterization is based on just one Var, but it could be based on Vars from several relations, and then not only is the inner loop reachable but it's wrongly coded. Despite those errors, it still appears that the whole thing is redundant given previous join_is_legal checks, so let's arrange to only run it in assert-enabled builds. Diagnosis and patch by Richard Guo, per fuzz testing by Justin Pryzby. Discussion: https://postgr.es/m/20230212235823.GW1653@telsasoft.com
* Remove obsolete pgindent options --code-base and --buildAndrew Dunstan2023-02-13
| | | | | | | | | | | | | | | | | | | | Now that we have the sources for pg_bsd_indent in our code base these are redundant. It is now required to provide a list of files or directories to pgindent, either by using --commit or on the command line. The equivalent of previously running pgindent with no parameters is now `pgindent .` Some extra checks are also added. duplicate files in the file list are skipped, and there is a warning if no files are specified. If the --commit option is used, the script now chdir's to the source root, as git always reports files relative to that. (Fixes a gripe from Justin Pryzby) Reviewed by Tom Lane Discussion: https://postgr.es/m/842819.1676219054@sss.pgh.pa.us
* Fix object identity string for transformsAlvaro Herrera2023-02-13
| | | | | | | | | | | | In commit ad89a5d115b3, we added an unhelpful 'ON' that doesn't match the input syntax. This was discovered while adding code to support for DDL in logical replication. No backpatch because of the change of behavior, however improbable it may be that somebody is depending on this. Author: Zheng Li <zhengli10@gmail.com> Discussion: https://postgr.es/m/CAAD30UKg8rXeGM8Oy_MAmxKBL_K5DiHXdeNF=hUefcu1C_6VfQ@mail.gmail.com
* Add wait_for_replay_catchup wrapper to Cluster.pmAlvaro Herrera2023-02-13
| | | | | | | This simplifies a few lines of Perl test code a bit. Author: Bertrand Drouvot Discussion: https://postgr.es/m/846724b5-0723-f4c2-8b13-75301ec7509e@gmail.com
* Fix pfree issue in presorted DISTINCT aggregate codeDavid Rowley2023-02-13
| | | | | | | | | | | The logic in this area was recently changed in 7da51590e, however, in that commit, I neglected to consider that the conditions in which we should pfree the old Datum needed to be updated after that change. This could result in trying to pfree a NULL value, as was demonstrated by Alexander Lakhin. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/4103db46-d888-6d1d-e88d-87c21ed99472@gmail.com
* Consolidate ItemPointer to Datum conversion functionsPeter Eisentraut2023-02-13
| | | | | | | | | Instead of defining the same set of macros several times, define it once in an appropriate header file. In passing, convert to inline functions. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/844dd4c5-e5a1-3df1-bfaf-d1e1c2a16e45%40enterprisedb.com
* Fix incorrect presorted DISTINCT aggregate if conditionDavid Rowley2023-02-13
| | | | | | | | | | | | | | | | | | | | | Here we fix a faulty "if" condition which failed to correctly handle two or more consecutive NULL transition values when checking if the new value is DISTINCT from the old value for presorted aggregates. Given a suitably non-strict aggregate transition function, a byref aggregate could cause a crash due to calling the type's equality function and passing along a (Datum) 0 value to test for equality, the equality function would then try to dereference that 0 Datum and segfault. For byval types, there'd have been no crash and the equality function would have seen that the two 0 Datums matched, which (only by chance) meant the calling code would have worked correctly. Here we ensure that we only call the equality function when neither of the input values are NULL. This code is all new as of 1349d2790, so no backpatch needed. Reported-by: Fujii Masao Discussion: https://postgr.es/m/860c6d6f-a3c5-3ae9-9da2-827177bede06@oss.nttdata.com
* Disable WindowAgg inverse transitions when subplans are presentDavid Rowley2023-02-13
| | | | | | | | | | | | | | | | | | | | | | | | | When an aggregate function is used as a WindowFunc and a tuple transitions out of the window frame, we ordinarily try to make use of the aggregate function's inverse transition function to "unaggregate" the exiting tuple. This optimization is disabled for various cases, including when the aggregate contains a volatile function. In such a case we'd be unable to ensure that the transition value was calculated to the same value during transitions and inverse transitions. Unfortunately, we did this check by calling contain_volatile_functions() which does not recursively search SubPlans for volatile functions. If the aggregate function's arguments or its FILTER clause contained a subplan with volatile functions then we'd fail to notice this. Here we fix this by just disabling the optimization when the WindowFunc contains any subplans. Volatile functions are not the only reason that a subplan may have nonrepeatable results. Bug: #17777 Reported-by: Anban Company Discussion: https://postgr.es/m/17777-860b739b6efde977%40postgresql.org Reviewed-by: Tom Lane Backpatch-through: 11
* Mark more nodes with attribute no_query_jumbleMichael Paquier2023-02-13
| | | | | | | | | | | | | | | | | | | | | | | | | | This commit removes most of the Plan and Path nodes, which should never be included in the query jumbling because we ignore these in Query nodes. This is facilitated by making no_query_jumble an inherited attribute, like no_copy, no_equal and no_read when the supertype of a node is found as marked with that. RawStmt is not used in parsed queries, so it can be removed from the query jumbling. A couple of nodes defined in pathnodes.h, plannodes.h and primnodes.h with NodeTag as supertype need to be marked individually. Forcing the execution of the query jumbling code with compute_query_id = auto while pg_stat_statements is loaded brings the code coverage of queryjumblefuncs.funcs.c to 95.6%. The core code does not yet include a way to enforce the execution in query jumbling except in pg_stat_statements, so the numbers I am mentioning above will not reflect on the default coverage report with just what is done in this commit. Reported-by: Tom Lane Reviewed-by: Tom Lane Discussion: https://postgr.es/m/3344827.1675809127@sss.pgh.pa.us
* Make pg_bsd_indent's .h files inclusion-order-safe.Tom Lane2023-02-12
| | | | As-is, they failed headerscheck. Per buildfarm.
* Avoid dereferencing an undefined pointer in DecodeInterval().Tom Lane2023-02-12
| | | | | | | | | | | | | | | Commit e39f99046 moved some code up closer to the start of DecodeInterval(), without noticing that it had been implicitly relying on previous checks to reject the case of empty input. Given empty input, we'd now dereference a pointer that hadn't been set, possibly leading to a core dump. (But if we fail to provoke a SIGSEGV, nothing bad happens, and the expected syntax error is thrown a bit later.) Per bug #17788 from Alexander Lakhin. Back-patch to v15 where the fault was introduced. Discussion: https://postgr.es/m/17788-dabac9f98f7eafd5@postgresql.org
* Integrate pg_bsd_indent into our build/test infrastructure.Tom Lane2023-02-12
| | | | | | | | | | | | | | | | | | | | | Update the Makefile and build directions for in-tree build, and add Meson build infrastructure. Also convert the ad-hoc test target into a TAP test. Currently, the Make build system will not build pg_bsd_indent by default, while the Meson system will. Both will test it during "make check-world" or "ninja test". Neither will install it automatically. (We might change some of these decisions later.) Also fix a few portability nits noted during early testing. Also, exclude pg_bsd_indent from pgindent's purview; at least for now, we'll leave it formatted similarly to the FreeBSD original. Tom Lane and Andres Freund Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us Discussion: https://postgr.es/m/20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de
* Sync pg_bsd_indent's copyright notices with Postgres practice.Tom Lane2023-02-12
| | | | | | | | | To avoid confusion, make the copyright notices in these files match the 3-clause form of the BSD license, per the blanket policy update that UC Berkeley issued years ago. Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us Discussion: https://postgr.es/m/20230123011002.fzcaa3krql3mqsfn@awork3.anarazel.de
* Import pg_bsd_indent sources.Tom Lane2023-02-12
| | | | | | | | This brings in an exact copy of the pg_bsd_indent repo as of commit d301442799cea44e5ccb04331afc537764ec77c5 (2020-12-28). Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us Discussion: https://postgr.es/m/20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de
* pgindent: filter files for the --commit optionAndrew Dunstan2023-02-12
| | | | | | | | | | | per gripe from Shi Yu, solution from Jelte Fennema Also add a check that the file exists, and issue a warning if it doesn't. As an efficiency measure, avoid processing any file more than once. Discussion: https://postgr.es/m/TYAPR01MB6315B86619944D4A6B56842DFDDE9@TYAPR01MB6315.jpnprd01.prod.outlook.com
* Add tests for pg_stat_ioAndres Freund2023-02-11
| | | | | | | Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de
* Create regress_tblspc in test_setupAndres Freund2023-02-11
| | | | | | | | | | | | | | An upcoming test needs to use a tablespace as part of its test. Historically, we wanted tablespace creation be done in a dedicated file, so it's easy to disable when testing replication. But that is not necessary anymore, due to allow_in_place_tablespaces. Create regress_tblspace tablespace in test_setup. Move the tablespace test to the end of the parallel schedule, so other tests can use it. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de
* Add pg_stat_io view, providing more detailed IO statisticsAndres Freund2023-02-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Builds on 28e626bde00 and f30d62c2fc6. See the former for motivation. Rows of the view show IO operations for a particular backend type, IO target object, IO context combination (e.g. a client backend's operations on permanent relations in shared buffers) and each column in the view is the total number of IO Operations done (e.g. writes). So a cell in the view would be, for example, the number of blocks of relation data written from shared buffers by client backends since the last stats reset. In anticipation of tracking WAL IO and non-block-oriented IO (such as temporary file IO), the "op_bytes" column specifies the unit of the "reads", "writes", and "extends" columns for a given row. Rows for combinations of IO operation, backend type, target object and context that never occur, are ommitted entirely. For example, checkpointer will never operate on temporary relations. Similarly, if an IO operation never occurs for such a combination, the IO operation's cell will be null, to distinguish from 0 observed IO operations. For example, bgwriter should not perform reads. Note that some of the cells in the view are redundant with fields in pg_stat_bgwriter (e.g. buffers_backend). For now, these have been kept for backwards compatibility. Bumps catversion. Author: Melanie Plageman <melanieplageman@gmail.com> Author: Samay Sharma <smilingsamay@gmail.com> Reviewed-by: Maciek Sakrejda <m.sakrejda@gmail.com> Reviewed-by: Lukas Fittl <lukas@fittl.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20200124195226.lth52iydq2n2uilq@alap3.anarazel.de