aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* pg_dump: Add missing relkind casePeter Eisentraut2021-12-02
| | | | | | | | | | | | Checking for RELKIND_MATVIEW was forgotten in guessConstraintInheritance(). This isn't a live problem, since it is checked in flagInhTables() which relkinds can have parents, and those entries will have numParents==0 after that. But after discussion it was felt that this place should be kept consistent with flagInhTables() and flagInhAttrs(). Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/a574c8f1-9c84-93ad-a9e5-65233d6fc00f@enterprisedb.com
* pg_waldump: Emit stats summary when interrupted by SIGINTMichael Paquier2021-12-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, pg_waldump would not display its statistics summary if it got interrupted by SIGINT (or say a simple Ctrl+C). It gains with this commit a signal handler for SIGINT, trapping the signal to exit at the earliest convenience to allow a display of the stats summary before exiting. This makes the reports more interactive, similarly to strace -c. This new behavior makes the combination of the options --stats and --follow much more useful, so as the user will get a report for any invocation of pg_waldump in such a case. Information about the LSN range of the stats computed is added as a header to the report displayed. This implementation comes from a suggestion by Álvaro Herrera and myself, following a complaint by the author of this patch about --stats and --follow not being useful together originally. As documented, this is not supported on Windows, though its support would be possible by catching the terminal events associated to Ctrl+C, for example (this may require a more centralized implementation, as other tools could benefit from a common API). Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACUUx3PcK2z9h0_m7vehreZAUbcmOky9WSEpe8TofhV=PQ@mail.gmail.com
* Move into separate file all the SQL queries used in pg_upgrade testsMichael Paquier2021-12-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | The existing pg_upgrade/test.sh and the buildfarm code have been holding the same set of SQL queries when doing cross-version upgrade tests to adapt the objects created by the regression tests before the upgrade (mostly, incompatible or non-existing objects need to be dropped from the origin, perhaps re-created). This moves all those SQL queries into a new, separate, file with a set of \if clauses to handle the version checks depending on the old version of the cluster to-be-upgraded. The long-term plan is to make the buildfarm code re-use this new SQL file, so as committers are able to fix any compatibility issues in the tests of pg_upgrade with a refresh of the core code, without having to poke at the buildfarm client. Note that this is only able to handle the main regression test suite, and that nothing is done yet for contrib modules yet (these have more issues like their database names). A backpatch down to 10 is done, adapting the version checks as this script needs to be only backward-compatible, so as it becomes possible to clean up a maximum amount of code within the buildfarm client. Author: Justin Pryzby, Michael Paquier Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com Backpatch-through: 10
* Avoid leaking memory during large-scale REASSIGN OWNED BY operations.Tom Lane2021-12-01
| | | | | | | | | | | | | | | | | | | | | | The various ALTER OWNER routines tend to leak memory in CurrentMemoryContext. That's not a problem when they're only called once per command; but in this usage where we might be touching many objects, it can amount to a serious memory leak. Fix that by running each call in a short-lived context. (DROP OWNED BY likely has a similar issue, except that you'll probably run out of lock table space before noticing. REASSIGN is worth fixing since for most non-table object types, it won't take any lock.) Back-patch to all supported branches. Unfortunately, in the back branches this helps to only a limited extent, since the sinval message queue bloats quite a lot in this usage before commit 3aafc030a, consuming memory more or less comparable to what's actually leaked. Still, it's clearly a leak with a simple fix, so we might as well fix it. Justin Pryzby, per report from Guillaume Lelarge Discussion: https://postgr.es/m/CAECtzeW2DAoioEGBRjR=CzHP6TdL=yosGku8qZxfX9hhtrBB0Q@mail.gmail.com
* psql: initialize comment-begin setting to a useful value by default.Tom Lane2021-12-01
| | | | | | | | | | | Readline's meta-# command is supposed to insert a comment marker at the start of the current line. However, the default marker is "#" which is entirely unhelpful for SQL. Set it to "-- " instead. (This setting can still be overridden in one's ~/.inputrc file, so this change won't affect people who have already taken steps to make the command useful.) Discussion: https://postgr.es/m/CAJcOf-cAdMVr7azeYR7nWKsNp7qhORzc84rV6d7m7knG5Hrtsw@mail.gmail.com
* psql: treat "--" comments between queries as separate history entries.Tom Lane2021-12-01
| | | | | | | | | | | | | | If we've not yet collected any non-whitespace, non-comment token for a new query, flush the current input line to history before reading another line. This aligns psql's history behavior with the observation that lines containing only comments are generally not thought of as being part of the next query. psql's prompting behavior is consistent with that view, too, since it won't change the prompt until you enter something that's neither whitespace nor a "--" comment. Greg Nancarrow, simplified a bit by me Discussion: https://postgr.es/m/CAJcOf-cAdMVr7azeYR7nWKsNp7qhORzc84rV6d7m7knG5Hrtsw@mail.gmail.com
* psql: include intra-query "--" comments in what's sent to the server.Tom Lane2021-12-01
| | | | | | | | | | | | | | | | | | | | | | | | psql's lexer has historically deleted dash-dash (single-line) comments from what's collected and sent to the server. This is inconsistent with what it does for slash-star comments, and people have complained before that they wish such comments would be captured in the server log. Undoing the decision completely seems like too big a behavioral change, however. In particular, comments on lines preceding the start of a query are generally not thought of as being part of that query. What we can do to improve the situation is to capture comments that are clearly *within* a query, that is after the first non-whitespace, non-comment token but before the query's ending semicolon or backslash command. This is a nearly trivial code change, and it affects only a few regression test results. (It is tempting to try to apply the same rule to slash-star comments. But it's hard to see how to do that without getting strange history behavior for comments that cross lines, especially if the user then starts a new query on the same line as the star-slash. In view of the lack of complaints, let's leave that case alone.) Discussion: https://postgr.es/m/CAJcOf-cAdMVr7azeYR7nWKsNp7qhORzc84rV6d7m7knG5Hrtsw@mail.gmail.com
* Remove unused includesPeter Eisentraut2021-12-01
| | | | | | | These haven't been needed for a long time. Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Improve some comments in scanner filesPeter Eisentraut2021-12-01
| | | | | Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Warning on SET of nonexisting setting with a prefix reserved by an extensionPeter Eisentraut2021-12-01
| | | | | | | | | | | | | | | | | | | An extension can already de facto reserve a GUC prefix using EmitWarningsOnPlaceholders(). But this was only checked against settings that exist at the time the extension is loaded (or the extension chooses to call this). No diagnostic is given when a SET command later uses a nonexisting setting with a custom prefix. With this change, EmitWarningsOnPlaceholders() saves the prefixes it reserves in a list, and SET checks when it finds a "placeholder" setting whether it belongs to a reserved prefix and issues a warning in that case. Add a regression test that checks the patch using the "plpgsql" registered prefix. Author: Florin Irion <florin.irion@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/CA+HEvJDhWuuTpGTJT9Tgbdzm4QS4EzPAwDBScWK18H2Q=FVJFw@mail.gmail.com
* Fix certificate paths to use perl2hostDaniel Gustafsson2021-12-01
| | | | | | | | | | | | | Commit c113d8ad50 moved the copying of certificates into a temporary path for the duration of the tests, instead of using the source tree. This broke the tests on msys as the absolute path wasn't adapted for the msys platform. Ensure to convert the path with perl2host before copying and passing in the connection string. While there also make certificate copying error handling uniform across all the test suites. Discussion: https://postgr.es/m/YacT3tm97xziSUFw@paquier.xyz
* Remove mention of TimeLineID update from commentsDaniel Gustafsson2021-12-01
| | | | | | | | Commit 4a92a1c3d removed the TimeLineID update from RecoveryInProgress, update comments accordingly. Author: Amul Sul <sulamul@gmail.com> Discussion: https://postgr.es/m/CAAJ_b96wyzs8N45jc-kYd-bTE02hRWQieLZRpsUtNbhap7_PuQ@mail.gmail.com
* Fix comment grammar in slotfuncs.cMichael Paquier2021-12-01
| | | | | Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACUkrNR2xTak+QaqxoTjPKGn8zXWripv7SR27t+Q5qF1Wg@mail.gmail.com
* Fix regression test failure caused by commit 8d74fc96db.Amit Kapila2021-12-01
| | | | | | | | | | | | | | | | | | | The tests didn't considered that an error unrelated to apply changes, e.g. "replication origin with OID %d is already active ...", could occur on the table sync worker before starting to copy changes. To make the test robust we instead need to check the expected error and the source of error which will be either tablesync or apply worker. In passing remove the harmless option "streaming = off" from Create Subscription command as that is anyway the default. Per buildfarm member sidewinder. Author: Masahiko Sawada Reviewed-by: Hou Zhijie, Vignesh C, Amit Kapila Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com Discussion: https://postgr.es/m/E1mrtvV-0002Gz-73@gemulon.postgresql.org
* Improve psql tab completion for various DROP commandsMichael Paquier2021-12-01
| | | | | | | | | | | | The following improvements are done: - Handling of RESTRICT/CASCADE for DROP OWNED, matviews and policies. - Handling of DROP TRANSFORM This is a continuation of the work done in 0cd6d3b and f44ceb4. Author: Ken Kato Reviewed-by: Asif Rehman Discussion: https://postgr.es/m/0fafb73f3a0c6bcec817a25ca9d5a853@oss.nttdata.com
* Extend the private key stat checking error handlingDaniel Gustafsson2021-11-30
| | | | | | | | | | | | If the stat operation on the private key failed, the code assumed it was due to an ENOENT, which may or may not be true. Extend the check by printing a different error message on non-ENOENT errors for easier debugging. Per suggestion by Tom Lane due to an issue with the fairywren animal in the buildfarm. Discussion: https://postgr.es/m/1632478.1638305700@sss.pgh.pa.us
* vacuumlazy.c: fix remaining "dead tuple" references.Peter Geoghegan2021-11-30
| | | | | | | Oversight in commit 4f8d9d12. Reported-By: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoDm38Em0bvRqeQKr4HPvOj65Y8cUgCP4idMk39iaLrxyw@mail.gmail.com
* Ignore BRIN indexes when checking for HOT udpatesTomas Vondra2021-11-30
| | | | | | | | | | | | | | | | When determining whether an index update may be skipped by using HOT, we can ignore attributes indexed only by BRIN indexes. There are no index pointers to individual tuples in BRIN, and the page range summary will be updated anyway as it relies on visibility info. This also removes rd_indexattr list, and replaces it with rd_attrsvalid flag. The list was not used anywhere, and a simple flag is sufficient. Patch by Josef Simanek, various fixes and improvements by me. Author: Josef Simanek Reviewed-by: Tomas Vondra, Alvaro Herrera Discussion: https://postgr.es/m/CAFp7QwpMRGcDAQumN7onN9HjrJ3u4X3ZRXdGFT0K5G2JWvnbWg%40mail.gmail.com
* Increase size of shared memory for pg_commit_tsAlvaro Herrera2021-11-30
| | | | | | | | | | Like 5364b357fb11 did for pg_commit, change the formula used to determine number of pg_commit_ts buffers, which helps performance with larger servers. Discussion: https://postgr.es/m/20210115220744.GA24457@alvherre.pgsql Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
* Disable unused-variable warning C4101 in MSVCDaniel Gustafsson2021-11-30
| | | | | | | | | | The C4101 warning for unused variable cannot be individually suppressed with PG_USED_FOR_ASSERTS_ONLY, and thus cause false-positive warnings for variables which are defined but only read/written in an assertion. Until a satisfactory solution for per-variable suppression like how we do for gcc and clang, disable the warning. Discussion: https://postgr.es/m/CAJcOf-c+KniGAp31pn8TC=9a-WHXpkX-3+8-2BkaCsZchhu=8w@mail.gmail.com
* Remove PF_USED_FOR_ASSERTS_ONLY from variables in general useDaniel Gustafsson2021-11-30
| | | | | | | | | | | | | | | | fsstate in process_pending_requests (in postgres_fdw.c) was added in 8998e3cafa2 as an assertion-only variable, 1ec7fca8592 stated using the variable outside of assertions. rd_index in get_index_column_opclass (in lsyscache.c) was introduced in 2a6368343ff, and then promptly used in the fix commit 7e041603904 shortly thereafter. This removes the PG_USED_FOR_ASSERTS_ONLY variable decoration from the above mentioned variables. Reviewed-by: Greg Nancarrow <gregn4422@gmail.com> Discussion: https://postgr.es/m/F959106C-0F21-43A5-B2AE-D007D51ACBEE@yesql.se
* Use test-specific temp path for keys during SSL testDaniel Gustafsson2021-11-30
| | | | | | | | | | | | The SSL and SCRAM TAP test suites both use temporary copies of the supplied test keys in order to ensure correct permissions. These were however copied inside the tree using temporary filenames rather than a true temporary folder. Fix by using tmp_check supplied by PostgreSQL::Test::Utils. Spotted by Tom Lane during review of the nearby sslinfo TAP test patch. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/599244.1638041239@sss.pgh.pa.us
* Add TAP tests for contrib/sslinfoDaniel Gustafsson2021-11-30
| | | | | | | | | | | | | This adds rudimentary coverage of the sslinfo extension into the SSL test harness. The output is validated by comparing with pg_stat_ssl to provide some level of test stability should the underlying certs be slightly altered. A new cert is added to provide an extension to test against. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://postgr.es/m/E23F9811-0C77-45DA-912F-D809AB140741@yesql.se
* Extend configure_test_server_for_ssl to add extensionsDaniel Gustafsson2021-11-30
| | | | | | | | | | | In order to be able to test extensions with SSL connections, allow configure_test_server_for_ssl to create any extensions passed as an array. Each extension is created in all the test databases. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://postgr.es/m/E23F9811-0C77-45DA-912F-D809AB140741@yesql.se
* Fix flags of some GUCs and improve some descriptionsMichael Paquier2021-11-30
| | | | | | | | | | | | | | | | | | | This commit fixes some issues with GUCs: - enable_incremental_sort was not marked as GUC_EXPLAIN, causing it to not be listed in the output of EXPLAIN (SETTINGS) if using a value different than the default, contrary to the other planner-level GUCs. - trace_recovery_messages missed GUC_NOT_IN_SAMPLE, like the other developer options. - ssl_renegotiation_limit should be marked as COMPAT_OPTIONS_PREVIOUS. While on it, this fixes one incorrect comment related to autovacuum_freeze_max_age, and improves the descriptions of some other GUCs, recently introduced. Extracted from a larger patch set by the same author. Author: Justin Pryzby Description: https://postgr.es/m/20211129030833.GJ17618@telsasoft.com
* Add a view to show the stats of subscription workers.Amit Kapila2021-11-30
| | | | | | | | | | | | | | | | | | | | | | | This commit adds a new system view pg_stat_subscription_workers, that shows information about any errors which occur during the application of logical replication changes as well as during performing initial table synchronization. The subscription statistics entries are removed when the corresponding subscription is removed. It also adds an SQL function pg_stat_reset_subscription_worker() to reset single subscription errors. The contents of this view can be used by an upcoming patch that skips the particular transaction that conflicts with the existing data on the subscriber. This view can be extended in the future to track other xact related statistics like the number of xacts committed/aborted for subscription workers. Author: Masahiko Sawada Reviewed-by: Greg Nancarrow, Hou Zhijie, Tang Haiying, Vignesh C, Dilip Kumar, Takamichi Osumi, Amit Kapila Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
* Fix typosMichael Paquier2021-11-30
| | | | | Author: Lingjie Qiang Discussion: https://postgr.es/m/OSAPR01MB71654E773F62AC88DC1FC8CC80669@OSAPR01MB7165.jpnprd01.prod.outlook.com
* Fix compatibility thinko for fstat() on standard streams in win32stat.cMichael Paquier2021-11-30
| | | | | | | | | | | | | | | | | | | | | | GetFinalPathNameByHandleA() cannot be used in compilation environments where _WIN32_WINNT < 0x0600, meaning at least Windows XP used by some buildfarm members under MinGW that Postgres still needs to support. This was reported as a compilation warning by the buildfarm, but this is actually worse than the report as the code would have not worked. Instead, this switches to GetFileInformationByHandle() that is able to fail for standard streams and succeed for redirected ones, which is what we are looking for herein the code emulating fstat(). We also know that it is able to work in all the environments still supported, thanks to the existing logic of win32stat.c. Issue introduced by 10260c7, so backpatch down to 14. Reported-by: Justin Pryzby, via buildfarm member jacana Author: Michael Paquier Reviewed-by: Juan José Santamaría Flecha Discussion: https://postgr.es/m/20211129050122.GK17618@telsasoft.com Backpatch-through: 14
* vacuumlazy.c: Rename dead_tuples to dead_items.Peter Geoghegan2021-11-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 8523492d simplified what it meant for an item to be considered "dead" to VACUUM: TIDs collected in memory (in preparation for index vacuuming) must always come from LP_DEAD stub line pointers in heap pages, found following pruning. This formalized the idea that index vacuuming (and heap vacuuming) are optional processes. Unlike pruning, they can be delayed indefinitely, without any risk of that violating fundamental invariants. For example, leaving LP_DEAD items behind clearly won't add to the risk of transaction ID wraparound. You can't have transaction ID wraparound without transaction IDs. Renaming anything that references DEAD tuples (tuples with storage) reinforces all this. Code outside vacuumlazy.c continues to fudge the distinction between dead/deleted tuples, and LP_DEAD items. This is necessary because autovacuum scheduling is still mostly driven by "dead items/tuples" statistics. In the future we may find it useful to replace this model with something more sophisticated, as a step towards teaching autovacuum to perform more frequent vacuuming that targeting individual indexes that happen to be more prone to becoming bloated through version churn. In passing, simplify some function signatures that deal with VACUUM's dead_items array. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAH2-WzktGBg4si6DEdmq3q6SoXSDqNi6MtmB8CmmTmvhsxDTLA@mail.gmail.com
* Simplify declaring variables exported from libpgcommon and libpgport.Tom Lane2021-11-29
| | | | | | | | | | | | | | | | This reverts commits c2d1eea9e and 11b500072, as well as similar hacks elsewhere, in favor of setting up the PGDLLIMPORT macro so that it can just be used unconditionally. That can work because in frontend code, we need no marking in either the defining or consuming files for a variable exported from these libraries; and frontend code has no need to access variables exported from the core backend, either. While at it, write some actual documentation about the PGDLLIMPORT and PGDLLEXPORT macros. Patch by me, based on a suggestion from Robert Haas. Discussion: https://postgr.es/m/1160385.1638165449@sss.pgh.pa.us
* Portability hack for pg_global_prng_state.Tom Lane2021-11-29
| | | | | | | | | PGDLLIMPORT is only appropriate for variables declared in the backend, not when the variable is coming from a library included in frontend code. (This isn't a particularly nice fix, but for now, use the same method employed elsewhere.) Discussion: https://postgr.es/m/E1mrWUD-000235-Hq@gemulon.postgresql.org
* Centralize timestamp computation of control file on updatesMichael Paquier2021-11-29
| | | | | | | | | | | | | | | | | | | This commit moves the timestamp computation of the control file within the routine of src/common/ in charge of updating the backend's control file, which is shared by multiple frontend tools (pg_rewind, pg_checksums and pg_resetwal) and the backend itself. This change has as direct effect to update the control file's timestamp when writing the control file in pg_rewind and pg_checksums, something that is helpful to keep track of control file updates for those operations, something also tracked by the backend at startup within its logs. This part is arguably a bug, as ControlFileData->time should be updated each time a new version of the control file is written, but this is a behavior change so no backpatch is done. Author: Amul Sul Reviewed-by: Nathan Bossart, Michael Paquier, Bharath Rupireddy Discussion: https://postgr.es/m/CAAJ_b97nd_ghRpyFV9Djf9RLXkoTbOUqnocq11WGq9TisX09Fw@mail.gmail.com
* Replace random(), pg_erand48(), etc with a better PRNG API and algorithm.Tom Lane2021-11-28
| | | | | | | | | | | | | | | | | | | Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating a bunch of platform dependencies as well as fundamentally-obsolete PRNG code. In addition, this API replacement will ease replacing the algorithm again in future, should that become necessary. xoroshiro128** is a few percent slower than the drand48 family, but it can produce full-width 64-bit random values not only 48-bit, and it should be much more trustworthy. It's likely to be noticeably faster than the platform's random(), depending on which platform you are thinking about; and we can have non-global state vectors easily, unlike with random(). It is not cryptographically strong, but neither are the functions it replaces. Fabien Coelho, reviewed by Dean Rasheed, Aleksander Alekseev, and myself Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo
* Improve psql tab completion for views, FDWs, sequences and transformsMichael Paquier2021-11-29
| | | | | | | | | | | | | The following improvements are done: - Addition of type completion for ALTER SEQUENCE AS. - Ignore ALTER for transforms, as the command is not supported. - Addition of more completion for ALTER FOREIGN DATA WRAPPER. - Addition of options related to columns in ALTER VIEW. This is a continuation of the work done in 0cd6d3b. Author: Ken Kato Discussion: https://postgr.es/m/9497ae9ca1b31eb9b1e97aded1c2ab07@oss.nttdata.com
* vacuumlazy.c: prefer the term "cleanup lock".Peter Geoghegan2021-11-27
| | | | | | | | The term "super-exclusive lock" is an acceptable synonym of "cleanup lock". Even still, switching from one term to the other in the same file is confusing. Standardize on "cleanup lock" within vacuumlazy.c. Per a complaint from Andres Freund.
* Update high level vacuumlazy.c comments.Peter Geoghegan2021-11-27
| | | | | | | | | | | | | | | | | Update vacuumlazy.c file header comments (as well as comments above the lazy_scan_heap function) that were largely written before the introduction of the HOT optimization, when lazy_scan_heap did far less, and didn't actually prune during its initial heap pass. Since lazy_scan_heap now outsources far more work to lower level functions, it makes sense to introduce the function by talking about the high level invariant that dictates the order in which each phase takes place. Also deemphasize the case where we run out of memory for TIDs, since delaying that discussion makes it easier to talk about issues of central importance. Finally, remove discussion of parallel VACUUM from header comments. These don't add much, and are in the wrong place.
* Harden be-gssapi-common.h for headerscheckAlvaro Herrera2021-11-26
| | | | | | | | | | Surround the contents with a test that the feature is enabled by configure, to silence header checking tools on systems without GSSAPI installed. Backpatch to 12, where the file appeared. Discussion: https://postgr.es/m/202111161709.u3pbx5lxdimt@alvherre.pgsql
* Go back to considering HOT on pages marked full.Peter Geoghegan2021-11-26
| | | | | | | | | | | | | | | | | | | | Commit 2fd8685e7f simplified the checking of modified attributes that takes place within heap_update(). This included a micro-optimization affecting pages marked PD_PAGE_FULL: don't even try to use HOT to save a few cycles on determining HOT safety. The assumption was that it won't work out this time around, since it can't have worked out last time around. Remove the micro-optimization. It could only ever save cycles that are consumed by the vast majority of heap_update() calls, which hardly seems worth the added complexity. It also seems quite possible that there are workloads that will do worse over time by repeated application of the micro-optimization, despite saving some cycles on average, in the short term. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CAH2-WznU1L3+DMPr1F7o2eJBT7=3bAJoY6ZkWABAxNt+-afyTA@mail.gmail.com
* Fix determination of broken LSN in OVERWRITTEN_CONTRECORDAlvaro Herrera2021-11-26
| | | | | | | | | | | | | | In commit ff9f111bce24 I mixed up inconsistent definitions of the LSN of the first record in a page, when the previous record ends exactly at the page boundary. The correct LSN is adjusted to skip the WAL page header; I failed to use that when setting XLogReaderState->overwrittenRecPtr, so at WAL replay time VerifyOverwriteContrecord would refuse to let replay continue past that record. Backpatch to 10. 9.6 also contains this bug, but it's no longer being maintained. Discussion: https://postgr.es/m/45597.1637694259@sss.pgh.pa.us
* Add test for REVOKE ADMIN OPTIONDaniel Gustafsson2021-11-26
| | | | | | | | The REVOKE ADMIN OPTION FOR <role_name> syntax didn't have ample test coverage. Fix by adding coverage in the privileges test suite. Author: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://postgr.es/m/333B0203-D19B-4335-AE64-90EB0FAF46F0@enterprisedb.com
* Fix GRANTED BY support in REVOKE ROLE statementsDaniel Gustafsson2021-11-26
| | | | | | | | | | | Commit 6aaaa76bb added support for the GRANTED BY clause in GRANT and REVOKE statements, but missed adding support for checking the role in the REVOKE ROLE case. Fix by checking that the parsed role matches the CURRENT_ROLE/CURRENT_USER requirement, and also add some tests for it. Backpatch to v14 where GRANTED BY support was introduced. Discussion: https://postgr.es/m/B7F6699A-A984-4943-B9BF-CEB84C003527@yesql.se Backpatch-through: 14
* Update commentsPeter Eisentraut2021-11-26
| | | | | | | | Various places wanted to point out that tuple descriptors don't contain the variable-length fields of pg_attribute. This started when attacl was added, but more fields have been added since, and these comments haven't been kept up to date consistently. Reword so that the purpose is clearer and we don't have to keep updating them.
* Remove unneeded Python includesPeter Eisentraut2021-11-25
| | | | | | | | | Inluding <compile.h> and <eval.h> has not been necessary since Python 2.4, since they are included via <Python.h>. Morever, <eval.h> is being removed in Python 3.11. So remove these includes. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/84884.1637723223%40sss.pgh.pa.us
* Block ALTER TABLE .. DROP NOT NULL on columns in replica identity indexMichael Paquier2021-11-25
| | | | | | | | | | | | | | | | | Replica identities that depend directly on an index rely on a set of properties, one of them being that all the columns defined in this index have to be marked as NOT NULL. There was a hole in the logic with ALTER TABLE DROP NOT NULL, where it was possible to remove the NOT NULL property of a column part of an index used as replica identity, so block it to avoid problems with logical decoding down the road. The same check was already done columns part of a primary key, so the fix is straight-forward. Author: Haiying Tang, Hou Zhijie Reviewed-by: Dilip Kumar, Michael Paquier Discussion: https://postgr.es/m/OS0PR01MB6113338C102BEE8B2FFC5BD9FB619@OS0PR01MB6113.jpnprd01.prod.outlook.com Backpatch-through: 10
* Fix fstat() emulation on Windows with standard streamsMichael Paquier2021-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | The emulation of fstat() in win32stat.c caused two issues with the existing in-core callers, failing on EINVAL when using a stream as argument: - psql's \copy would crash when using a stream. - pg_recvlogical would fail with -f -. The tests in copyselect.sql from the main test suite covers the first case, and there is a TAP test for the second case. However, in both cases, as the standard streams are always redirected, automated tests did not notice those issues, requiring a terminal on Windows to be reproducible. This issue has been introduced in bed9075, and the origin of the problem is that GetFileInformationByHandle() does not work directly on streams, so this commit adds an extra code path to emulate and return a set of stats that match best with the reality. Note that redirected streams rely on handles that can be queried with GetFileInformationByHandle(), but we can rely on GetFinalPathNameByHandleA() to detect this case. Author: Dmitry Koval, Juan José Santamaría Flecha Discussion: https://postgr.es/m/17288-6b58a91025a8a8a3@postgresql.org Backpatch-through: 14
* Replace straggling uses of ReadRecPtr/EndRecPtr.Andres Freund2021-11-24
| | | | | | | d2ddfa681db removed ReadRecPtr/EndRecPtr, but two uses within an #ifdef WAL_DEBUG escaped. Discussion: https://postgr.es/m/20211124231206.gbadj5bblcljb6d5@alap3.anarazel.de
* xlog.c: Remove global variables ReadRecPtr and EndRecPtr.Robert Haas2021-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In most places, the variables necessarily store the same value as the eponymous members of the XLogReaderState that we use during WAL replay, because ReadRecord() assigns the values from the structure members to the global variables just after XLogReadRecord() returns. However, XLogBeginRead() adjusts the structure members but not the global variables, so after XLogBeginRead() and before the completion of XLogReadRecord() the values can differ. Otherwise, they must be identical. According to my analysis, the only place where either variable is referenced at a point where it might not have the same value as the structure member is the refrence to EndRecPtr within XLogPageRead. Therefore, at every other place where we are using the global variable, we can just switch to using the structure member instead, and remove the global variable. However, we can, and in fact should, do this in XLogPageRead() as well, because at that point in the code, the global variable will actually store the start of the record we want to read - either because it's where the last WAL record ended, or because the read position has been changed using XLogBeginRead since the last record was read. The structure member, on the other hand, will already have been updated to point to the end of the record we just read. Elsewhere, the latter is what we use as an argument to emode_for_corrupt_record(), so we should do the same here. This part of the patch is perhaps a bug fix, but I don't think it has any important consequences, so no back-patch. The point here is just to continue to whittle down the entirely excessive use of global variables in xlog.c. Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
* Fix corner-case failure to detect improper timeline switch.Robert Haas2021-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | rescanLatestTimeLine() contains a guard against switching to a timeline that forked off from the current one prior to the current recovery point, but that guard does not work if the timeline switch occurs before the first WAL recod (which must be the checkpoint record) is read. Without this patch, an improper timeline switch is therefore possible in such cases. This happens because rescanLatestTimeLine() relies on the global variable EndRecPtr to understand the current position of WAL replay. However, EndRecPtr at this point in the code contains the endpoint of the last-replayed record, not the startpoint or endpoint of the record being replayed now. Thus, before any records have been replayed, it's zero, which causes the sanity check to always pass. To fix, pass down the correct timeline explicitly. The EndRecPtr value we want is the one from the xlogreader, which will be the starting position of the record we're about to try to read, rather than the global variable, which is the ending position of the last record we successfully read. They're usually the same, but not in the corner case described here. No back-patch, because in v14 and earlier branhes, we were using the wrong TLI here as well as the wrong LSN. In master, that was fixed by commit 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd, but that and it's prerequisite patches are too invasive to back-patch for such a minor issue. Patch by me, reviewed by Amul Sul. Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
* Remove useless LZ4 system call on failure when writing file headerMichael Paquier2021-11-24
| | | | | | | | | | | | If an error occurs when writing the LZ4 file header, LZ4F_compressEnd() was called in the error code path of write(), followed by LZ4F_freeCompressionContext() to finish the cleanup. The code as-is was not broken, but the LZ4F_compressEnd() proves to not be necessary as there are no contents to flush at this stage, so remove it. Per gripe from Jeevan Ladhe and Robert Haas. Discussion: https://postgr.es/m/CAOgcT0PE33wbD7giAT1OSkNJt=p-vu8huq++qh=ny9O=SCP5aA@mail.gmail.com
* Flush Memoize cache when non-key parameters change, take 2David Rowley2021-11-24
| | | | | | | | | | | | | | | | It's possible that a subplan below a Memoize node contains a parameter from above the Memoize node. If this parameter changes then cache entries may become out-dated due to the new parameter value. Previously Memoize was mistakenly not aware of this. We fix this here by flushing the cache whenever a parameter that's not part of the cache key changes. Bug: #17213 Reported by: Elvis Pranskevichus Author: David Rowley Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org Backpatch-through: 14, where Memoize was added