aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* 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
* meson: Improve PG_VERSION_STR generationAndres Freund2022-12-09
| | | | | | | | | Previously the host operating system and 32/64 bit were not included and the build machine's cpu was used, which is potentially wrong for cross builds. Author: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAC+AXB16gwYhKCdS+t0pk3U7kKtpVj5L-ynmhK3Gbea330At3w@mail.gmail.com
* 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
* Add option to specify segment size in blocksAndres Freund2022-12-07
| | | | | | | | | | | | | | The tests don't have much coverage of segment related code, as we don't create large enough tables. To make it easier to test these paths, add a new option specifying the segment size in blocks. Set the new option to 6 blocks in one of the CI tasks. Smaller numbers currently fail one of the tests, for understandable reasons. While at it, fix some segment size related issues in the meson build. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20221107171355.c23fzwanfzq2pmgt@awork3.anarazel.de
* 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
* Doc: subdivide System Information Functions and Operators.Tom Lane2022-12-07
| | | | | | | | | | Provide <sect2> subdivisions in 9.26 System Information Functions and Operators. This is useful because it adds a mini-TOC at the top of the page to aid jumping to portions of what's become quite a long section. Also, now that several of the subsections contain multiple tables, it's hard to see the overall structure without headings. Discussion: https://postgr.es/m/4026789.1670426602@sss.pgh.pa.us
* 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 typoAlvaro Herrera2022-12-06
|
* 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
* doc: Add missing <varlistentry> markups for developer GUCsMichael Paquier2022-12-05
| | | | | | | | | Missing such markups makes it impossible to create links back to these GUCs, and all the other parameters have one already. Author: Ian Lawrence Barwick Discussion: https://postgr.es/m/CAB8KJ=jx=6dFB_EN3j0UkuvG3cPu5OmQiM-ZKRAz+fKvS+u8Ng@mail.gmail.com Backpatch-through: 11
* 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
* Update top-level .gitignore.Tom Lane2022-12-04
| | | | | | | | | | Add a comment explaining our policy that we don't put excludes for nonstandard tools into committed .gitignore files. Also, remove the entries about libraries with .sl extensions, since that convention was only used on now-desupported HP-UX. Discussion: https://postgr.es/m/CAHxW8BAiyPwfXbN813GhorQozwMBs4f3DTxLkKNxiGQuJuw4Vw@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
* Doc: flesh out fmgr/README's description of context-node usage.Tom Lane2022-12-03
| | | | | | | I wrote this to provide a home for a planned discussion of error return conventions for non-error-throwing functions. But it seems useful as documentation of existing code no matter what becomes of that proposal, so commit separately.
* Fix DEFAULT handling for multi-row INSERT rules.Dean Rasheed2022-12-03
| | | | | | | | | | | | | | | | | | | | | | | | | | When updating a relation with a rule whose action performed an INSERT from a multi-row VALUES list, the rewriter might skip processing the VALUES list, and therefore fail to replace any DEFAULTs in it. This would lead to an "unrecognized node type" error. The reason was that RewriteQuery() assumed that a query doing an INSERT from a multi-row VALUES list would necessarily only have one item in its fromlist, pointing to the VALUES RTE to read from. That assumption is correct for the original query, but not for product queries produced for rule actions. In such cases, there may be multiple items in the fromlist, possibly including multiple VALUES RTEs. What is required instead is for RewriteQuery() to skip any RTEs from the product query's originating query, which might include one or more already-processed VALUES RTEs. What's left should then include at most one VALUES RTE (from the rule action) to be processed. Patch by me. Thanks to Tom Lane for reviewing. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAEZATCV39OOW7LAR_Xq4i%2BLc1Byux%3DeK3Q%3DHD_pF1o9LBt%3DphA%40mail.gmail.com
* Add missing const qualifierDavid Rowley2022-12-03
| | | | | | This is present in the declaration for ReadDataFromArchive, so we'd better have it in the definition too in order to avoid compilers from complaining about the mismatch of function signatures.
* Prevent pgstats from getting confused when relkind of a relation changesAndres Freund2022-12-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | When the relkind of a relache entry changes, because a table is converted into a view, pgstats can get confused in 15+, leading to crashes or assertion failures. For HEAD, Tom fixed this in b23cd185fd5, by removing support for converting a table to a view, removing the source of the inconsistency. This commit just adds an assertion that a relcache entry's relkind does not change, just in case we end up with another case of that in the future. As there's no cases of changing relkind anymore, we can't add a test that that's handled correctly. For 15, fix the problem by not maintaining the association with the old pgstat entry when the relkind changes during a relcache invalidation processing. In that case the pgstat entry needs to be unlinked first, to avoid PgStat_TableStatus->relation getting out of sync. Also add a test reproducing the issues. No known problem exists in 11-14, so just add the test there. Reported-by: vignesh C <vignesh21@gmail.com> Author: Andres Freund <andres@anarazel.de> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com Discussion: https://postgr.es/m/CALDaNm3oZA-8Wbps2Jd1g5_Gjrr-x3YWrJPek-mF5Asrrvz2Dg@mail.gmail.com Backpatch: 15-
* Fix broken hash function hashbpcharextended().Jeff Davis2022-12-02
| | | | | | | | | | | | | | | Ignore trailing spaces for non-deterministic collations when hashing. The previous behavior could lead to tuples falling into the wrong partitions when hash partitioning is combined with the BPCHAR type and a non-deterministic collation. Fortunately, it did not affect hash indexes, because hash indexes do not use extended hash functions. Decline to backpatch, per discussion. Discussion: https://postgr.es/m/eb83d0ac7b299eb08f9b900dd08a5a0c5d90e517.camel@j-davis.com Reviewed-by: Richard Guo, Tom Lane
* Remove gen_node_support.pl's special treatment of EquivalenceClasses.Tom Lane2022-12-02
| | | | | | | | | | It seems better to deal with this by explicit annotations on the fields in question, instead of magic knowledge embedded in the script. While that creates a risk-of-omission from failing to annotate fields, the preceding commit should catch any such oversights. Discussion: https://postgr.es/m/263413.1669513145@sss.pgh.pa.us
* Add some error cross-checks to gen_node_support.pl.Tom Lane2022-12-02
| | | | | | | | | | | | | | | | | | Check that if we generate a call to copy, compare, write, or read a specific node type, that node type does have the appropriate support function. (This doesn't protect against trying to invoke nonexistent code when considering generic field types such as "Node *", but it seems like a useful check anyway.) Check that array_size() refers to a field appearing earlier in the struct. Aside from catching obvious errors like a misspelled field name, this protects against a more subtle mistake: if the size field appears later in the struct than the array field, then compare and read functions would misbehave. There is actually exactly that situation in PlannerInfo, but it's okay since we do not need compare or read functionality for that (today anyway). Discussion: https://postgr.es/m/263413.1669513145@sss.pgh.pa.us
* Fix psql's \sf and \ef for new-style SQL functions.Tom Lane2022-12-02
| | | | | | | | | | | | | | Some options of these commands need to be able to identify the start of the function body within the output of pg_get_functiondef(). It used to be that that always began with "AS", but since the introduction of new-style SQL functions, it might also start with "BEGIN" or "RETURN". Fix that on the psql side, and add some regression tests. Noted by me awhile ago, but I didn't do anything about it. Thanks to David Johnston for a nag. Discussion: https://postgr.es/m/AM9PR01MB8268D5CDABDF044EE9F42173FE8C9@AM9PR01MB8268.eurprd01.prod.exchangelabs.com