aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Introduce "builtin" collation provider.Jeff Davis2024-03-13
| | | | | | | | | | | | | | | | | | | | | | | | | New provider for collations, like "libc" or "icu", but without any external dependency. Initially, the only locale supported by the builtin provider is "C", which is identical to the libc provider's "C" locale. The libc provider's "C" locale has always been treated as a special case that uses an internal implementation, without using libc at all -- so the new builtin provider uses the same implementation. The builtin provider's locale is independent of the server environment variables LC_COLLATE and LC_CTYPE. Using the builtin provider, the database collation locale can be "C" while LC_COLLATE and LC_CTYPE are set to "en_US", which is impossible with the libc provider. By offering a new builtin provider, it clarifies that the semantics of a collation using this provider will never depend on libc, and makes it easier to document the behavior. Discussion: https://postgr.es/m/ab925f69-5f9d-f85e-b87c-bd2a44798659@joeconway.com Discussion: https://postgr.es/m/dd9261f4-7a98-4565-93ec-336c1c110d90@manitou-mail.org Discussion: https://postgr.es/m/ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com Reviewed-by: Daniel Vérité, Peter Eisentraut, Jeremy Schneider
* Put genbki.pl output into src/include/catalog/ directlyPeter Eisentraut2024-03-14
| | | | | | | | | | | | | | | | With the makefile rules, the output of genbki.pl was written to src/backend/catalog/, and then the header files were linked to src/include/catalog/. This changes it so that the output files are written directly to src/include/catalog/. This makes the logic simpler, and it also makes the behavior consistent with the meson build system. Also, the list of catalog files is now kept in parallel in src/include/catalog/{meson.build,Makefile}, while before the makefiles had it in src/backend/catalog/Makefile. Reviewed-by: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/21b74bdc-183d-4dd5-9c27-9378d178f459@eisentraut.org
* Revert "Add basic TAP tests for the low-level backup method"Michael Paquier2024-03-14
| | | | | | | | | | | | | | | This reverts commit 99b4a63bef94. The test is proving to be unstable, so revert it for now. One of the failures seen involves the cluster started without the backup_label, where the archives of the primary are overwritten, causing recovery failures on Windows. This is simple to fix, but there is another issue that's creeping behind in the form of an "invalid data in file" ERROR when parsing the backup_label for the second recovery case, as an effect of the backup_label data written after retrieving its contents from pg_backup_stop(). _ Per buildfarm member sidewinder.
* Add basic TAP tests for the low-level backup methodMichael Paquier2024-03-14
| | | | | | | | | | | | | | | | | There are currently no tests for the low-level backup method where pg_backup_start() and pg_backup_stop() are involved while taking a file-system backup. The tests introduced in this commit rely on a background psql process to make sure that the backup is taken while the session doing the SQL start and stop calls remains alive. Two cases are checked here with the backup taken: - Recovery without a backup_label, leading to a corrupted state. - Recovery with a backup_label, with a consistent state reached. Both cases cross-check some patterns in the logs generated when running recovery. Author: David Steele Discussion: https://postgr.es/m/f20fcc82-dadb-478d-beb4-1e2ffb0ace76@pgmasters.net
* Improve documentation for pg_stat_checkpointer fieldsAlexander Korotkov2024-03-14
| | | | | | | | | | | | pg_stat_checkpointer contains statistics for checkpoints and restartpoints. Before 12915a58eec9 documentation said only about checkpoints implying that restartpoint is the variation of checkpoint. 12915a58eec9 introduced new separate statistics fields for restartpoints. This commit explicitly documents fields that are relevant for both checkpoints and restartpoints. Reported-by: Magnus Hagander Discussion: https://postgr.es/m/CABUevExav5-SR0x%2BG9kBUMV0G8XsvSUfuyyqmYBBJi6VHns6sw%40mail.gmail.com Reviewed-by: Anton A. Melnikov
* Reintroduce MAINTAIN privilege and pg_maintain predefined role.Nathan Bossart2024-03-13
| | | | | | | | | | | | | | | | | | Roles with MAINTAIN on a relation may run VACUUM, ANALYZE, REINDEX, REFRESH MATERIALIZE VIEW, CLUSTER, and LOCK TABLE on the relation. Roles with privileges of pg_maintain may run those same commands on all relations. This was previously committed for v16, but it was reverted in commit 151c22deee due to concerns about search_path tricks that could be used to escalate privileges to the table owner. Commits 2af07e2f74, 59825d1639, and c7ea3f4229 resolved these concerns by restricting search_path when running maintenance commands. Bumps catversion. Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/20240305161235.GA3478007%40nathanxps13
* Add the system identifier to backup manifests.Robert Haas2024-03-13
| | | | | | | | | | | | | | | | | | Before this patch, if you took a full backup on server A and then tried to use the backup manifest to take an incremental backup on server B, it wouldn't know that the manifest was from a different server and so the incremental backup operation could potentially complete without error. When you later tried to run pg_combinebackup, you'd find out that your incremental backup was and always had been invalid. That's poor timing, because nobody likes finding out about backup problems only at restore time. With this patch, you'll get an error when trying to take the (invalid) incremental backup, which seems a lot nicer. Amul Sul, revised by me. Review by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoYLZzbSAMM3cAjV4Y+iCRZn-bR9H2+Mdz7NdaJFU1Zb5w@mail.gmail.com
* Hopefully make libpq_pipeline's new cancel test more reliableAlvaro Herrera2024-03-13
| | | | | | | | | | | The newly introduced cancel test in libpq_pipeline was flaky. It's not completely clear why, but one option is that the check for "active" was actually seeing the active state for the previous query. This change should address any such race condition by first waiting until the connection is reported as idle. Author: Jelte Fennema-Nio <me@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQRvmUK5-d68A+cm+fgmfht9Dv2uZ28-qq3QiaF6EAZqPQ@mail.gmail.com
* Expose new function get_controlfile_by_exact_path().Robert Haas2024-03-13
| | | | | | | | | This works just like get_controlfile(), but expects the path to the control file rather than the path to the data directory that contains the control file. This makes more sense in cases where the caller has already constructed the path to the control file itself. Amul Sul and Robert Haas, reviewed by Michael Paquier
* Make the order of the header file includes consistentPeter Eisentraut2024-03-13
| | | | | | | | Similar to commit 7e735035f20. Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-WhpCFMbXCjtJ%2BFzmjfPrp7Hw1pk4p%2BZpU95Kh3ofZ1A%40mail.gmail.com
* Fix incorrect format placeholdersPeter Eisentraut2024-03-13
|
* Add tests for more row patterns with COPY FROM .. (ON_ERROR ignore)Michael Paquier2024-03-13
| | | | | | | | | | | | While digging into the code of this feature, I got confused by the fact that a line is skipped when a value cannot be converted to its expected attribute even if the line has fewer attributes than the target relation. The tests had a check for the case of an empty line, this commit a couple more patterns where a line is incomplete, but skipped because of a conversion error. Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/Ze_7kZqexdt0BiyC@paquier.xyz
* Fix a random failure in 038_save_logical_slots_shutdown.pl.Amit Kapila2024-03-13
| | | | | | | | | | | | | | The test ensures that all the WAL on the publisher is sent to the subscriber before shutdown by comparing the confirmed_flush_lsn of the associated slot with the shutdown_checkpoint WAL location. But if the shutdown_checkpoint location falls into a new page in the WAL then the check won't work. So, ensure that the shutdown_checkpoint WAL record doesn't fall into a new page. Reported-by: Bharath Rupireddy Author: Bharath Rupireddy Reviewed-by: Vignesh C, Kuroda Hayato, Amit Kapila Discussion: https://postgr.es/m/CALj2ACVLzH5CN-h9=S26mdRHPuJ9yDLUw70yh4JOiPw03WL0CQ@mail.gmail.com
* ci: Use a RAM disk and more CPUs on FreeBSD.Thomas Munro2024-03-13
| | | | | | | | | | | | | | Run the tests in a RAM disk. It's still a UFS file system and is backed by 20GB of disk, but this avoids a lot of I/O. Even though we disable fsync, our tests do a lot of directory manipulations, some of which force file system meta-data to disk and flush slow device write caches on UFS. This was a bottleneck preventing effective scaling beyond 2 CPUs. Now we can use 4 CPUs like on other OSes, for a huge speedup. Reviewed-by: Maxim Orlov <orlovmg@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BFXLcEg1dyTqJjDiNQ8pGom4KrJj4wF38C90thti9dVA%40mail.gmail.com
* Add some asserts based on LWLockHeldByMe() for replication slot statisticsMichael Paquier2024-03-13
| | | | | | | | | | | | Two assertions checking that ReplicationSlotAllocationLock is acquired are added to pgstat_create_replslot() and pgstat_drop_replslot(), corresponding to the routines in charge of the creation and the drop of replication slot statistics. The code previously relied on this assumption and documented it in comments, but did not enforce this policy at runtime. Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Ze_p-hmD_yFeVYXg@paquier.xyz
* Fix version check in 002_pg_upgrade.pl.Jeff Davis2024-03-12
| | | | | | | | | Commit f696c0cd5f tried to account for the version in a way that includes development versions, but it was broken. Fix with suggestion from Tom Lane. Discussion: https://postgr.es/m/1553991.1710191312@sss.pgh.pa.us Reported-by: Tom Lane
* Fix confusion about the return rowtype of SQL-language procedures.Tom Lane2024-03-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | There is a very ancient hack in check_sql_fn_retval that allows a single SELECT targetlist entry of composite type to be taken as supplying all the output columns of a function returning composite. (This is grotty and fundamentally ambiguous, but it's really hard to do nested composite-returning functions without it.) As far as I know, that doesn't cause any problems in ordinary functions. It's disastrous for procedures however. All procedures that have any output parameters are labeled with prorettype RECORD, and the CALL code expects it will get back a record with one column per output parameter, regardless of whether any of those parameters is composite. Doing something else leads to an assertion failure or core dump. This is simple enough to fix: we just need to not apply that rule when considering procedures. However, that requires adding another argument to check_sql_fn_retval, which at least in principle might be getting called by external callers. Therefore, in the back branches convert check_sql_fn_retval into an ABI-preserving wrapper around a new function check_sql_fn_retval_ext. Per report from Yahor Yuzefovich. This has been broken since we implemented procedures, so back-patch to all supported branches. Discussion: https://postgr.es/m/CABz5gWHSjj2df6uG0NRiDhZ_Uz=Y8t0FJP-_SVSsRsnrQT76Gg@mail.gmail.com
* Fix incorrect filename reference in commentDavid Rowley2024-03-13
| | | | | Author: Cary Huang Discussion: https://postgr.es/m/18e34071af0.dbfc9663424635.8571906799773344646@highgo.ca
* libpq: Add encrypted and non-blocking query cancellation routinesAlvaro Herrera2024-03-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The existing PQcancel API uses blocking IO, which makes PQcancel impossible to use in an event loop based codebase without blocking the event loop until the call returns. It also doesn't encrypt the connection over which the cancel request is sent, even when the original connection required encryption. This commit adds a PQcancelConn struct and assorted functions, which provide a better mechanism of sending cancel requests; in particular all the encryption used in the original connection are also used in the cancel connection. The main entry points are: - PQcancelCreate creates the PQcancelConn based on the original connection (but does not establish an actual connection). - PQcancelStart can be used to initiate non-blocking cancel requests, using encryption if the original connection did so, which must be pumped using - PQcancelPoll. - PQcancelReset puts a PQcancelConn back in state so that it can be reused to send a new cancel request to the same connection. - PQcancelBlocking is a simpler-to-use blocking API that still uses encryption. Additional functions are - PQcancelStatus, mimicks PQstatus; - PQcancelSocket, mimicks PQcancelSocket; - PQcancelErrorMessage, mimicks PQerrorMessage; - PQcancelFinish, mimicks PQfinish. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Denis Laxalde <denis.laxalde@dalibo.com> Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com
* Fix copying SockAddr structHeikki Linnakangas2024-03-12
| | | | | | | | | | | | | | | | | | | | | | | Valgrind alerted about accessing uninitialized bytes after commit 4945e4ed4a: ==700242== VALGRINDERROR-BEGIN ==700242== Conditional jump or move depends on uninitialised value(s) ==700242== at 0x6D8A2A: getnameinfo_unix (ip.c:253) ==700242== by 0x6D8BD1: pg_getnameinfo_all (ip.c:122) ==700242== by 0x4B3EB6: BackendInitialize (postmaster.c:4266) ==700242== by 0x4B684E: BackendStartup (postmaster.c:4114) ==700242== by 0x4B6986: ServerLoop (postmaster.c:1780) ==700242== by 0x4B80CA: PostmasterMain (postmaster.c:1478) ==700242== by 0x3F7424: main (main.c:197) ==700242== Uninitialised value was created by a stack allocation ==700242== at 0x4B6934: ServerLoop (postmaster.c:1737) ==700242== ==700242== VALGRINDERROR-END That was because the SockAddr struct was not copied correctly. Per buildfarm animal "skink".
* Move initialization of the Port struct to the child processHeikki Linnakangas2024-03-12
| | | | | | | | | | | | | | | | In postmaster, use a more lightweight ClientSocket struct that encapsulates just the socket itself and the remote endpoint's address that you get from accept() call. ClientSocket is passed to the child process, which initializes the bigger Port struct. This makes it more clear what information postmaster initializes, and what is left to the child process. Rename the StreamServerPort and StreamConnection functions to make it more clear what they do. Remove StreamClose, replacing it with plain closesocket() calls. Reviewed-by: Tristan Partin, Andres Freund Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
* Pass CAC as an argument to the backend processHeikki Linnakangas2024-03-12
| | | | | | | | | | We used to smuggle it to the child process in the Port struct, but it seems better to pass it down as a separate argument. This paves the way for the next commit, which moves the initialization of the Port struct to the backend process, after forking. Reviewed-by: Tristan Partin, Andres Freund Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
* Set socket options in child process after forkingHeikki Linnakangas2024-03-12
| | | | | | | | | | | | Try to minimize the work done in the postmaster process for each accepted connection, so that postmaster can quickly proceed with its duties. These function calls are very fast so this doesn't make any measurable performance difference in practice, but it's nice to have all the socket options initialization code in one place for sake of readability too. This also paves the way for an upcoming commit that will move the initialization of the Port struct to the child process. Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
* Disconnect if socket cannot be put into non-blocking modeHeikki Linnakangas2024-03-12
| | | | | | | | | | | | | | | | | | | | Commit 387da18874 moved the code to put socket into non-blocking mode from socket_set_nonblocking() into the one-time initialization function, pq_init(). In socket_set_nonblocking(), there indeed was a risk of recursion on failure like the comment said, but in pq_init(), ERROR or FATAL is fine. There's even another elog(FATAL) just after this, if setting FD_CLOEXEC fails. Note that COMMERROR merely logged the error, it did not close the connection, so if putting the socket to non-blocking mode failed we would use the connection anyway. You might not immediately notice, because most socket operations in a regular backend wait for the socket to become readable/writable anyway. But e.g. replication will be quite broken. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/d40a5cd0-2722-40c5-8755-12e9e811fa3c@iki.fi
* libpq: Move pg_cancel to fe-cancel.cAlvaro Herrera2024-03-12
| | | | | | | | No other files need to access this struct, so there is no need to have its definition in a header file. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/202403061822.spfzqbf7dsgg@alvherre.pgsql
* Keep replication slot statistics on invalidationMichael Paquier2024-03-12
| | | | | | | | | | | | | | | | | | | | | | | | | The code path in charge of invalidating a replication slot includes a call to pgstat_drop_replslot(), which would result in removing the statistics of the slot once invalidated. However, there is no need to remove the statistics of an invalidated slot as one could still be interested in looking at them to understand the activity of the slot until its actual removal. The initial design of the feature committed in be87200efd used the approach to drop the slots, which is likely why the statistics were still removed during the invalidation. Another problem with this operation is that it was done without holding ReplicationSlotAllocationLock, leaving it unprotected on concurrent activity. This part is arguably a bug, but that's a limited problem in practice so no backpatch is done. In passing, this commit adds a test to check this behavior. The only remaining code path where slot statistics are dropped now related to the slot getting dropped. Author: Bertrand Drouvot Discussion: https://postgr.es/m/ZermH08Eq6YydHpO@ip-10-97-1-34.eu-west-3.compute.internal
* Remove redundant fetch of the recent flush pointer in WalSndWaitForWal.Amit Kapila2024-03-12
| | | | | | | | | | In WalSndWaitForWal(), we fetch a recent flush pointer both outside the loop and inside the loop. But we start using RecentFlushPtr only after we fetch it inside the loop. So we can remove one outside the loop. Author: Shveta Malik Reviewed-by: Bertrand Drouvot, Matthias van de Meent, Amit Kapila Discussion: https://postgr.es/m/CAJpy0uBSCQz1yMD-WiEthzEe23dti2-Kr_pitVb7vAPFbFKm=A@mail.gmail.com
* Use printf's %m format instead of strerror(errno) in more placesMichael Paquier2024-03-12
| | | | | | | | | | | | | | | | | Most callers of strerror() are removed from the backend code. The remaining callers require special handling with a saved errno from a previous system call. The frontend code still needs strerror() where error states need to be handled outside of fprintf. Note that pg_regress is not changed to use %m as the TAP output may clobber errno, since those functions call fprintf() and friends before evaluating the format string. Support for %m in src/port/snprintf.c has been added in d6c55de1f99a, hence all the stable branches currently supported include it. Author: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/87sf13jhuw.fsf@wibble.ilmari.org
* Update obsolete index scan TID comments.Peter Geoghegan2024-03-11
| | | | Oversight in commit c2fe139c20.
* Fix 002_pg_upgrade.pl.Jeff Davis2024-03-11
| | | | | | | | | | | | | | | | | Commit f696c0cd5f caused a test failure in 002_pg_upgrade.pl, because an earlier s/// operator caused qr// to no longer match the empty string. Use qr/^$/ instead, which is a better test anyway, because we expect the stderr to be empty. Initially this appeared to be a perl bug, but per discussion, it seems that it was a misunderstanding of how perl works: an empty pattern uses the last successful pattern. Given how surprising that behavior is to perl non-experts, we will need to look for similar problems elsewhere and eliminate the use of empty patterns throughout the code. For now, address this one instance to fix the buildfarm. Discussion: https://postgr.es/m/0ef325fa06e7a1605c4e119c4ecb637c67e5fb4e.camel@j-davis.com Reviewed-by: Tom Lane
* Add tests for libpq query cancellation APIsAlvaro Herrera2024-03-11
| | | | | | | This is in preparation of making changes and additions to these APIs. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQRb21spiiykQ48rzz8w+Hcykz+mB2_hxR65D9Qk6nnw=w@mail.gmail.com
* reindexdb: Allow specifying objects to process in all databases.Nathan Bossart2024-03-11
| | | | | | | | | | | | | Presently, reindexdb's --table, --schema, --index, and --system options cannot be used together with --all, i.e., you cannot specify objects to process in all databases. This commit removes this unnecessary restriction. Furthermore, it removes the restriction that --system cannot be used with --table, --schema, and --index. There is no such restriction for the latter options, and there is no technical reason to disallow these combinations. Reviewed-by: Kyotaro Horiguchi, Dean Rasheed Discussion: https://postgr.es/m/20230628232402.GA1954626%40nathanxps13
* Remove unneeded vacuum_delay_point from heap_vac_scan_get_next_blockHeikki Linnakangas2024-03-11
| | | | | | | | | | heap_vac_scan_get_next_block() does relatively little work, so there is no need to call vacuum_delay_point(). A future commit will call heap_vac_scan_get_next_block() from a callback, and we would like to avoid calling vacuum_delay_point() in that callback. Author: Melanie Plageman Discussion: https://postgr.es/m/CAAKRu_Yf3gvXGcCnqqfoq0Q8LX8UM-e-qbm_B1LeZh60f8WhWA%40mail.gmail.com
* Confine vacuum skip logic to lazy_scan_skip()Heikki Linnakangas2024-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename lazy_scan_skip() to heap_vac_scan_next_block() and move more code into the function, so that the caller doesn't need to know about ranges or skipping anymore. heap_vac_scan_next_block() returns the next block to process, and the logic for determining that block is all within the function. This makes the skipping logic easier to understand, as it's all in the same function, and makes the calling code easier to understand as it's less cluttered. The state variables needed to manage the skipping logic are moved to LVRelState. heap_vac_scan_next_block() now manages its own VM buffer separately from the caller's vmbuffer variable. The caller's vmbuffer holds the VM page for the current block its processing, while heap_vac_scan_next_block() keeps a pin on the VM page for the next unskippable block. Most of the time they are the same, so we hold two pins on the same buffer, but it's more convenient to manage them separately. For readability inside heap_vac_scan_next_block(), move the logic of finding the next unskippable block to separate function, and add some comments. This refactoring will also help future patches to switch to using a streaming read interface, and eventually AIO (https://postgr.es/m/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com) Author: Melanie Plageman, Heikki Linnakangas Reviewed-by: Andres Freund (older version) Discussion: https://postgr.es/m/CAAKRu_Yf3gvXGcCnqqfoq0Q8LX8UM-e-qbm_B1LeZh60f8WhWA%40mail.gmail.com
* clusterdb: Allow specifying tables to process in all databases.Nathan Bossart2024-03-11
| | | | | | | | | | | | | Presently, clusterdb's --table option cannot be used together with --all, i.e., you cannot specify tables to process in all databases. This commit removes this unnecessary restriction. In passing, change the synopsis in the documentation to use "[option...]" instead of "[--verbose | -v]". There are other general-purpose options (e.g., --quiet and --echo), but the synopsis currently only lists --verbose. Reviewed-by: Kyotaro Horiguchi, Dean Rasheed Discussion: https://postgr.es/m/20230628232402.GA1954626%40nathanxps13
* Add missing connection statuses to docsAlvaro Herrera2024-03-11
| | | | | | | | | | | | | | | | | The list of connection statuses that PQstatus might return during an asynchronous connection attempt was outdated: 1. CONNECTION_SETENV is never returned anymore and is only part of the enum for backwards compatibility, so remove it from the docs. 2. CONNECTION_CHECK_STANDBY and CONNECTION_GSS_STARTUP were not listed, so add them. CONNECTION_NEEDED and CONNECTION_CHECK_TARGET are not listed in the docs on purpose, since these are internal states that can never be observed by a caller of PQstatus. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQRb21spiiykQ48rzz8w+Hcykz+mB2_hxR65D9Qk6nnw=w@mail.gmail.com
* vacuumdb: Allow specifying objects to process in all databases.Nathan Bossart2024-03-11
| | | | | | | | | | | Presently, vacuumdb's --table, --schema, and --exclude-schema options cannot be used together with --all, i.e., you cannot specify tables or schemas to process in all databases. This commit removes this unnecessary restriction, thus enabling potentially useful commands like "vacuumdb --all --schema pg_catalog". Reviewed-by: Kyotaro Horiguchi, Dean Rasheed Discussion: https://postgr.es/m/20230628232402.GA1954626%40nathanxps13
* Set all_visible_according_to_vm correctly with DISABLE_PAGE_SKIPPINGHeikki Linnakangas2024-03-11
| | | | | | | | | | | | | | | | | | | It's important for 'all_visible_according_to_vm' to correctly reflect whether the VM bit is set or not, even when we are not trusting the VM to skip pages, because contrary to what the comment said, lazy_scan_prune() relies on it. If it's incorrectly set to 'false', when the VM bit is in fact set, lazy_scan_prune() will try to set the VM bit again and dirty the page unnecessarily. As a result, if you used DISABLE_PAGE_SKIPPING, all heap pages were dirtied, even if there were no changes. We would also fail to clear any VM bits that were set incorrectly. This was broken in commit 980ae17310, so backpatch to v16. Backpatch-through: 16 Reviewed-by: Melanie Plageman, Peter Geoghegan Discussion: https://www.postgresql.org/message-id/3df2b582-dc1c-46b6-99b6-38eddd1b2784@iki.fi
* Don't destroy SMgrRelations at relcache invalidationHeikki Linnakangas2024-03-11
| | | | | | | | | | | | | | | | With commit 21d9c3ee4e, SMgrRelations remain valid until end of transaction (or longer if they're "pinned"). Relcache invalidation can happen in the middle of a transaction, so we must not destroy them at relcache invalidation anymore. This was revealed by failures in the 'constraints' test in buildfarm animals using -DCLOBBER_CACHE_ALWAYS. That started failing with commit 8af2565248, which was the first commit that started to rely on an SMgrRelation living until end of transaction. Diagnosed-by: Tomas Vondra, Thomas Munro Reviewed-by: Thomas Munro Discussion: https://www.postgresql.org/message-id/CA%2BhUKGK%2B5DOmLaBp3Z7C4S-Yv6yoROvr1UncjH2S1ZbPT8D%2BZg%40mail.gmail.com
* Fix incorrect accessing of pfree'd memory in MemoizeDavid Rowley2024-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | For pass-by-reference types, the code added in 0b053e78b, which aimed to resolve a memory leak, was overly aggressive in resetting the per-tuple memory context which could result in pfree'd memory being accessed resulting in failing to find previously cached results in the hash table. What was happening was prepare_probe_slot() was switching to the per-tuple memory context and calling ExecEvalExpr(). ExecEvalExpr() may have required a memory allocation. Both MemoizeHash_hash() and MemoizeHash_equal() were aggressively resetting the per-tuple context and after determining the hash value, the context would have gotten reset before MemoizeHash_equal() was called. This could have resulted in MemoizeHash_equal() looking at pfree'd memory. This is less likely to have caused issues on a production build as some other allocation would have had to have reused the pfree'd memory to overwrite it. Otherwise, the original contents would have been intact. However, this clearly caused issues on MEMORY_CONTEXT_CHECKING builds. Author: Tender Wang, Andrei Lepikhov Reported-by: Tender Wang (using SQLancer) Reviewed-by: Andrei Lepikhov, Richard Guo, David Rowley Discussion: https://postgr.es/m/CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com Backpatch-through: 14, where Memoize was added
* Improve consistency of replication slot statisticsMichael Paquier2024-03-11
| | | | | | | | | | | | | | | | | | | | | | | | | The replication slot stats stored in shared memory rely on an internal index number. Both pgstat_reset_replslot() and pgstat_fetch_replslot() lacked some LWLock protections with ReplicationSlotControlLock while operating on these index numbers. This issue could cause these two functions to potentially operate on incorrect slots when taken in isolation in the event of slots dropped and/or re-created concurrently. Note that pg_stat_get_replication_slot() is called once per slot when querying pg_stat_replication_slots, meaning that the stats are retrieved across multiple ReplicationSlotControlLock acquisitions. So, while this commit improves more consistency, it may still be possible that statistics are not completely consistent for a single scan of pg_stat_replication_slots under concurrent replication slot drop or creation activity. The issue should unlikely be a problem in practice, causing the report of inconsistent stats or or the stats reset of an incorrect slot, so no backpatch is done. Author: Bertrand Drouvot Reviewed-by: Heikki Linnakangas, Shveta Malik, Michael Paquier Discussion: https://postgr.es/m/ZeGq1HDWFfLkjh4o@ip-10-97-1-34.eu-west-3.compute.internal
* Add some checkpoint and redo LSNs to a couple of recovery errorsMichael Paquier2024-03-11
| | | | | | | | | | | | | | | | | | Two FATALs and one PANIC gain details about the LSNs they fail at: - When restoring from a backup_label, the FATAL log generated when not finding the checkpoint record now reports its LSN. - When restoring from a backup_label, the FATAL log generated when not finding the redo record referenced by a checkpoint record now shows both the redo and checkpoint record LSNs. - When not restoring from a backup_label, the PANIC error generated when not finding the checkpoint record now reports its LSN. This information is useful when debugging corruption issues, and these LSNs may not show up in the logs depending on the level of logging configured in the backend. Author: David Steele Discussion: https://postgr.es/m/0e90da89-77ca-4ccf-872c-9626d755e288@pgmasters.net
* Improve support for ExplainOneQuery() hookMichael Paquier2024-03-11
| | | | | | | | | | | | | | | | | | | | There is a hook called ExplainOneQuery_hook that gives modules the possibility to plug into this code path, but, like utility.c for utility statement execution, there is no corresponding "standard" routine in the case of EXPLAIN executed for one Query. This commit adds a new standard_ExplainOneQuery() in explain.c, which is able to run explain on a non-utility Query without calling its hook. Per the feedback received from a couple of hackers, this change gives the possibility to cut a few hundred lines of code in some of the popular out-of-core modules as these maintained a copy of ExplainOneQuery(), adding custom extra information at the beginning or the end of the EXPLAIN output. Author: Mats Kindahl Reviewed-by: Aleksander Alekseev, Jelte Fennema-Nio, Andrei Lepikhov Discussion: https://postgr.es/m/CA+14427V_B4EAoC_o-iYYucRdMSOTfpuH9k-QbexffY1HYJBiA@mail.gmail.com
* Combine headerscheck and cpluspluscheck scriptsPeter Eisentraut2024-03-10
| | | | | | | | | | | They are mostly the same, and it is tedious to maintain two copies of essentially the same exclude list. headerscheck now has a new option --cplusplus to select the cpluspluscheck functionality. The top-level make targets are still the same. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/4754a5b0-a32b-4036-a99a-6de14cf9fd72@eisentraut.org
* Catalog changes preparing for builtin collation provider.Jeff Davis2024-03-09
| | | | | | | | | | | | Rename pg_collation.colliculocale to colllocale, and pg_database.daticulocale to datlocale. These names reflects that the fields will be useful for the upcoming builtin provider as well, not just for ICU. This is purely a rename; no changes to the meaning of the fields. Discussion: https://postgr.es/m/ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com Reviewed-by: Peter Eisentraut
* Simplify and merge unwanted-module drop logic in AdjustUpgrade.pm.Tom Lane2024-03-09
| | | | | | | | | | In be7800674 and followups, we failed to notice that there was already a better way to do it: instead of using DROP DATABASE IF EXISTS, we can check the list of existing DBs. Also, there seems no reason not to merge this into the pre-existing code for getting rid of unwanted module databases. Discussion: https://postgr.es/m/1066872.1710006597@sss.pgh.pa.us
* Run perltidy on 002_pg_upgrade.pl.Jeff Davis2024-03-09
|
* Fix cross-version pg_upgrade test.Jeff Davis2024-03-09
| | | | | | | | Pass each statement as a separate '-c' arg, so they don't get combined into a single transaction. Discussion: https://postgr.es/m/bca97aecb50b2026b7dbc26604bf31861c819a64.camel@j-davis.com Reviewed-by: Tom Lane
* Document units of "timeout" in ConditionVariableTimedSleep()Michael Paquier2024-03-09
| | | | | | | The timeout is passed down to WaitLatch() as milliseconds. Author: Shveta Malik Discussion: https://postgr.es/m/CAJpy0uC=xiBQD1WapgYYvOiytap6ULJaakLd867zZXqu9tYc8w@mail.gmail.com
* Fix type signedness error in commit 5c40364dd6.Jeff Davis2024-03-08
| | | | | | | Use ssize_t instead of size_t. Discussion: https://postgr.es/m/b20d6d97-7338-48ea-ba33-837a1c8ef98e@iki.fi Reported-by: Heikki Linnakangas