summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-10-27Move pg_pwritev_with_retry() to src/common/file_utils.cMichael Paquier
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
2022-10-27Add some tests to check the SQL functions of control fileMichael Paquier
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
2022-10-26Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappingsMichael Paquier
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
2022-10-26Fix variable assignment thinko in hba.cMichael Paquier
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
2022-10-26Refactor code handling the names of files loaded in hba.cMichael Paquier
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
2022-10-25Doc/improve confusing, inefficient tests to locate CTID variable.Tom Lane
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.
2022-10-25Update outdated comment for TransactionIdSetTreeStatusHeikki Linnakangas
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
2022-10-25Clean up some GUC declarations and commentsMichael Paquier
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
2022-10-25Fix unlink() for STATUS_DELETE_PENDING on Windows.Thomas Munro
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
2022-10-25Fix stat() for recursive junction points on Windows.Thomas Munro
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
2022-10-25Fix readlink() for non-PostgreSQL junction points on Windows.Thomas Munro
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
2022-10-25Fix lstat() for broken junction points on Windows.Thomas Munro
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
2022-10-25Fix readlink() return value on Windows.Thomas Munro
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
2022-10-25Fix symlink() errno on Windows.Thomas Munro
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
2022-10-25doc: Fix type of cursor_position in jsonlog tableMichael Paquier
This entry was listed as a "string", but it is a "number. The other fields are correctly described, on a second look. Reported-by: Nuko Yokohama Author: Tatsuo Ishii Discussion: https://postgr.es/m/CAF3Gu1awoVoDP5d0_eN=cR=QkGVwH+OtFvwJkkc5cB_ZMWjyeA@mail.gmail.com Backpatch-through: 15
2022-10-24Update some comments that should've covered MERGEAlvaro Herrera
Oversight in 7103ebb7aae8. Backpatch to 15. Author: Richard Guo <guofenglinux@gmail.com> Discussion: https://postgr.es/m/CAMbWs48gnDjZXq3-b56dVpQCNUJ5hD9kdtWN4QFwKCEapspNsA@mail.gmail.com
2022-10-24Fix recently added incorrect assertionAlvaro Herrera
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
2022-10-24Improve coverage of ruleutils.c for SQLValueFunctionsMichael Paquier
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
2022-10-24Improve tab completion for ALTER STATISTICS <name> SET in psqlMichael Paquier
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
2022-10-24Fix and improve TAP tests for pg_hba.conf and regexpsMichael Paquier
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.
2022-10-24Add support for regexps on database and user entries in pg_hba.confMichael Paquier
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
2022-10-22Remove pgpid_t type, use pid_t insteadPeter Eisentraut
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
2022-10-22psql: Fix exit status when query is canceledPeter Eisentraut
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.
2022-10-22Improve memory handling across SQL-callable backup functionsMichael Paquier
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
2022-10-21pg_basebackup: Fix cross-platform tablespace relocation.Robert Haas
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
2022-10-21Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.Amit Kapila
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
2022-10-21Refactor more logic for compilation of regular expressions in hba.cMichael Paquier
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
2022-10-21Doc: fix outdated wording about parallel seq scansDavid Rowley
56788d215 adjusted the parallel seq scan code so that instead of handing out a single block at a time to parallel workers, it now hands out ranges of blocks. Here we update the documentation which still claimed that workers received just 1 block at a time. Reported-by: Zhang Mingli Discussion: https://postgr.es/m/17c99615-2c3b-4e4e-9d0b-424a66a7bccd@Spark Backpatch-through: 14, where 56788d215 was added.
2022-10-20Make finding openssl program a configure or meson optionPeter Eisentraut
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
2022-10-20Improve the accuracy of numeric power() for integer exponents.Dean Rasheed
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
2022-10-20Use proper macro to access TransactionIdAlvaro Herrera
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
2022-10-20Fix assertion failures while processing NEW_CID record in logical decoding.Amit Kapila
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
2022-10-19Better handle interrupting TAP testsAlvaro Herrera
Set up a signal handler for INT/TERM so that we run our END block if we get them. In END, if the exit status indicates a problem, call _update_pid(-1) to improve chances of the stop working in case start() hasn't returned yet. Also, change END's teardown_node() so that it passes fail_ok=>1, so that if a node fails to stop, we still stop the other nodes in the same test. Per complaint from Andres Freund. This doesn't seem important enough to backpatch, at least for now. Discussion: https://postgr.es/m/20220930040734.mbted42oiynhn2t6@awork3.anarazel.de
2022-10-19Get rid of XLogCtlInsert->forcePageWritesAlvaro Herrera
After commit 39969e2a1e4d, ->forcePageWrites is no longer very interesting: we can just test whether runningBackups is different from 0. This simplifies some code, so do away with it. Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/39969e2a1e4d7f5a37f3ef37d53bbfe171e7d77a
2022-10-19Track LLVM 15 changes.Thomas Munro
Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque pointers still exists and we can request that on our context. We have until LLVM 16 to move to opaque pointers, a much larger change. Back-patch to 11, where LLVM support arrived. Author: Thomas Munro <thomas.munro@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAMHz58Sf_xncdyqsekoVsNeKcruKootLtVH6cYXVhhUR1oKPCg%40mail.gmail.com
2022-10-19Remove pg_backup_start_callback and reuse similar codeAlvaro Herrera
We had two copies of almost identical logic to revert shared memory state when a running backup aborts; we can remove pg_backup_start_callback if we adapt do_pg_abort_backup so that it can be used for this purpose too. However, in order for this to work, we have to repurpose the flag passed to do_pg_abort_backup. It used to indicate whether to throw a warning (and the only caller always passed true). It now indicates whether the callback is being called at start time (in which case the session backup state is known not to have been set to RUNNING yet, so action is always taken) or shmem time (in which case action is only taken if the session backup state is RUNNING). Thus the meaning of the flag is no longer superfluous, but it's actually quite critical to get right. I (Álvaro) chose to change the polarity and the code flow re. the flag from what Bharath submitted, for coding clarity. Co-authored-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://www.postgresql.org/message-id/20221013111330.564fk5tkwe3ha77l%40alvherre.pgsql
2022-10-19Rework shutdown callback of archiver modulesMichael Paquier
As currently designed, with a callback registered in a ERROR_CLEANUP block, the shutdown callback would get called twice when updating archive_library on SIGHUP, which is something that we want to avoid to ease the life of extension writers. Anyway, an ERROR in the archiver process is treated as a FATAL, stopping it immediately, hence there is no need for a ERROR_CLEANUP block. Instead of that, the shutdown callback is not called upon before_shmem_exit(), giving to the modules the opportunity to do any cleanup actions before the server shuts down its subsystems. While on it, this commit adds some testing coverage for the shutdown callback. Neither shell_archive nor basic_archive have been using it, and one is added to shell_archive, whose trigger is checked in a TAP test through a shutdown sequence. Author: Nathan Bossart, Bharath Rupireddy Reviewed-by: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20221015221328.GB1821022@nathanxps13 Backpatch-through: 15
2022-10-19Enhance make_ctags and make_etags.Tatsuo Ishii
make_ctags did not include field members of structs since the commit 964d01ae90c314eb31132c2e7712d5d9fc237331. For example, in the following field of RestrictInfo: Selectivity norm_selec pg_node_attr(equal_ignore); pg_node_attr was mistakenly interpreted to be the name of the field. To fix this, add -I option to ctags command if the command is Exuberant ctags or Universal ctags (for plain old ctags, struct members are not included in the tags file anyway). Also add "-e" and "-n" options to make_ctags. The -e option invokes ctags command with -e option, which produces TAGS file for emacs. This allows to eliminate duplicate codes in make_etags so that make_etags just exec make_ctags with -e option. The -n option allows not to produce symbolic links in each sub directory (the default is producing symbolic links). Author: Yugo Nagata Reviewers: Alvaro Herrera, Tatsuo Ishii Discussion: https://postgr.es/m/flat/20221007154442.76233afc7c5b255c4de6528a%40sraoss.co.jp
2022-10-19Fix typos in logical/launcher.cMichael Paquier
Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pvbma5HCc7==-B1ycyLQVyu7Fqq-qV=jhC5Zx4pWqk3uw@mail.gmail.com
2022-10-19Refactor regular expression handling in hba.cMichael Paquier
AuthToken gains a regular expression, and IdentLine is changed so as it uses an AuthToken rather than tracking separately the ident user string used for the regex compilation and its generated regex_t. In the case of pg_ident.conf, a set of AuthTokens is built in the pre-parsing phase of the file, and an extra regular expression is compiled when building the list of IdentLines, after checking the sanity of the fields in a pre-parsed entry. The logic in charge of computing and executing regular expressions is now done in a new set of routines called respectively regcomp_auth_token() and regexec_auth_token() that are wrappers around pg_regcomp() and pg_regexec(), working on AuthTokens. While on it, this patch adds a routine able to free an AuthToken, free_auth_token(), to simplify a bit the logic around the requirement of using a specific free routine for computed regular expressions. Note that there are no functional or behavior changes introduced by this commit. The goal of this patch is to ease the use of regular expressions with more items of pg_hba.conf (user list, database list, potentially hostnames) where AuthTokens are used extensively. This will be tackled later in a separate patch. Author: Bertrand Drouvot, Michael Paquier Discussion: https://postgr.es/m/fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com
2022-10-18Fix confusion about havingQual vs hasHavingQual in planner.Tom Lane
Preprocessing of the HAVING clause will reduce havingQual to NIL if the clause is constant-TRUE. This is one case where that convention is rather unfortunate, because "HAVING TRUE" is not at all the same as not having any HAVING clause at all. (Per the SQL spec, it still forces the query to be grouped.) The planner deals with this by having a boolean hasHavingQual that records whether havingQual was originally nonempty; places that just want to check whether HAVING was specified are supposed to consult that. I found three places that got that wrong. Fortunately, these could only affect cost estimates not correctness. It'd be hard even to demonstrate the errors; for example, the one in allpaths.c would only matter in a query that has HAVING TRUE but no GROUP BY and no aggregates, which would require a completely variable-free SELECT list, making the case probably of only academic interest. Hence, while these are worth fixing before someone copies the incorrect coding somewhere more critical, they don't seem worth back-patching. I didn't bother trying to devise regression tests, either. Discussion: https://postgr.es/m/2503888.1666042643@sss.pgh.pa.us
2022-10-18Remove no-longer-needed compatibility hackAlvaro Herrera
Our Perl version requirement was raised to 5.14 by commit 4c1532763a00 Discussion: https://postgr.es/m/20221017081649.fjcd2kjqif77uyf2@alvherre.pgsql
2022-10-18Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATIONAlvaro Herrera
The original hint says to use SET PUBLICATION when really ADD/DROP PUBLICATION is called for, so this is arguably a bug fix. Also, a very similar message elsewhere was using an inconsistent SQLSTATE. While at it, unwrap some strings. Backpatch to 15. Author: Hou zj <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/OS0PR01MB57160AD0E7386547BA978EB394299@OS0PR01MB5716.jpnprd01.prod.outlook.com
2022-10-18Remove compatibility declarations for InitMaterializedSRF()Michael Paquier
These routines have been renamed in a19e5ce. There is no need to keep the compatibility declarations on HEAD, as once an extension moves to the new routine name when compiling with v16~ the code would work the same way when recompiled on v15. No backpatch to v15 for this one, because ABI compatibility has to be maintained there. Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de
2022-10-18Rename SetSingleFuncCall() to InitMaterializedSRF()Michael Paquier
Per discussion, the existing routine name able to initialize a SRF function with materialize mode is unpopular, so rename it. Equally, the flags of this function are renamed, as of: - SRF_SINGLE_USE_EXPECTED -> MAT_SRF_USE_EXPECTED_DESC - SRF_SINGLE_BLESS -> MAT_SRF_BLESS The previous function and flags introduced in 9e98583 are kept around for compatibility purposes, so as any extension code already compiled with v15 continues to work as-is. The declarations introduced here for compatibility will be removed from HEAD in a follow-up commit. The new names have been suggested by Andres Freund and Melanie Plageman. Discussion: https://postgr.es/m/20221013194820.ciktb2sbbpw7cljm@awork3.anarazel.de Backpatch-through: 15
2022-10-17doc: move the mention of aggregate JSON functions up in sectionBruce Momjian
It was previously easily overlooked at the end of several tables. Reported-by: Alex Denman Discussion: https://postgr.es/m/166335888474.659.16897487975376230364@wrigleys.postgresql.org Backpatch-through: 10
2022-10-17doc: warn pg_stat_reset() can cause vacuum/analyze problemsBruce Momjian
The fix is to run ANALYZE. Discussion: https://postgr.es/m/YzRr+ys98UzVQJvK@momjian.us, https://postgr.es/m/flat/CAKJS1f8DTbCHf9gedU0He6ARsd58E6qOhEHM1caomqj_r9MOiQ%40mail.gmail.com, https://postgr.es/m/CAKJS1f80o98hcfSk8j%3DfdN09S7Sjz%2BvuzhEwbyQqvHJb_sZw0g%40mail.gmail.com Backpatch-through: 10
2022-10-17Record dependencies of a cast on other casts that it requires.Tom Lane
When creating a cast that uses a conversion function, we've historically allowed the input and result types to be binary-compatible with the function's input and result types, rather than necessarily being identical. This means that the new cast is logically dependent on the binary-compatible cast or casts that it references: if those are defined by pg_cast entries, and you try to restore the new cast without having defined them, it'll fail. Hence, we should make pg_depend entries to record these dependencies so that pg_dump knows that there is an ordering requirement. This is not the only place where we allow such shortcuts; aggregate functions for example are similarly lax, and in principle should gain similar dependencies. However, for now it seems sufficient to fix the cast-versus-cast case, as pg_dump's other ordering heuristics should keep it out of trouble for other object types. Per report from David Turoň; thanks also to Robert Haas for preliminary investigation. I considered back-patching, but seeing that this issue has existed for many years without previous reports, it's not clear it's worth the trouble. Moreover, back-patching wouldn't be enough to ensure that the new pg_depend entries exist in existing databases anyway. Discussion: https://postgr.es/m/OF0A160F3E.578B15D1-ONC12588DA.003E4857-C12588DA.0045A428@notes.linuxbox.cz
2022-10-17Reject non-ON-SELECT rules that are named "_RETURN".Tom Lane
DefineQueryRewrite() has long required that ON SELECT rules be named "_RETURN". But we overlooked the converse case: we should forbid non-ON-SELECT rules that are named "_RETURN". In particular this prevents using CREATE OR REPLACE RULE to overwrite a view's _RETURN rule with some other kind of rule, thereby breaking the view. Per bug #17646 from Kui Liu. Back-patch to all supported branches. Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org
2022-10-17Guard against table-AM-less relations in planner.Tom Lane
The executor will dump core if it's asked to execute a seqscan on a relation having no table AM, such as a view. While that shouldn't really happen, it's possible to get there via catalog corruption, such as a missing ON SELECT rule. It seems worth installing a defense against that. There are multiple plausible places for such a defense, but I picked the planner's get_relation_info(). Per discussion of bug #17646 from Kui Liu. Back-patch to v12 where the tableam APIs were introduced; in older versions you won't get a SIGSEGV, so it seems less pressing. Discussion: https://postgr.es/m/17646-70c93cfa40365776@postgresql.org