aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Remove extra space from dumped ALTER DEFAULT PRIVILEGES.Jeff Davis2022-12-12
| | | | | Author: Nathan Bossart Discussion: https://postgr.es/m/20221206232744.GA3560301@nathanxps13
* Fix failure to advance content pointer in sendFileWithContent.Robert Haas2022-12-12
| | | | | | | | | | | | | | | | | | | If sendFileWithContent were used to send a file larger than the bbsink buffer size, this would result in corruption. The only files that are sent via sendFileWithContent are the backup label file, the tablespace map file, and .done files for WAL segments included in the backup. Of these, it seems that only the tablespace_map file can become large enough to cause a problem, and then only if you have a lot of tablespaces. If you do have that situation, you might end up with a corrupted tablespace_map file, which would be bad. My commit bef47ff85df18bf4a3a9b13bd2a54820e27f3614 introduced this problem. Report and patch by Antonin Houska. Discussion: http://postgr.es/m/15764.1670528645@antos
* Order getopt argumentsPeter Eisentraut2022-12-12
| | | | | | | | | | Order the letters in the arguments of getopt() and getopt_long(), as well as in the subsequent switch statements. In most cases, I used alphabetical with lower case first. In a few cases, existing different orders (e.g., upper case first) was kept to reduce the diff size. Discussion: https://www.postgresql.org/message-id/flat/3efd0fe8-351b-f836-9122-886002602357%40enterprisedb.com
* Get rid of recursion-marker values in enum AlterTableTypeAlvaro Herrera2022-12-12
| | | | | | | | | | | | | | | During ALTER TABLE execution, when prep-time handling of subcommands of certain types determine that execution-time handling requires recursion, they signal this by changing the subcommand type to a special value. This can be done in a simpler way by using a separate flag introduced by commit ec0925c22a3d, so do that. Catversion bumped. It's not clear to me that ALTER TABLE subcommands are stored anywhere in catalogs (CREATE FUNCTION rejects it in BEGIN ATOMIC function bodies), but we do have both write and read support for them, so be safe. Discussion: https://postgr.es/m/20220929090033.zxuaezcdwh2fgfjb@alvherre.pgsql
* Add support for GRANT SET in psql tab completionMichael Paquier2022-12-12
| | | | | | | | | 3d14e17 has added support for this query but psql was not able to complete it. Spotted while working on a different patch in the same area. Reviewed-by: Robert Haas Discussion: https://postgr.es/m/Y3hw7yvG0VwpC1jq@paquier.xyz
* Remove direct call to GetNewObjectId() for pg_auth_members.oidMichael Paquier2022-12-12
| | | | | | | | | This routine should not be called directly as mentioned at its top, so replace it by GetNewOidWithIndex(). Issue introduced by 6566133 when pg_auth_members.oid got added, so no backpatch is needed. Author: Maciek Sakrejda Discussion: https://postgr.es/m/CAOtHd0Ckbih7Ur7XeVyLAJ26VZOfTNcq9qV403bNF4uTGtAN+Q@mail.gmail.com
* Convert domain_in to report errors softly.Tom Lane2022-12-11
| | | | | | | | | | | | | This is straightforward as far as it goes. However, it does not attempt to trap errors occurring during the execution of domain CHECK constraints. Since those are general user-defined expressions, the only way to do that would involve starting up a subtransaction for each check. Of course the entire point of the soft-errors feature is to not need subtransactions, so that would be self-defeating. For now, we'll rely on the assumption that domain checks are written to avoid throwing errors. Discussion: https://postgr.es/m/1181028.1670635727@sss.pgh.pa.us
* Convert json_in and jsonb_in to report errors softly.Tom Lane2022-12-11
| | | | | | | | | | | | | | | | This requires a bit of further infrastructure-extension to allow trapping errors reported by numeric_in and pg_unicode_to_server, but otherwise it's pretty straightforward. In the case of jsonb_in, we are only capturing errors reported during the initial "parse" phase. The value-construction phase (JsonbValueToJsonb) can also throw errors if assorted implementation limits are exceeded. We should improve that, but it seems like a separable project. Andrew Dunstan and Tom Lane Discussion: https://postgr.es/m/3bac9841-fe07-713d-fa42-606c225567d6@dunslane.net
* Change JsonSemAction to allow non-throw error reporting.Tom Lane2022-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Formerly, semantic action functions for the JSON parser returned void, so that there was no way for them to affect the parser's behavior. That means in particular that they can't force an error exit except by longjmp'ing. That won't do in the context of our project to make input functions return errors softly. Hence, change them to return the same JsonParseErrorType enum value as the parser itself uses. If an action function returns anything besides JSON_SUCCESS, the parse is abandoned and that error code is returned. Action functions can thus easily return the same error conditions that the parser already knows about. As an escape hatch for expansion, also invent a code JSON_SEM_ACTION_FAILED that the core parser does not know the exact meaning of. When returning this code, an action function must use some out-of-band mechanism for reporting the error details. This commit simply makes the API change and causes all the existing action functions to return JSON_SUCCESS, so that there is no actual change in behavior here. This is long enough and boring enough that it seemed best to commit it separately from the changes that make real use of the new mechanism. In passing, remove a duplicate assignment of transform_string_values_scalar. Discussion: https://postgr.es/m/1436686.1670701118@sss.pgh.pa.us
* Standardize error reports in unimplemented I/O functions.Tom Lane2022-12-10
| | | | | | | | | | | | | | | | We chose a specific wording of the not-implemented errors for pseudotype I/O functions and other cases where there's little value in implementing input and/or output. gtsvectorin never got that memo though, nor did most of contrib. Make these all fall in line, mostly because I'm a neatnik but also to remove unnecessary translatable strings. gbtreekey_in needs a bit of extra love since it supports multiple SQL types. Sadly, gbtreekey_out doesn't have the ability to do that, but I think it's unreachable anyway. Noted while surveying datatype input functions to see what we have left to fix.
* Use the macro, not handwritten code, to construct anymultirange_in().Tom Lane2022-12-10
| | | | | | | | | | | | | | Apparently anymultirange_in was written before we converted all these pseudotype input functions to use a common macro, and it didn't get fixed before committing. Sloppy merging probably explains its unintuitive ordering, too, so rearrange. Noted while surveying datatype input functions to see what we have left to fix. I'm inclined to leave the pseudotypes as throwing hard errors, because it's difficult to see a reason why anyone would need something else. But in any case, if we want to change that, we shouldn't have to change multiple copies of the code.
* Add subquery pullup handling for WindowClause runConditionDavid Rowley2022-12-10
| | | | | | | | | | | | | | | | | | | | | | | 9d9c02ccd added code to allow WindowAgg to take some shortcuts when a monotonic WindowFunc reached some value that it could never come back from due to the function's monotonic nature. That commit added a runCondition field to WindowClause to store the condition which, when it becomes false we can start taking shortcuts in nodeWindowAgg.c. Here we fix an issue where subquery pullups didn't properly update the runCondition to update the Vars to properly reference the new query level. Here we also add a missing call to preprocess_expression() for the WindowClause's runCondtion. The WindowFuncs in the targetlist will have had this process done, so we must also do it for the WindowFuncs in the runCondition so that they can be correctly found in the targetlist during setrefs.c Bug: #17709 Reported-by: Alexey Makhmutov Author: Richard Guo, David Rowley Discussion: https://postgr.es/m/17709-4f557160e3e8ee9a@postgresql.org Backpatch-through: 15, where 9d9c02ccd was introduced
* Fix macro definitions in pgstatfuncs.cMichael Paquier2022-12-10
| | | | | | | | | | | | | Buildfarm member wrasse has been complaining about empty declarations as an effect of 8018ffb and 83a1a1b due to extra semicolons. While on it, remove also the last backslash of the macros definitions, causing more lines to be eaten in it than necessary, per comment from Tom Lane. Reported-by: Tom Lane, and buildfarm member wrasse Author: Nathan Bossart, Michael Paquier Discussion: https://postgr.es/m/1188769.1670640236@sss.pgh.pa.us
* Restructure soft-error handling in formatting.c.Tom Lane2022-12-09
| | | | | | | | | | | | | Replace the error trapping scheme introduced in 5bc450629 with our shiny new errsave/ereturn mechanism. This doesn't have any real functional impact (although I think that the new coding is able to report a few more errors softly than v15 did). And I doubt there's any measurable performance difference either. But this gets rid of an ad-hoc, one-of-a-kind design in favor of a mechanism that will be widely used going forward, so it should be a net win for code readability. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Convert datetime input functions to use "soft" error reporting.Tom Lane2022-12-09
| | | | | | | | | This patch converts the input functions for date, time, timetz, timestamp, timestamptz, and interval to the new soft-error style. There's some related stuff in formatting.c that remains to be cleaned up, but that seems like a separable project. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Allow DateTimeParseError to handle bad-timezone error messages.Tom Lane2022-12-09
| | | | | | | | | | | | | | | | | | | | | | | | Pay down some ancient technical debt (dating to commit 022fd9966): fix a couple of places in datetime parsing that were throwing ereport's immediately instead of returning a DTERR code that could be interpreted by DateTimeParseError. The reason for that was that there was no mechanism for passing any auxiliary data (such as a zone name) to DateTimeParseError, and these errors seemed to really need it. Up to now it didn't matter that much just where the error got thrown, but now we'd like to have a hard policy that datetime parse errors get thrown from just the one place. Hence, invent a "DateTimeErrorExtra" struct that can be used to carry any extra values needed for specific DTERR codes. Perhaps in the future somebody will be motivated to use this to improve the specificity of other DateTimeParseError messages, but for now just deal with the timezone-error cases. This is on the way to making the datetime input functions report parse errors softly; but it's really an independent change, so commit separately. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Const-ify a couple of datetime parsing subroutines.Tom Lane2022-12-09
| | | | | | More could be done in this line, but I just grabbed some low-hanging fruit. Principal objective was to remove the need for several ugly unconstify() usages in formatting.c.
* Convert a few datatype input functions to use "soft" error reporting.Tom Lane2022-12-09
| | | | | | | | | | | | | | | This patch converts the input functions for bool, int2, int4, int8, float4, float8, numeric, and contrib/cube to the new soft-error style. array_in and record_in are also converted. There's lots more to do, but this is enough to provide proof-of-concept that the soft-error API is usable, as well as reference examples for how to convert input functions. This patch is mostly by me, but it owes very substantial debt to earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul. Thanks to Andres Freund for review. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Add test scaffolding for soft error reporting from input functions.Tom Lane2022-12-09
| | | | | | | | | | | | | | | | | pg_input_is_valid() returns boolean, while pg_input_error_message() returns the primary error message if the input is bad, or NULL if the input is OK. The main reason for having two functions is so that we can test both the details-wanted and the no-details-wanted code paths. Although these are primarily designed with testing in mind, it could well be that they'll be useful to end users as well. This patch is mostly by me, but it owes very substantial debt to earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul. Thanks to Andres Freund for review. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Create infrastructure for "soft" error reporting.Tom Lane2022-12-09
| | | | | | | | | | | | | | | | | | | | | | | | | | Postgres' standard mechanism for reporting errors (ereport() or elog()) is used for all sorts of error conditions. This means that throwing an exception via ereport(ERROR) requires an expensive transaction or subtransaction abort and cleanup, since the exception catcher dare not make many assumptions about what has gone wrong. There are situations where we would rather have a lighter-weight mechanism for dealing with errors that are known to be safe to recover from without a full transaction cleanup. This commit creates infrastructure to let us adapt existing error-reporting code for that purpose. See the included documentation changes for details. Follow-on commits will provide test code and usage examples. The near-term plan is to convert most if not all datatype input functions to report invalid input "softly". This will enable implementing some SQL/JSON features cleanly and without the cost of subtransactions, and it will also allow creating COPY options to deal with bad input without cancelling the whole COPY. This patch is mostly by me, but it owes very substantial debt to earlier work by Nikita Glukhov, Andrew Dunstan, and Amul Sul. Thanks also to Andres Freund for review. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Fix invalid role names introduced in 096dd80f3cAlexander Korotkov2022-12-09
| | | | | 096dd80f3c added new regression tests dealing with roles. By oversight, role names didn't start with regress_ prefix. This commit fixes that.
* Add USER SET parameter values for pg_db_role_settingAlexander Korotkov2022-12-09
| | | | | | | | | | | | | | | | The USER SET flag specifies that the variable should be set on behalf of an ordinary role. That lets ordinary roles set placeholder variables, which permission requirements are not known yet. Such a value wouldn't be used if the variable finally appear to require superuser privileges. The new flags are stored in the pg_db_role_setting.setuser array. Catversion is bumped. This commit is inspired by the previous work by Steve Chavez. Discussion: https://postgr.es/m/CAPpHfdsLd6E--epnGqXENqLP6dLwuNZrPMcNYb3wJ87WR7UBOQ%40mail.gmail.com Author: Alexander Korotkov, Steve Chavez Reviewed-by: Pavel Borisov, Steve Chavez
* Update MERGE docs to mention that ONLY is supported.Dean Rasheed2022-12-09
| | | | | | | | | | | | | | | | Commit 7103ebb7aa added support for MERGE, which included support for inheritance hierarchies, but didn't document the fact that ONLY could be specified before the source and/or target tables to exclude tables inheriting from the tables specified. Update merge.sgml to mention this, and while at it, add some regression tests to cover it. Dean Rasheed, reviewed by Nathan Bossart. Backpatch to 15, where MERGE was added. Discussion: https://postgr.es/m/CAEZATCU0XM-bJCvpJuVRU3UYNRqEBS6g4-zH%3Dj9Ye0caX8F6uQ%40mail.gmail.com
* Remove unnecessary castsPeter Eisentraut2022-12-08
| | | | | | | | | Some code carefully cast all data buffer arguments for BufFileWrite() and BufFileRead() to void *, even though the arguments are already void * (and AFAICT were never anything else). Remove this unnecessary clutter. Discussion: https://www.postgresql.org/message-id/flat/11dda853-bb5b-59ba-a746-e168b1ce4bdb%40enterprisedb.com
* Update types in File APIPeter Eisentraut2022-12-08
| | | | | | | | | | | | | Make the argument types of the File API match stdio better: - Change the data buffer to void *, from char *. - Change FileWrite() data buffer to const on top of that. - Change amounts to size_t, from int. In passing, change the FilePrefetch() amount argument from int to off_t, to match the underlying posix_fadvise(). Discussion: https://www.postgresql.org/message-id/flat/11dda853-bb5b-59ba-a746-e168b1ce4bdb%40enterprisedb.com
* Remove new structure member from ResultRelInfo.Etsuro Fujita2022-12-08
| | | | | | | | | | | | | | | In commit ffbb7e65a, I added a ModifyTableState member to ResultRelInfo to save the owning ModifyTableState for use by nodeModifyTable.c when performing batch inserts, but as pointed out by Tom Lane, that changed the array stride of es_result_relations, and that would break any previously-compiled extension code that accesses that array. Fix by removing that member from ResultRelInfo and instead adding a List member at the end of EState to save such ModifyTableStates. Per report from Tom Lane. Back-patch to v14, like the previous commit; I chose to apply the patch to HEAD as well, to make back-patching easy. Discussion: http://postgr.es/m/4065383.1669395453%40sss.pgh.pa.us
* Avoid unnecessary streaming of transactions during logical replication.Amit Kapila2022-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After restart, we don't perform streaming of an in-progress transaction if it was previously decoded and confirmed by the client. To achieve that we were comparing the END location of the WAL record being decoded with the WAL location we have already decoded and confirmed by the client. While decoding the commit record, to decide whether to process and send the complete transaction, we compare its START location with the WAL location we have already decoded and confirmed by the client. Now, if we need to queue some change in the transaction while decoding the commit record (e.g. snapshot), it is possible that we decide to stream the transaction but later commit processing decides to skip it. In such a case, we would needlessly send the changes and later when we decide to skip it, we will send stream abort. We also sometimes decide to stream the changes when we actually just need to process them locally like a change for invalidations. This will lead us to send empty streams. To avoid this, while queuing each change for decoding, we remember whether the transaction has any change that actually needs to be sent downstream and use that information later to decide whether to stream the transaction or not. Note, we can't avoid all cases where we have to send empty streams like the case where the plugin later decides that the change is not publishable. However, we will no longer need to send stream_abort when we skip sending a particular transaction. Author: Dilip Kumar Reviewed-by: Hou Zhijie, Ashutosh Bapat, Shi yu, Amit Kapila Discussion: https://postgr.es/m/CAFiTN-tHK=7LzfrPs8fbT2ksrOJGQbzywcgXst2bM9-rJJAAUg@mail.gmail.com
* meson: Add 'running' test setup, as a replacement for installcheckAndres Freund2022-12-07
| | | | | | | | | | | | | To run all tests that support running against existing server: $ meson test --setup running To run just the main pg_regress tests against existing server: $ meson test --setup running regress-running/regress To ensure the 'running' setup continues to work, test it as part of the freebsd CI task. Discussion: https://postgr.es/m/CAH2-Wz=XDQcmLoo7RR_i6FKQdDmcyb9q5gStnfuuQXrOGhB2sQ@mail.gmail.com
* Minor code refactoring in elog.c (no functional change).Tom Lane2022-12-07
| | | | | | | | | | | | | | | | | | Combine some duplicated code stanzas by creating small functions. Most of these duplications arose at a time when I wouldn't have trusted C compilers to auto-inline small functions intelligently, but they're probably poor practice now. Similarly split out some bits that aren't actually duplicative as the code stands, but would become so after an upcoming patch to add another error-handling code path. Take the opportunity to add some lengthier comments about what we're doing here, too. Re-order one function that seemed not very well-placed. Patch by me, per suggestions from Andres Freund. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
* Fix FK comment think-oPeter Eisentraut2022-12-07
| | | | | | | | from commit d6f96ed94e7 Author: Paul Jungwirth <pj@illuminatedcomputing.com> Reviewed-by: Ian Lawrence Barwick <barwick@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/6a7c7338-1aa2-4689-d171-0b0b294fdd84%40illuminatedcomputing.com
* Update outdated comment in ApplyRetrieveRuleAlvaro Herrera2022-12-07
| | | | | | | After a61b1f74823c. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqGZm7hb2VAy8HGM22-fTDaQzqE6T=5GbAk=GkT9H0hJEg@mail.gmail.com
* meson: Add basic PGXS compatibilityAndres Freund2022-12-06
| | | | | | | | | | | | | Generate a Makefile.global that's complete enough for PGXS to work for some extensions. It is likely that this compatibility layer will not suffice for every extension and not all platforms - we can expand it over time. This allows extensions to use a single buildsystem across all the supported postgres versions. Once all supported PG versions support meson, we can remove the compatibility layer. Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de
* autoconf: Move export_dynamic determination to configureAndres Freund2022-12-06
| | | | | | | | | | | | | Previously export_dynamic was set in src/makefiles/Makefile.$port. For solaris this required exporting with_gnu_ld. The determination of with_gnu_ld would be nontrivial to copy for meson PGXS compatibility. It's also nice to delete libtool.m4. This uses -Wl,--export-dynamic on all platforms, previously all platforms but FreeBSD used -Wl,-E. The likelihood of a name conflict seems lower with the longer spelling. Discussion: https://postgr.es/m/20221005200710.luvw5evhwf6clig6@awork3.anarazel.de
* Generate pg_stat_get*() functions for databases using macrosMichael Paquier2022-12-07
| | | | | | | | | | | | | | | The same code pattern is repeated 21 times for int64 counters (0 for missing entry) and 5 times for doubles (0 for missing entry) on database entries. This code is switched to use macros for the basic code instead, shaving a few hundred lines of originally-duplicated code patterns. The function names remain the same, but some fields of PgStat_StatDBEntry have to be renamed to cope with the new style. This is in the same spirit as 83a1a1b. Author: Michael Paquier Reviewed-by: Nathan Bossart, Bertrand Drouvot Discussion: https://postgr.es/m/Y46stlxQ2LQE20Na@paquier.xyz
* meson: Basic cygwin supportAndres Freund2022-12-06
| | | | | | | | | | | There likely are further issues, but as evidenced by the CI task proposed by Justin in the referenced thread, this suffices to build and run basic tests in cygwin (some fixes for the test infrastructure are needed, but that's independent of the meson aspect). Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20221021034040.GT16921@telsasoft.com
* Rework query relation permission checkingAlvaro Herrera2022-12-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, information about the permissions to be checked on relations mentioned in a query is stored in their range table entries. So the executor must scan the entire range table looking for relations that need to have permissions checked. This can make the permission checking part of the executor initialization needlessly expensive when many inheritance children are present in the range range. While the permissions need not be checked on the individual child relations, the executor still must visit every range table entry to filter them out. This commit moves the permission checking information out of the range table entries into a new plan node called RTEPermissionInfo. Every top-level (inheritance "root") RTE_RELATION entry in the range table gets one and a list of those is maintained alongside the range table. This new list is initialized by the parser when initializing the range table. The rewriter can add more entries to it as rules/views are expanded. Finally, the planner combines the lists of the individual subqueries into one flat list that is passed to the executor for checking. To make it quick to find the RTEPermissionInfo entry belonging to a given relation, RangeTblEntry gets a new Index field 'perminfoindex' that stores the corresponding RTEPermissionInfo's index in the query's list of the latter. ExecutorCheckPerms_hook has gained another List * argument; the signature is now: typedef bool (*ExecutorCheckPerms_hook_type) (List *rangeTable, List *rtePermInfos, bool ereport_on_violation); The first argument is no longer used by any in-core uses of the hook, but we leave it in place because there may be other implementations that do. Implementations should likely scan the rtePermInfos list to determine which operations to allow or deny. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqGjJDmUhDSfv-U2qhKJjt9ST7Xh9JXC_irsAQ1TAUsJYg@mail.gmail.com
* Fix 32-bit build dangling pointer issue in WindowAggDavid Rowley2022-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 9d9c02ccd added window "run conditions", which allows the evaluation of monotonic window functions to be skipped when the run condition is no longer true. Prior to this commit, once the run condition was no longer true and we stopped evaluating the window functions, we simply just left the ecxt_aggvalues[] and ecxt_aggnulls[] arrays alone to store whatever value was stored there the last time the window function was evaluated. Leaving a stale value in there isn't really a problem on 64-bit builds as all of the window functions which we recognize as monotonic all return int8, which is passed by value on 64-bit builds. However, on 32-bit builds, this was a problem as the value stored in the ecxt_values[] element would be a by-ref value and it would be pointing to some memory which would get reset once the tuple context is destroyed. Since the WindowAgg node will output these values in the resulting tupleslot, this could be problematic for the top-level WindowAgg node which must look at these values to filter out the rows that don't meet its filter condition. Here we fix this by just zeroing the ecxt_aggvalues[] and setting the ecxt_aggnulls[] array to true when the run condition first becomes false. This results in the WindowAgg's output having NULLs for the WindowFunc's columns rather than the stale or pointer pointing to possibly freed memory. These tuples with the NULLs can only make it as far as the top-level WindowAgg node before they're filtered out. To ensure that these tuples *are* always filtered out, we now insist that OpExprs making up the run condition are strict OpExprs. Currently, all the window functions which the planner recognizes as monotonic return INT8 and the operator which is used for the run condition must be a member of a btree opclass. In reality, these restrictions exclude nothing that's built-in to Postgres and are unlikely to exclude anyone's custom operators due to the requirement that the operator is part of a btree opclass. It would be unusual if those were not strict. Reported-by: Sergey Shinderuk, using valgrind Reviewed-by: Richard Guo, Sergey Shinderuk Discussion: https://postgr.es/m/29184c50-429a-ebd7-f1fb-0589c6723a35@postgrespro.ru Backpatch-through: 15, where 9d9c02ccd was added
* Generate pg_stat_get*() functions for tables using macrosMichael Paquier2022-12-06
| | | | | | | | | | | | | The same code pattern is repeated 17 times for int64 counters (0 for missing entry) and 5 times for timestamps (NULL for missing entry) on table entries. This code is switched to use a macro for the basic code instead, shaving a few hundred lines of originally-duplicated code. The function names remain the same, but some fields of PgStat_StatTabEntry have to be renamed to cope with the new style. Author: Bertrand Drouvot Reviewed-by: Nathan Bossart Discussion: https:/postgr.es/m/20221204173207.GA2669116@nathanxps13
* Check the snapshot argument of index_beginscan and familyAlexander Korotkov2022-12-06
| | | | | | | | | | | | Passing a NULL snapshot (InvalidSnapshot) is going to work but only as long as the index can't find any matching rows. This can be confusing for the extension authors, so add an explicit check for this argument. The check is implemented with Assert() in order to avoid overhead in release builds. Reported-by: Sven Klemm Discussion: https://postgr.es/m/CAJ7c6TPxitD4vbKyP-mpmC1XwyHdPPqvjLzm%2BVpB88h8LGgneQ%40mail.gmail.com Author: Aleksander Alekseev Reviewed-by: Pavel Borisov
* Provide test coverage in pg_dump for default behaviors with compressionMichael Paquier2022-12-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | By default, the contents generated by the custom and directory dump formats are compressed. However, with the existing test facility, the restore program will succeed regardless of whether the dumped output was compressed or not without checking if anything has been compressed. This commit implements a portable way to check the contents of the custom and directory dump formats: - glob_patterns, that can be defined for each test as an array of glob()-compilable strings, tracking the contents that should or should not be compressed. While this is useful to make sure that the table data is compressed, this also checks that blobs.toc and toc.dat are never compressed. - command_like, to execute a command on a dump and check its generated output. This is used here in correlation with pg_restore -l to check if the dumps have been compressed or not, depending on if the build supports gzip, or not. This hole in the tests has come up when working on 5e73a60, where compression has to be applied by default, if available, for both dump formats. The idea of glob_patterns comes from me, and Georgios has come up with the design for command_like. Author: Georgios Kokolatos, Michael Paquier Discussion: https://postgr.es/m/DQn4czCWR1rcbGPLL7p3LfEr5-kGmlySm-H05VgroINdikvhtS5r9EdI6b8D8sjnbKdJ09k-cxs2AqijBeHAWk9Q8gvEAxPRHuLRhwONcGc=@pm.me
* initdb: Refactor PG_CMD_PUTS loopsPeter Eisentraut2022-12-05
| | | | | | | | | | | | | | Keeping the SQL commands that initdb runs in string arrays before feeding them to PG_CMD_PUTS() seems unnecessarily verbose and inflexible. In some cases, the array only has one member. In other cases, one might want to use PG_CMD_PRINTF() instead, to parametrize a command, but that would require breaking up the loop or using workarounds like replace_token(). Unwind all that; it's much simpler that way. Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://www.postgresql.org/message-id/flat/2c50823b-f453-bb97-e38b-34751c51dcdf%40enterprisedb.com
* Fix Memoize to work with partitionwise joining.Tom Lane2022-12-05
| | | | | | | | | | | | A couple of places weren't up to speed for this. By sheer good luck, we didn't fail but just selected a non-memoized join plan, at least in the test case we have. Nonetheless, it's a bug, and I'm not quite sure that it couldn't have worse consequences in other examples. So back-patch to v14 where Memoize came in. Richard Guo Discussion: https://postgr.es/m/CAMbWs48GkNom272sfp0-WeD6_0HSR19BJ4H1c9ZKSfbVnJsvRg@mail.gmail.com
* pg_dump: Remove "blob" terminologyPeter Eisentraut2022-12-05
| | | | | | | | | | | | | | | For historical reasons, pg_dump refers to large objects as "BLOBs". This term is not used anywhere else in PostgreSQL, and it also means something different in the SQL standard and other SQL systems. This patch renames internal functions, code comments, documentation, etc. to use the "large object" or "LO" terminology instead. There is no functionality change, so the archive format still uses the name "BLOB" for the archive entry. Additional long command-line options are added with the new naming. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/868a381f-4650-9460-1726-1ffd39a270b4%40enterprisedb.com
* Add LSN location in some error messages related to WAL pagesMichael Paquier2022-12-05
| | | | | | | | | | | | | | The error messages reported during any failures while reading or validating the header of a WAL currently includes only the offset of the page but not the compiled LSN referring to the page, requiring an extra step to compile it if looking at the surroundings with pg_waldump or similar. Adding this information costs a bit in translation, but also eases debugging. Author: Bharath Rupireddy Reviewed-by: Álvaro Herrera, Kyotaro Horiguchi, Maxim Orlov, Michael Paquier Discussion: https://postgr.es/m/CALj2ACWV=FCddsxcGbVOA=cvPyMr75YCFbSQT6g4KDj=gcJK4g@mail.gmail.com
* Fix thinko introduced in 6b423ec67David Rowley2022-12-05
| | | | | | | | | | | | | | | | | | As pointed out by Dean Rasheed, we really should be using tmp > -(PG_INTNN_MIN / 10) rather than tmp > (PG_INTNN_MAX / 10) for checking for overflows in the accumulation in the pg_strtointNN functions. This does happen to be the same number when dividing by 10, but there is a pending patch which adds other bases and this is not the same number if we were to divide by 2 rather than 10, for example. If the base 2 parsing was to follow this example then we could accidentally think a string containing the value of PG_INT32_MIN was an overflow in pg_strtoint32. Clearly that shouldn't overflow. This does not fix any actual live bugs, only some bad examples of overflow checks for future bases. Reported-by: Dean Rasheed Discussion: https://postgr.es/m/CAEZATCVEtwfhdm-K-etZYFB0=qsR0nT6qXta_W+GQx4RYph1dg@mail.gmail.com
* Re-pgindent a few files.Tom Lane2022-12-04
| | | | | | Just because I'm a neatnik, and I'm currently working on code in this area. It annoys me to not be able to pgindent my patches without working around unrelated changes.
* Fix broken MemoizePath support in reparameterize_path().Tom Lane2022-12-04
| | | | | | | | | | | | | | It neglected to recurse to the subpath, meaning you'd get back a path identical to the input. This could produce wrong query results if the omission meant that the subpath fails to enforce some join clause it should be enforcing. We don't have a test case for this at the moment, but the code is obviously broken and the fix is equally obvious. Back-patch to v14 where Memoize was introduced. Richard Guo Discussion: https://postgr.es/m/CAMbWs4_R=ORpz=Lkn2q3ebPC5EuWyfZF+tmfCPVLBVK5W39mHA@mail.gmail.com
* Add missing MaterialPath support in reparameterize_path[_by_child].Tom Lane2022-12-04
| | | | | | | | | | These two functions failed to cover MaterialPath. That's not a fatal problem, but we can generate better plans in some cases if we support it. Tom Lane and Richard Guo Discussion: https://postgr.es/m/1854233.1669949723@sss.pgh.pa.us
* Fix generate_partitionwise_join_paths() to tolerate failure.Tom Lane2022-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | We might fail to generate a partitionwise join, because reparameterize_path_by_child() does not support all path types. This should not be a hard failure condition: we should just fall back to a non-partitioned join. However, generate_partitionwise_join_paths did not consider this possibility and would emit the (misleading) error "could not devise a query plan for the given query" if we'd failed to make any paths for a child join. Fix it to give up on partitionwise joining if so. (The accepted technique for giving up appears to be to set rel->nparts = 0, which I find pretty bizarre, but there you have it.) I have not added a test case because there'd be little point: any omissions of this sort that we identify would soon get fixed by extending reparameterize_path_by_child(), so the test would stop proving anything. However, right now there is a known test case based on failure to cover MaterialPath, and with that I've found that this is broken in all supported versions. Hence, patch all the way back. Original report and patch by me; thanks to Richard Guo for identifying a test case that works against committed versions. Discussion: https://postgr.es/m/1854233.1669949723@sss.pgh.pa.us
* Improve performance of pg_strtointNN functionsDavid Rowley2022-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Experiments have shown that modern versions of both gcc and clang are unable to fully optimize the multiplication by 10 that we're doing in the pg_strtointNN functions. Both compilers seem to be making use of "imul", which is not the most efficient way to multiply by 10. This seems to be due to the overflow checking that we're doing. Without the overflow checks, both those compilers switch to a more efficient method of multiplying by 10. In absence of overflow concern, integer multiplication by 10 can be done by bit-shifting left 3 places to multiply by 8 and then adding the original value twice. To allow compilers this flexibility, here we adjust the code so that we accumulate the number as an unsigned version of the type and remove the use of pg_mul_sNN_overflow() and pg_sub_sNN_overflow(). The overflow checking can be done simply by checking if the accumulated value has gone beyond a 10th of the maximum *signed* value for the given type. If it has then the accumulation of the next digit will cause an overflow. After this is done, we do a final overflow check before converting the unsigned version of the number back to its signed counterpart. Testing has shown about an 8% speedup of a COPY into a table containing 2 INT columns. Author: David Rowley, Dean Rasheed Discussion: https://postgr.es/m/CAApHDvrL6_+wKgPqRHr7gH_6xy3hXM6a3QCsZ5ForurjDFfenA@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvrdYByjfj-=WbmVNFgmVZg88-dE7heukw8p55aJ+W=qxQ@mail.gmail.com