aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix copy-and-pasteo in comment.Etsuro Fujita2022-11-02
|
* Improve the description of XLOG_RUNNING_XACTS.Amit Kapila2022-11-02
| | | | | | | | | | | | | | Previously, the description of XLOG_RUNNING_XACTS showed only top-transaction XIDs and whether subtransactions overflowed. This commit improves it to show individual subtransaction XIDs. This also improves the description of overflowed subtransactions. This additional information can be helpful for testing and debugging purposes. Author: Masahiko Sawada Reviewd by: Fujii Masao, Kyotaro Horiguchi, Ashutosh Bapat, Bharath Rupireddy Discussion: https://postgr.es/m/CAD21AoAqvaE+XEeXHHPdAGQPcCoGXxuoeutq_nWhUSQvTt5+tA@mail.gmail.com
* Fix outdated comment in tuplesort.hDavid Rowley2022-11-02
| | | | | | This was outdated by 77bae396d. Backpatch-through: 15, where 77bae396d was added
* Remove code handling FORCE_NULL and FORCE_NOT_NULL for COPY TOMichael Paquier2022-11-02
| | | | | | | | | | These two options are only available with COPY FROM, so the extra logic in charge of checking the validity of the attributes given has no purpose. Author: Zhang Mingli Reviewed-by: Richard Guo, Kyotaro Horiguchi Discussion: https://postgr.es/m/F28F0B5A-766F-4D33-BF44-43B3A052D833@gmail.com
* Add doubly linked count list implementationDavid Rowley2022-11-02
| | | | | | | | | | | | | | | | | | | | | | | | | | We have various requirements when using a dlist_head to keep track of the number of items in the list. This, traditionally, has been done by maintaining a counter variable in the calling code. Here we tidy this up by adding "dclist", which is very similar to dlist but also keeps track of the number of items stored in the list. Callers may use the new dclist_count() function when they need to know how many items are stored. Obtaining the count is an O(1) operation. For simplicity reasons, dclist and dlist both use dlist_node as their node type and dlist_iter/dlist_mutable_iter as their iterator type. dclists have all of the same functionality as dlists except there is no function named dclist_delete(). To remove an item from a list dclist_delete_from() must be used. This requires knowing which dclist the given item is stored in. Additionally, here we also convert some dlists where additional code exists to keep track of the number of items stored and to make these use dclists instead. Author: David Rowley Reviewed-by: Bharath Rupireddy, Aleksander Alekseev Discussion: https://postgr.es/m/CAApHDvrtVxr+FXEX0VbViCFKDGxA3tWDgw9oFewNXCJMmwLjLg@mail.gmail.com
* Add more tests for COPY with incorrect option combinationsMichael Paquier2022-11-02
| | | | | | | | Based on the existing coverage report, some combinations were not checked at all, so add some tests to do so. Spotted while looking at the area. Discussion: https://postgr.es/m/Y2DNm9u7hzIxCXHn@paquier.xyz
* Update time zone data files to tzdata release 2022f.Tom Lane2022-11-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | DST law changes in Chile, Fiji, Iran, Jordan, Mexico, Palestine, and Syria. Historical corrections for Chile, Crimea, Iran, and Mexico. Also, the Europe/Kiev zone has been renamed to Europe/Kyiv (retaining the old name as a link). The following zones have been merged into nearby, more-populous zones whose clocks have agreed since 1970: Antarctica/Vostok, Asia/Brunei, Asia/Kuala_Lumpur, Atlantic/Reykjavik, Europe/Amsterdam, Europe/Copenhagen, Europe/Luxembourg, Europe/Monaco, Europe/Oslo, Europe/Stockholm, Indian/Christmas, Indian/Cocos, Indian/Kerguelen, Indian/Mahe, Indian/Reunion, Pacific/Chuuk, Pacific/Funafuti, Pacific/Majuro, Pacific/Pohnpei, Pacific/Wake and Pacific/Wallis. (This indirectly affects zones that were already links to one of these: Arctic/Longyearbyen, Atlantic/Jan_Mayen, Iceland, Pacific/Ponape, Pacific/Truk, and Pacific/Yap.) America/Nipigon, America/Rainy_River, America/Thunder_Bay, Europe/Uzhgorod, and Europe/Zaporozhye were also merged into nearby zones after discovering that their claimed post-1970 differences from those zones seem to have been errors. While the IANA crew have been working on merging zones that have no post-1970 differences for some time, this batch of changes affects some zones that are significantly more populous than those merged in the past, notably parts of Europe. The loss of pre-1970 timezone history for those zones may be troublesome for applications expecting consistency of timestamptz display. As an example, the stored value '1944-06-01 12:00 UTC' would previously display as '1944-06-01 13:00:00+01' if the Europe/Stockholm zone is selected, but now it will read out as '1944-06-01 14:00:00+02'. There exists a "packrat" option that will build the timezone data files with this old data preserved, but the problem is that it also resurrects a bunch of other, far less well-attested data; so much so that actually more zones' contents change from 2022a with that option than without it. I have chosen not to do that here, for that reason and because it appears that no major OS distributions are using the "packrat" option, so that doing so would cause Postgres' behavior to diverge significantly depending on whether it was built with --with-system-tzdata. However, for anyone for whom these changes pose significant problems, there is a solution: build a set of timezone files with the "packrat" option and use those with Postgres.
* Fix planner failure with extended statistics on partitioned tables.Tom Lane2022-11-01
| | | | | | | | | | Some cases would result in "cache lookup failed for statistics object", due to trying to fetch inherited statistics when only non-inherited ones are available or vice versa. Richard Guo and Justin Pryzby Discussion: https://postgr.es/m/20221030170520.GM16921@telsasoft.com
* psql: Improve tab completion for ALTER TABLE on identity columnsPeter Eisentraut2022-11-01
| | | | | | | | | | | - Add tab completion for ALTER SEQUENCE … START … - Add tab completion for ALTER COLUMN … SET GENERATED … - Add tab completion for ALTER COLUMN … SET <sequence option> - Add tab completion for ALTER COLUMN … ADD GENERATED … AS IDENTITY Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Reviewed-by: Matheus Alcantara <mths.dev@pm.me> Discussion: https://www.postgresql.org/message-id/flat/87mta1jfax.fsf@wibble.ilmari.org
* Add basic regression tests for semi/antijoin recognition.Tom Lane2022-10-31
| | | | | | | | | | | Add some simple tests that the planner recognizes all the standard idioms for SEMI and ANTI joins. Failure to optimize in this way won't necessarily cause any visible change in query results, so check the plans. We had no similar coverage before, at least for some variants of antijoin, as noted by Richard Guo. Discussion: https://postgr.es/m/CAMbWs4-mvPPCJ1W6iK6dD5HiNwoJdi6mZp=-7mE8N9Sh+cd0tQ@mail.gmail.com
* Fix ALTER COLLATION "default" REFRESH VERSION.Jeff Davis2022-10-31
| | | | | | | Issue a helpful error message rather than an internal error. Discussion: https://postgr.es/m/51fb77507cafd43fc1a2e733c23045873d93ae60.camel%40j-davis.com Reviewed-by: Thomas Munro
* Enable pg_collation_actual_version() to work on the default collation.Jeff Davis2022-10-31
| | | | | | | Previously, it would simply return NULL, which was less useful. Discussion: https://postgr.es/m/51fb77507cafd43fc1a2e733c23045873d93ae60.camel%40j-davis.com Reviewed-by: Thomas Munro
* pg_dump test: Make concatenated create_sql commands more readablePeter Eisentraut2022-10-31
| | | | | | | | | | | | | | | | | When the pg_dump 002_pg_dump.pl test generates the command to load the schema, it does # Add terminating semicolon $create_sql{$test_db} .= $tests{$test}->{create_sql} . ";"; In some cases, this creates a duplicate semicolon, but more importantly, this doesn't add any newline. So if you look at the result in either the server log or in tmp_check/log/regress_log_002_pg_dump, it looks like a complete mess. This patch makes the output look cleaner for manual inspection: add semicolon only if necessary, and add two newlines. Discussion: https://www.postgresql.org/message-id/flat/d6aec95a-8729-43cc-2578-f2a5e46640e0%40enterprisedb.com
* Add check on initial and boot values when loading GUCsMichael Paquier2022-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a function to perform a cross-check between the initial value of the C declaration associated to a GUC and its actual boot value in assert-enabled builds. The purpose of this is to prevent anybody reading these C declarations from being fooled by mismatched values before they are loaded at program startup. The following rules apply depending on the GUC type: * bool - can be false, or same as boot_val. * int - can be 0, or same as the boot_val. * real - can be 0.0, or same as the boot_val. * string - can be NULL, or strcmp'd equal to the boot_val. * enum - equal to the boot_val. This is done for the system as well custom GUCs loaded by external modules, which may require extension developers to adapt the C declaration of the variables used by these GUCs (testing this change with some of my own modules has allowed me to catch some stupid typos, FWIW). This may finish by being a bad experiment depending on the feedbcak received, but let's see how it goes. Author: Peter Smith Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, Justin Pryzby Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
* Clean up some inconsistencies with GUC declarationsMichael Paquier2022-10-31
| | | | | | | | | | | | | | | | | | | | This is similar to 7d25958, and this commit takes care of all the remaining inconsistencies between the initial value used in the C variable associated to a GUC and its default value stored in the GUC tables (as of pg_settings.boot_val). Some of the initial values of the GUCs updated rely on a compile-time default. These are refactored so as the GUC table and its C declaration use the same values. This makes everything consistent with other places, backend_flush_after, bgwriter_flush_after, port, checkpoint_flush_after doing so already, for example. Extracted from a larger patch by Peter Smith. The spots updated in the modules are from me. Author: Peter Smith, Michael Paquier Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
* Under has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.Noah Misch2022-10-29
| | | | | | | Per buildfarm member kittiwake. Back-patch to v15, where this test first appeared. Discussion: https://postgr.es/m/20220116210241.GC756210@rfd.leadboat.com
* Use Limit instead of Unique to implement DISTINCT, when possibleDavid Rowley2022-10-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When all of the query's DISTINCT pathkeys have been marked as redundant due to EquivalenceClasses existing which contain constants, we can just implement the DISTINCT operation on a query by just limiting the number of returned rows to 1 instead of performing a Unique on all of the matching (duplicate) rows. This applies in cases such as: SELECT DISTINCT col,col2 FROM tab WHERE col = 1 AND col2 = 10; If there are any matching rows, then they must all be {1,10}. There's no point in fetching all of those and running a Unique operator on them to leave only a single row. Here we effectively just find the first row and then stop. We are obviously unable to apply this optimization if either the col = 1 or col2 = 10 were missing from the WHERE clause or if there were any additional columns in the SELECT clause. Such queries are probably not all that common, but detecting when we can apply this optimization amounts to checking if the distinct_pathkeys are NULL, which is very cheap indeed. Nothing is done here to check if the query already has a LIMIT clause. If it does then the plan may end up with 2 Limits nodes. There's no harm in that and it's probably not worth the complexity to unify them into a single Limit node. Author: David Rowley Reviewed-by: Richard Guo Discussion: https://postgr.es/m/CAApHDvqS0j8RUWRUSgCAXxOqnYjHUXmKwspRj4GzVfOO25ByHA@mail.gmail.com Discussion: https://postgr.es/m/MEYPR01MB7101CD5DA0A07C9DE2B74850A4239@MEYPR01MB7101.ausprd01.prod.outlook.com
* Remove AssertArg and AssertStatePeter Eisentraut2022-10-28
| | | | | | | | | These don't offer anything over plain Assert, and their usage had already been declared obsolescent. Author: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/20221009210148.GA900071@nathanxps13
* Allow nodeSort to perform Datum sorts for byref typesDavid Rowley2022-10-28
| | | | | | | | | | | | | | | | | | | | | | | | Here we add a new 'copy' parameter to tuplesort_getdatum so that we can instruct the function not to datumCopy() byref Datums before returning. Similar to 91e9e89dc, this can provide significant performance improvements in nodeSort when sorting by a single byref column and the sort's targetlist contains only that column. This allows us to re-enable Datum sorts for byref types which was disabled in 3a5817695 due to a reported memory leak. Additionally, here we slightly optimize DISTINCT aggregates so that we no longer perform any datumCopy() when we find the current value not to be distinct from the previous value. Previously the code would always take a copy of the most recent Datum and pfree the previous value, even when the values were the same. Testing shows a small but noticeable performance increase when aggregate transitions are skipped due to the current transition value being the same as the prior one. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvqS6wC5U==k9Hd26E4EQXH3QR67-T4=Q1rQ36NGvjfVSg@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvqHonfe9G1cVaKeHbDx70R_zCrM3qP2AGXpGrieSKGnhA@mail.gmail.com
* Avoid making commutatively-duplicate clauses in EquivalenceClasses.Tom Lane2022-10-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we decide we need to make a derived clause equating a.x and b.y, we already will re-use a previously-made clause "a.x = b.y". But we might instead have "b.y = a.x", which is perfectly usable because equivclass.c has never promised anything about the operand order in clauses it builds. Saving construction of a new RestrictInfo doesn't matter all that much in itself --- but because we cache selectivity estimates and so on per-RestrictInfo, there's a possibility of saving a fair amount of duplicative effort downstream. Hence, check for commutative matches as well as direct ones when seeing if we have a pre-existing clause. This changes the visible clause order in several regression test cases, but they're all clearly-insignificant changes. Checking for the reverse operand order is simple enough, but if we wanted to check for operator OID match we'd need to call get_commutator here, which is not so cheap. I concluded that we don't really need the operator check anyway, so I just removed it. It's unlikely that an opfamily contains more than one applicable operator for a given pair of operand datatypes; and if it does they had better give the same answers, so there seems little need to insist that we use exactly the one select_equality_operator chose. Using the current core regression suite as a test case, I see this change reducing the number of new join clauses built by create_join_clause from 9673 to 5142 (out of 26652 calls). So not quite 50% savings, but pretty close to it. Discussion: https://postgr.es/m/78062.1666735746@sss.pgh.pa.us
* Move pg_pwritev_with_retry() to src/common/file_utils.cMichael Paquier2022-10-27
| | | | | | | | | | | | | This commit moves pg_pwritev_with_retry(), a convenience wrapper of pg_writev() able to handle partial writes, to common/file_utils.c so that the frontend code is able to use it. A first use-case targetted for this routine is pg_basebackup and pg_receivewal, for the zero-padding of a newly-initialized WAL segment. This is used currently in the backend when the GUC wal_init_zero is enabled (default). Author: Bharath Rupireddy Reviewed-by: Nathan Bossart, Thomas Munro Discussion: https://postgr.es/m/CALj2ACUq7nAb7=bJNbK3yYmp-SZhJcXFR_pLk8un6XgDzDF3OA@mail.gmail.com
* Add some tests to check the SQL functions of control fileMichael Paquier2022-10-27
| | | | | | | | | | | | | | | | | | | | | As the recent commit 05d4cbf (reverted after as a448e49) has proved, there is zero coverage for the four SQL functions that can scan the control file data: - pg_control_checkpoint() - pg_control_init() - pg_control_recovery() - pg_control_system() This commit adds a minimal coverage for these functions, checking that their execution is able to complete. This would have been enough to catch the problems introduced in the commit mentioned above. More checks could be done for each individual fields, but it is unclear whether this would be better than the other checks in place in the backend code. Per discussion with Bharath Rupireddy. Discussion: https://postgr.es/m/Y1d2FZmQmyAhPSRG@paquier.xyz
* Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappingsMichael Paquier2022-10-26
| | | | | | | | | | | | | | | | | | | | These numbers are strictly-monotone identifiers assigned to each rule of pg_hba_file_rules and each map of pg_ident_file_mappings when loading the HBA and ident configuration files, indicating the order in which they are checked at authentication time, until a match is found. With only one file loaded currently, this is equivalent to the line numbers assigned to the entries loaded if one wants to know their order, but this becomes mandatory once the inclusion of external files is added to the HBA and ident files to be able to know in which order the rules and/or maps are applied at authentication. Note that NULL is used when a HBA or ident entry cannot be parsed or validated, aka when an error exists, contrary to the line number. Bump catalog version. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Fix variable assignment thinko in hba.cMichael Paquier2022-10-26
| | | | | | | | | | | The intention behind 1b73d0b was to limit the use of TokenizedAuthLine, but I have fat-fingered one location in parse_hba_line() when creating the HbaLine, where this should use the local variable and not the value coming from TokenizedAuthLine. This logic is the exactly the same, but let's be clean about all that on consistency grounds. Reported-by: Julien Rouhaud Discussion: https://postgr.es/m/20221026032730.k3sib5krgm7l6njk@jrouhaud
* Refactor code handling the names of files loaded in hba.cMichael Paquier2022-10-26
| | | | | | | | | | | | | | | | | | | | | | | | This has the advantage to limit the presence of the GUC values hba_file and ident_file to the code paths where these files are loaded, easing the introduction of an upcoming feature aimed at adding inclusion logic for files and directories in HBA and ident files. Note that this needs the addition of the source file name to HbaLine, in addition to the line number, which is something needed by the backend in two places of auth.c (authentication failure details and auth_id log when log_connections is enabled). While on it, adjust a log generated on authentication failure to report the name of the actual HBA file on which the connection attempt matched, where the line number and the raw line written in the HBA file were already included. This was previously hardcoded as pg_hba.conf, which would be incorrect when a custom value is used at postmaster startup for the GUC hba_file. Extracted from a larger patch by the same author. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Doc/improve confusing, inefficient tests to locate CTID variable.Tom Lane2022-10-25
| | | | | | | | | | | | | | | | | The IsCTIDVar() tests in nodeTidscan.c and nodeTidrangescan.c look buggy at first sight: they aren't checking that the varno matches the table to be scanned. Actually they're safe because any Var in a scan-level qual must be for the correct table ... but if we're depending on that, it's pretty pointless to verify varlevelsup. (Besides which, varlevelsup is *always* zero at execution, since we've flattened the rangetable long since.) Remove the useless varlevelsup check, and instead add some commentary explaining why we don't need to check varno. Noted while fooling with a planner change that causes the order of "t1.ctid = t2.ctid" to change in some tidscan.sql tests; I was briefly fooled into thinking there was a live bug here.
* Update outdated comment for TransactionIdSetTreeStatusHeikki Linnakangas2022-10-25
| | | | | | | | | Commit 06da3c570f changed the way subtransactions are marked as SUBCOMMITTED, but the example it included actually documented the old way. Update it. Author: Japin Li Discussion: https://www.postgresql.org/message-id/MEYP282MB16690BC96DFBE08CC857E1E3B6319%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Clean up some GUC declarations and commentsMichael Paquier2022-10-25
| | | | | | | | | | This adjusts a few things for GUCs related to logical replication, replication slots and WAL senders, in the shape of incorrect comments and values inconsistent with their initial default value. Author: Peter Smith Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com
* Fix unlink() for STATUS_DELETE_PENDING on Windows.Thomas Munro2022-10-25
| | | | | | | | | | | | | | | | | | | | Commit f357233c assumed that it was OK to return ENOENT directly if lstat() failed that way. If we got STATUS_DELETE_PENDING while trying to unlink a file that we had already unlinked successfully once before but someone else still had open (on a kernel version that has "pending" unlinks by default), then we would no longer reach the retry loop in pgunlink(). That loop claims to be only for handling sharing violations (a different phenomenon), but the errno is the same. Restore that behavior with an explicit check, to see if it fixes the occasional 'directory not empty' failures seen in the pg_upgrade tests on CI. Further improvements are possible with proposed upgrades to modern Windows APIs that would replace this convoluted code. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20220920013122.GA31833%40telsasoft.com Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
* Fix stat() for recursive junction points on Windows.Thomas Munro2022-10-25
| | | | | | | | | | | | | | | | Commit c5cb8f3b supposed that we'd only ever have to follow one junction point in stat(), because we don't construct longer chains of them ourselves. When examining a parent directory supplied by the user, we should really be able to cope with longer chains, just in case someone has their system set up that way. Choose an arbitrary cap of 8, to match the minimum acceptable value of SYMLOOP_MAX in POSIX. Previously I'd avoided reporting ELOOP thinking Windows didn't have it, but it turns out that it does, so we can use the proper error number. Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru> Discussion: https://postgr.es/m/CA%2BhUKGJ7JDGWYFt9%3D-TyJiRRy5q9TtPfqeKkneWDr1XPU1%2Biqw%40mail.gmail.com Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
* Fix readlink() for non-PostgreSQL junction points on Windows.Thomas Munro2022-10-25
| | | | | | | | | | | | | | | | | | | Since commit c5cb8f3b taught stat() to follow symlinks, and since initdb uses pg_mkdir_p(), and that examines parent directories, our humble readlink() implementation can now be exposed to junction points not of PostgreSQL origin. Those might be corrupted by our naive path mangling, which doesn't really understand NT paths in general. Simply decline to transform paths that don't look like a drive absolute path. That means that readlink() returns the NT path directly when checking a parent directory of PGDATA that happen to point to a drive using "rooted" format. That works for the purposes of our stat() emulation. Reported-by: Roman Zharkov <r.zharkov@postgrespro.ru> Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru> Discussion: https://postgr.es/m/4590c37927d7b8ee84f9855d83229018%40postgrespro.ru Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
* Fix lstat() for broken junction points on Windows.Thomas Munro2022-10-25
| | | | | | | | | | | | | When using junction points to emulate symlinks on Windows, one edge case was not handled correctly by commit c5cb8f3b: if a junction point is broken (pointing to a non-existent path), we'd report ENOENT. This doesn't break any known use case, but was noticed while developing a test suite for these functions and is fixed here for completeness. Also add translation ERROR_CANT_RESOLVE_FILENAME -> ENOENT, as that is one of the errors Windows can report for some kinds of broken paths. Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
* Fix readlink() return value on Windows.Thomas Munro2022-10-25
| | | | | | Ancient bug noticed while working on a test suite for these functions. Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
* Fix symlink() errno on Windows.Thomas Munro2022-10-25
| | | | | | Ancient bug noticed while working on a test suite for these functions. https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com
* Update some comments that should've covered MERGEAlvaro Herrera2022-10-24
| | | | | | | Oversight in 7103ebb7aae8. Backpatch to 15. Author: Richard Guo <guofenglinux@gmail.com> Discussion: https://postgr.es/m/CAMbWs48gnDjZXq3-b56dVpQCNUJ5hD9kdtWN4QFwKCEapspNsA@mail.gmail.com
* Fix recently added incorrect assertionAlvaro Herrera2022-10-24
| | | | | | | | | | Commit df3737a651f4 added an incorrect assertion about the preconditions for invoking the backup cleanup callback: it misfires at session end in case a backup completes successfully. Fix it, using coding from Michaël Paquier. Also add some tests for the various cases. Reported by Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20221021.161038.1277961198945653224.horikyota.ntt@gmail.com
* Improve coverage of ruleutils.c for SQLValueFunctionsMichael Paquier2022-10-24
| | | | | | | | | | | | | While looking at how these are handled in the parser and the executor, I have noticed that there is no test coverage for most of these when reverse-engineering an expression for a SQLValueFunction node in ruleutils.c, including how these are reparsed when included in a FROM clause. Some hacking in this area has showed me that these could break easily, so add some coverage to track the existing compatibility. Extracted from a much larger patch by me. Discussion: https://postgr.es/m/YzaG3MoryCguUOym@paquier.xyz
* Improve tab completion for ALTER STATISTICS <name> SET in psqlMichael Paquier2022-10-24
| | | | | | | | | The code was completing this pattern with a list of settable characters, and it was possible to reach this state after completing a "ALTER STATISTICS <name>" with SET. Author: Vignesh C Discussion: https://postgr.es/m/CALDaNm2HHF_371o+EeSjxDDS17Cx7d-ko2h1fLU94=ob=4_ktg@mail.gmail.com
* Fix and improve TAP tests for pg_hba.conf and regexpsMichael Paquier2022-10-24
| | | | | | | | | | | | | | | | The new tests have been reporting a warning hidden in the logs, as of "Odd number of elements in hash assignment" (perlcritic or similar did not report an issue, actually). This comes down to a typo in the test "matching regexp for username" for a double-quoted regexp using commas, where we passed an extra argument. The test is intended to pass, but this was causing the test to fail. This also pointed out that the newly-added role "md5,role" lacks an entry in the password file used to provide the password, so add one. While on it, make the tests pickier by checking the contents of the logs generated on successful authentication. Oversights in 8fea868.
* Add support for regexps on database and user entries in pg_hba.confMichael Paquier2022-10-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As of this commit, any database or user entry beginning with a slash (/) is considered as a regular expression. This is particularly useful for users, as now there is no clean way to match pattern on multiple HBA lines. For example, a user name mapping with a regular expression needs first to match with a HBA line, and we would skip the follow-up HBA entries if the ident regexp does *not* match with what has matched in the HBA line. pg_hba.conf is able to handle multiple databases and roles with a comma-separated list of these, hence individual regular expressions that include commas need to be double-quoted. At authentication time, user and database names are now checked in the following order: - Arbitrary keywords (like "all", the ones beginning by '+' for membership check), that we know will never have a regexp. A fancy case is for physical WAL senders, we *have* to only match "replication" for the database. - Regular expression matching. - Exact match. The previous logic did the same, but without the regexp step. We have discussed as well the possibility to support regexp pattern matching for host names, but these happen to lead to tricky issues based on what I understand, particularly with host entries that have CIDRs. This commit relies heavily on the refactoring done in a903971 and fc579e1, so as the amount of code required to compile and execute regular expressions is now minimal. When parsing pg_hba.conf, all the computed regexps needs to explicitely free()'d, same as pg_ident.conf. Documentation and TAP tests are added to cover this feature, including cases where the regexps use commas (for clarity in the docs, coverage for the parsing logic in the tests). Note that this introduces a breakage with older versions, where a database or user name beginning with a slash are treated as something to check for an equal match. Per discussion, we have discarded this as being much of an issue in practice as it would require a cluster to have database and/or role names that begin with a slash, as well as HBA entries using these. Hence, the consistency gained with regexps in pg_ident.conf is more appealing in the long term. **This compatibility change should be mentioned in the release notes.** Author: Bertrand Drouvot Reviewed-by: Jacob Champion, Tom Lane, Michael Paquier Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com
* Remove pgpid_t type, use pid_t insteadPeter Eisentraut2022-10-22
| | | | | | | | | | | | | It's unclear why a separate type would be needed here. We use plain pid_t (or int) everywhere else. (The only relevant platform where pid_t is not int is 64-bit MinGW, where it is long long int. So defining pid_t as long (which is 32-bit on Windows), as was done here, doesn't even accommodate that one.) Reverts 66fa6eba5a61be740a6c07de92c42221fae79e9c. Discussion: https://www.postgresql.org/message-id/289c2e45-c7d9-5ce4-7eff-a9e2a33e1580@enterprisedb.com
* psql: Fix exit status when query is canceledPeter Eisentraut2022-10-22
| | | | | | Because of a small thinko in 7844c9918a43b494adde3575891d217a37062378, psql -c would exit successfully when a query is canceled. Fix this so that it exits with a nonzero status, just like for all other errors.
* Improve memory handling across SQL-callable backup functionsMichael Paquier2022-10-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since pg_backup_start() and pg_backup_stop() exist, the tablespace map data and the backup state data (backup_label string until 7d70809) have been allocated in the TopMemoryContext. This approach would cause memory leaks in the session calling these functions if failures happen before pg_backup_stop() ends, leaking more memory on repeated failures. Both things need little memory so that would not be really noticeable for most users, except perhaps connection poolers with long-lived connections able to trigger backup failures with these functions. This commit improves the logic in this area by not allocating anymore the backup-related data that needs to travel across the SQL-callable backup functions in TopMemoryContext, by using instead a dedicated memory context child of TopMemoryContext. The memory context is created in pg_backup_start() and deleted when finishing pg_backup_stop(). In the event of an in-flight failure, this memory context gets reset in the follow-up pg_backup_start() call, so as we are sure that only one run worth of data is leaked at any time. Some cleanup was already done for the backup data on a follow-up call of pg_backup_start(), but using a memory context makes the whole simpler. BASE_BACKUP commands are executed in isolation, relying on the memory context created for replication commands, hence these do not need such an extra logic. Author: Bharath Rupireddy Reviewed-by: Robert Haas, Alvaro Herrera, Cary Huang, Michael Paquier Discussion: https://postgr.es/m/CALj2ACXqvfKF2B0beQ=aJMdWnpNohmBPsRg=EDQj_6y1t2O8mQ@mail.gmail.com
* pg_basebackup: Fix cross-platform tablespace relocation.Robert Haas2022-10-21
| | | | | | | | | | | | | | Specifically, when pg_basebackup is invoked with -Tx=y, don't error out if x could plausibly be an absolute path either on Windows or on non-Windows systems. We don't know whether the remote system is running the same OS as the local system, so it's not appropriate to assume that our local rule about absolute pathnames is the same as the rule on the remote system. Patch by me, reviewed by Tom Lane, Andrew Dunstan, and Davinder Singh. Discussion: http://postgr.es/m/CA+TgmoY+jC3YiskomvYKDPK3FbrmsDU7_8+wMHt02HOdJeRb0g@mail.gmail.com
* Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.Amit Kapila2022-10-21
| | | | | | | | | | | Previously in commit 42681dffaf, we added CFI during decoding changes but missed another similar case that can happen while restoring changes spilled to disk back into memory in a loop. Reported-by: Robert Haas Author: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/CA+TgmoaLObg0QbstbC8ykDwOdD1bDkr4AbPpB=0DPgA2JW0mFg@mail.gmail.com
* Refactor more logic for compilation of regular expressions in hba.cMichael Paquier2022-10-21
| | | | | | | | | | | | | | | | | | | | It happens that the parts of hba.conf that are planned to be extended to support regular expressions would finish by using the same error message as the one used currently for pg_ident.conf when a regular expression cannot be compiled, as long as the routine centralizing the logic, regcomp_auth_token(), knows from which file the regexp comes from and its line location in the so-said file. This change makes the follow-up patches slightly simpler, and the logic remains the same. I suspect that this makes the proposal to add support for file inclusions in pg_ident.conf and pg_hba.conf slightly simpler, as well. Extracted from a larger patch by the same author. This is similar to the refactoring done in fc579e1. Author: Bertrand Drouvot Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com
* Make finding openssl program a configure or meson optionPeter Eisentraut2022-10-20
| | | | | | | | | | | | | | | | | Various test suites use the "openssl" program as part of their setup. There isn't a way to override which openssl program is to be used, other than by fiddling with the path, perhaps. This has gotten increasingly problematic because different versions of openssl have different capabilities and do different things by default. This patch checks for an openssl binary in configure and meson setup, with appropriate ways to override it. This is similar to how "lz4" and "zstd" are handled, for example. The meson build system actually already did this, but the result was only used in some places. This is now applied more uniformly. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/dc638b75-a16a-007d-9e1c-d16ed6cf0ad2%40enterprisedb.com
* Improve the accuracy of numeric power() for integer exponents.Dean Rasheed2022-10-20
| | | | | | | | | | | | | | | | | | | | | | | | | | This makes the choice of result scale of numeric power() for integer exponents consistent with the choice for non-integer exponents, and with the result scale of other numeric functions. Specifically, the result scale will be at least as large as the scale of either input, and sufficient to ensure that the result has at least 16 significant digits. Formerly, the result scale was based only on the scale of the first input, without taking into account the weight of the result. For results with negative weight, that could lead to results with very few or even no non-zero significant digits (e.g., 10.0 ^ (-18) produced 0.0000000000000000). Fix this by moving responsibility for the choice of result scale into power_var_int(), which already has code to estimate the result weight. Per report by Adrian Klaver and suggested fix by Tom Lane. No back-patch -- arguably this is a bug fix, but one which is easy to work around, so it doesn't seem worth the risk of changing query results in stable branches. Discussion: https://postgr.es/m/12a40226-70ac-3a3b-3d3a-fdaf9e32d312%40aklaver.com
* Use proper macro to access TransactionIdAlvaro Herrera2022-10-20
| | | | | | | | | | In commit f10a025cfe97 I mistakenly used list_member_oid in a place where list_member_xid is called for. (Currently innocuous as both typedefs are pretty much identical, but if we change either, it'll become broken.) Repair. Author: Hou Zhijie <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/OS0PR01MB5716E2399494D4CB1A28A091942A9@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Fix assertion failures while processing NEW_CID record in logical decoding.Amit Kapila2022-10-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | When the logical decoding restarts from NEW_CID, since there is no association between the top transaction and its subtransaction, both are created as top transactions and have the same LSN. This caused the assertion failure in AssertTXNLsnOrder(). This patch skips the assertion check until we reach the LSN at which we start decoding the contents of the transaction, specifically start_decoding_at LSN in SnapBuild. This is okay because we don't guarantee to make the association between top transaction and subtransaction until we try to decode the actual contents of transaction. The ordering of the records prior to the start_decoding_at LSN should have been checked before the restart. The other assertion failure is due to the reason that we forgot to track that we have considered top-level transaction id in the list of catalog changing transactions that were committed when one of its subtransactions is marked as containing catalog change. Reported-by: Tomas Vondra, Osumi Takamichi Author: Masahiko Sawada, Kuroda Hayato Reviewed-by: Amit Kapila, Dilip Kumar, Kuroda Hayato, Kyotaro Horiguchi, Masahiko Sawada Backpatch-through: 10 Discussion: https://postgr.es/m/a89b46b6-0239-2fd5-71a9-b19b1f7a7145%40enterprisedb.com Discussion: https://postgr.es/m/TYCPR01MB83733C6CEAE47D0280814D5AED7A9%40TYCPR01MB8373.jpnprd01.prod.outlook.com