aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix typo in reference to __FreeBSD__.Thomas Munro2022-12-16
| | | | | | | | | | Commit a2a8acd152 introduced a platform-dependent mechanism to prevent developers from referencing errno in the argument list of elog()/ereport(), but didn't use the right macro to detect FreeBSD, so it didn't actually work there. Reported-by: Japin Li <japinli@hotmail.com> Discussion: https://postgr.es/m/MEYP282MB16693AAEEF84F47D8F7CA007B6E69%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Remove pessimistic cost penalization from Incremental SortDavid Rowley2022-12-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When incremental sorts were added in v13 a 1.5x pessimism factor was added to the cost modal. Seemingly this was done because the cost modal only has an estimate of the total number of input rows and the number of presorted groups. It assumes that the input rows will be evenly distributed throughout the presorted groups. The 1.5x pessimism factor was added to slightly reduce the likelihood of incremental sorts being used in the hope to avoid performance regressions where an incremental sort plan was picked and turned out slower due to a large skew in the number of rows in the presorted groups. An additional quirk with the path generation code meant that we could consider both a sort and an incremental sort on paths with presorted keys. This meant that with the pessimism factor, it was possible that we opted to perform a sort rather than an incremental sort when the given path had presorted keys. Here we remove the 1.5x pessimism factor to allow incremental sorts to have a fairer chance at being chosen against a full sort. Previously we would generally create a sort path on the cheapest input path (if that wasn't sorted already) and incremental sort paths on any path which had presorted keys. This meant that if the cheapest input path wasn't completely sorted but happened to have presorted keys, we would create a full sort path *and* an incremental sort path on that input path. Here we change this logic so that if there are presorted keys, we only create an incremental sort path, and create sort paths only when a full sort is required. Both the removal of the cost pessimism factor and the changes made to the path generation make it more likely that incremental sorts will now be chosen. That, of course, as with teaching the planner any new tricks, means an increased likelihood that the planner will perform an incremental sort when it's not the best method. Our standard escape hatch for these cases is an enable_* GUC. enable_incremental_sort already exists for this. This came out of a report by Pavel Luzanov where he mentioned that the master branch was choosing to perform a Seq Scan -> Sort -> Group Aggregate for his query with an ORDER BY aggregate function. The v15 plan for his query performed an Index Scan -> Group Aggregate, of course, the aggregate performed the final sort internally in nodeAgg.c for the aggregate's ORDER BY. The ideal plan would have been to use the index, which provided partially sorted input then use an incremental sort to provide the aggregate with the sorted input. This was not being chosen due to the pessimism in the incremental sort cost modal, so here we remove that and rationalize the path generation so that sort and incremental sort plans don't have to needlessly compete. We assume that it's senseless to ever use a full sort on a given input path where an incremental sort can be performed. Reported-by: Pavel Luzanov Reviewed-by: Richard Guo Discussion: https://postgr.es/m/9f61ddbf-2989-1536-b31e-6459370a6baa%40postgrespro.ru
* Re-adjust drop-index-concurrently-1 isolation testDavid Rowley2022-12-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It seems that drop-index-concurrently-1 has started to forget what it was originally meant to be testing. d2d8a229b, which added incremental sorts changed the expected plan to be an Index Scan plan instead of a Seq Scan plan. This occurred as the primary key index of the table in question provided presorted input and, because that index happened to be the cheapest input path due to enable_seqscan being disabled, the incremental sort changes just added a Sort on top of that. It seems based on the name of the PREPAREd statement that the intention here is that the query produces a seqscan plan. The reason this test has become broken seems to be due to how the test was originally coded. The test was trying to force a seqscan plan by performing some casting to make it so the test_dc index couldn't be used to perform the required filtering. Trying to coax the planner into using a plan which has costed in a disable_cost seems like it's always going to be flakey as small changes in costs are drowned out by the large disable_cost combined with add_path's STD_FUZZ_FACTOR. Here we get rid of the casts that we're using to try to trick the planner into a seqscan and instead toggle enable_seqscan as and when required to get the desired plan. Additionally, rename a few things in the test and add some additional wording to the comments to try and make it more clear in the future what we expect this test to be doing. Discussion: https://postgr.es/m/CAApHDvrbDhObhLV+=U_K_-t+2Av2av1aL9d+2j_3AO-XndaviA@mail.gmail.com Backpatch-through: 13, where d2d8a229b changed the expected test output
* Speed up creation of command completion tagsDavid Rowley2022-12-16
| | | | | | | | | | | | | | | | | | | The building of command completion tags could often be seen showing up in profiles when running high tps workloads. The query completion tags were being built with snprintf, which is slow at the best of times when compared with more manual ways of formatting strings. Here we introduce BuildQueryCompletionString() to do this job for us. We also now store the completion tag's strlen in the CommandTagBehavior struct so that we can quickly memcpy this number of bytes into the completion tag string. Appending the rows affected is done via pg_ulltoa_n. BuildQueryCompletionString returns the length of the built string. This saves us having to call strlen to figure out how many bytes to pass to pq_putmessage(). Author: David Rowley, Andres Freund Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CAHoyFK-Xwqc-iY52shj0G+8K9FJpse+FuZ36XBKy78wDVnd=Qg@mail.gmail.com
* Convert range_in and multirange_in to report errors softly.Tom Lane2022-12-15
| | | | | | | | | | | | | | | | This is mostly straightforward, except that if the range type has a canonical function, that might throw an error during range input. (Such errors probably only occur for edge cases: in the in-core canonical functions, it happens only if a bound has the maximum valid value for the underlying type.) Hence, this patch extends the soft-error regime to allow canonical functions to return errors softly as well. Extensions implementing range canonical functions will need modification anyway because of the API change for range_serialize(); while at it, they might want to do something similar to what's been done here in the in-core canonical functions. Discussion: https://postgr.es/m/3284599.1671075185@sss.pgh.pa.us
* Static assertions cleanupPeter Eisentraut2022-12-15
| | | | | | | | | | | | | | | | | | | | | Because we added StaticAssertStmt() first before StaticAssertDecl(), some uses as well as the instructions in c.h are now a bit backwards from the "native" way static assertions are meant to be used in C. This updates the guidance and moves some static assertions to better places. Specifically, since the addition of StaticAssertDecl(), we can put static assertions at the file level. This moves a number of static assertions out of function bodies, where they might have been stuck out of necessity, to perhaps better places at the file level or in header files. Also, when the static assertion appears in a position where a declaration is allowed, then using StaticAssertDecl() is more native than StaticAssertStmt(). Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/941a04e7-dd6f-c0e4-8cdf-a33b3338cbda%40enterprisedb.com
* Move provariadic sanity check to a more appropriate placePeter Eisentraut2022-12-15
| | | | | | | | 35f059e9bdfb3b14ac9d22a9e159d36ec0ccf804 put the provariadic sanity check into type_sanity.sql, even though it's not about types, and moreover in the middle of some connected test group, which makes it all very confusing. Move it to opr_sanity.sql, where it is in better company.
* Convert a few more datatype input functions to report errors softly.Tom Lane2022-12-14
| | | | | | | Convert the remaining string-category input functions (bpcharin, varcharin, byteain) to the new style. Discussion: https://postgr.es/m/3038346.1671060258@sss.pgh.pa.us
* Convert a few more datatype input functions to report errors softly.Tom Lane2022-12-14
| | | | | | | | Convert cash_in and uuid_in to the new style. Amul Sul, minor mods by me Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com
* Convert a few more datatype input functions to report errors softly.Tom Lane2022-12-14
| | | | | | | | | | | | | | | Convert assorted internal-ish datatypes, namely aclitemin, int2vectorin, oidin, oidvectorin, pg_lsn_in, pg_snapshot_in, and tidin to the new style. (Some others you might expect to find in this group, such as cidin and xidin, need no changes because they never throw errors at all. That seems a little cheesy ... but it is not in the charter of this patch series to add new error conditions.) Amul Sul, minor mods by me Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com
* Convert the geometric input functions to report errors softly.Tom Lane2022-12-14
| | | | | | | | | | | | | | | | | | Convert box_in, circle_in, line_in, lseg_in, path_in, point_in, and poly_in to the new style. line_in still throws hard errors for overflows/underflows that can occur when the input is specified as two points rather than in the canonical "Ax + By + C = 0" style. I'm not too concerned about that: it won't be reached in normal dump/restore cases, and it's fairly debatable that such conversion should ever have been made part of a type input function in the first place. But in any case, I don't want to extend the soft error conventions into float.h without more discussion than this patch has had. Amul Sul, minor mods by me Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com
* Convert a few more datatype input functions to report errors softly.Tom Lane2022-12-14
| | | | | | | | | Convert bit_in, varbit_in, inet_in, cidr_in, macaddr_in, and macaddr8_in to the new style. Amul Sul, minor mods by me Discussion: https://postgr.es/m/CAAJ_b97KeDWUdpTKGOaFYPv0OicjOu6EW+QYWj-Ywrgj_aEy1g@mail.gmail.com
* Rearrange some static assertions for consistencyPeter Eisentraut2022-12-14
| | | | | | | Put lengthof first. Reported-by: Peter Smith <smithpb2250@gmail.com> Discussion: https://www.postgresql.org/message-id/CAHut+PsUDMySVRuRc=h+P5N3+=TGvj4W_mi32XXg9dt4o-BXbA@mail.gmail.com
* Non-decimal integer literalsPeter Eisentraut2022-12-14
| | | | | | | | | | | | | | | | | | | Add support for hexadecimal, octal, and binary integer literals: 0x42F 0o273 0b100101 per SQL:202x draft. This adds support in the lexer as well as in the integer type input functions. Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Add grantable MAINTAIN privilege and pg_maintain role.Jeff Davis2022-12-13
| | | | | | | | | | | | | Allows VACUUM, ANALYZE, REINDEX, REFRESH MATERIALIZED VIEW, CLUSTER, and LOCK TABLE. Effectively reverts 4441fc704d. Instead of creating separate privileges for VACUUM, ANALYZE, and other maintenance commands, group them together under a single MAINTAIN privilege. Author: Nathan Bossart Discussion: https://postgr.es/m/20221212210136.GA449764@nathanxps13 Discussion: https://postgr.es/m/45224.1670476523@sss.pgh.pa.us
* Remove SHA256_HMAC_B from scram-common.hMichael Paquier2022-12-14
| | | | | | | | | | This referred to the size of the buffers for k_ipad and k_opad in HMAC computations. This is unused since e6bdfd9, where SCRAM has switched to the cryptohash routines for its HMAC calculations rather than its own maths. Reviewed-by: Jacob Champion Discussion: https://postgr.es/m/Y5gGMjXhyp0oK0mH@paquier.xyz
* Rethink handling of [Prevent|Is]InTransactionBlock in pipeline mode.Tom Lane2022-12-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commits f92944137 et al. made IsInTransactionBlock() set the XACT_FLAGS_NEEDIMMEDIATECOMMIT flag before returning "false", on the grounds that that kept its API promises equivalent to those of PreventInTransactionBlock(). This turns out to be a bad idea though, because it allows an ANALYZE in a pipelined series of commands to cause an immediate commit, which is unexpected. Furthermore, if we return "false" then we have another issue, which is that ANALYZE will decide it's allowed to do internal commit-and-start-transaction sequences, thus possibly unexpectedly committing the effects of previous commands in the pipeline. To fix the latter situation, invent another transaction state flag XACT_FLAGS_PIPELINING, which explicitly records the fact that we have executed some extended-protocol command and not yet seen a commit for it. Then, require that flag to not be set before allowing InTransactionBlock() to return "false". Having done that, we can remove its setting of NEEDIMMEDIATECOMMIT without fear of causing problems. This means that the API guarantees of IsInTransactionBlock now diverge from PreventInTransactionBlock, which is mildly annoying, but it seems OK given the very limited usage of IsInTransactionBlock. (In any case, a caller preferring the old behavior could always set NEEDIMMEDIATECOMMIT for itself.) For consistency also require XACT_FLAGS_PIPELINING to not be set in PreventInTransactionBlock. This too is meant to prevent commands such as CREATE DATABASE from silently committing previous commands in a pipeline. Per report from Peter Eisentraut. As before, back-patch to all supported branches (which sadly no longer includes v10). Discussion: https://postgr.es/m/65a899dd-aebc-f667-1d0a-abb89ff3abf8@enterprisedb.com
* Refactor ExecGrant_*() functionsPeter Eisentraut2022-12-13
| | | | | | | | | | | | Instead of half a dozen of mostly-duplicate ExecGrant_Foo() functions, write one common function ExecGrant_generic() that can handle most of them. We already have all the information we need, such as which system catalog corresponds to which catalog table and which column is the ACL column. Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Antonin Houska <ah@cybertec.at> Discussion: https://www.postgresql.org/message-id/flat/22c7e802-4e7d-8d87-8b71-cba95e6f4bcf%40enterprisedb.com
* Fix jsonb subscripting to cope with toasted subscript values.Tom Lane2022-12-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | jsonb_get_element() was incautious enough to use VARDATA() and VARSIZE() directly on an arbitrary text Datum. That of course fails if the Datum is short-header, compressed, or out-of-line. The typical result would be failing to match any element of a jsonb object, though matching the wrong one seems possible as well. setPathObject() was slightly brighter, in that it used VARDATA_ANY and VARSIZE_ANY_EXHDR, but that only kept it out of trouble for short-header Datums. push_path() had the same issue. This could result in faulty subscripted insertions, though keys long enough to cause a problem are likely rare in the wild. Having seen these, I looked around for unsafe usages in the rest of the adt/json* files. There are a couple of places where it's not immediately obvious that the Datum can't be compressed or out-of-line, so I added pg_detoast_datum_packed() to cope if it is. Also, remove some other usages of VARDATA/VARSIZE on Datums we just extracted from a text array. Those aren't actively broken, but they will become so if we ever start allowing short-header array elements, which does not seem like a terribly unreasonable thing to do. In any case they are not great coding examples, and they could also do with comments pointing out that we're assuming we don't need pg_detoast_datum_packed. Per report from exe-dealer@yandex.ru. Patch by me, but thanks to David Johnston for initial investigation. Back-patch to v14 where jsonb subscripting was introduced. Discussion: https://postgr.es/m/205321670615953@mail.yandex.ru
* 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