aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Remove dependence on -fwrapv semantics in jsonb.Nathan Bossart2024-08-16
| | | | | | | | | | | | | | | | | | | This commit updates a couple of places in the jsonb code to no longer rely on signed integer wrapping for correctness. Like commit 9e9a2b7031, this is intended to move us closer towards removing -fwrapv, which may enable some compiler optimizations. However, there is presently no plan to actually remove that compiler option in the near future. This commit makes use of the newly introduced pg_abs_s32() routine to negate a signed integer (that is known to be negative) for comparison with an unsigned integer. In passing, change one use of INT_MIN to the more portable PG_INT32_MIN. Reported-by: Alexander Lakhin Author: Joseph Koshakow Reviewed-by: Jian He Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
* Relax fsyncing at end of a bulk load that was not WAL-loggedHeikki Linnakangas2024-08-16
| | | | | | | | | And improve the comments. Backpatch to v17 where this was introduced. Reviewed-by: Noah Misch Discussion: https://www.postgresql.org/message-id/cac7d1b6-8358-40be-af0b-21bc9b27d34c@iki.fi
* Refactor CopyOneRowToHeikki Linnakangas2024-08-16
| | | | | | | | | | The handling of binary and text formats are quite different here, so it's more clear to check for the format first and have two separate loops. Author: jian he <jian.universality@gmail.com> Reviewed-by: Ilia Evdokimov, Junwang Zhao Discussion: https://www.postgresql.org/message-id/CACJufxFzHCeFBQF0M%2BSgk_NwknWuQ4oU7tS1isVeBrbhcKOHkg@mail.gmail.com
* Remove unused 'cur_skey' argument from IndexScanOK()Heikki Linnakangas2024-08-16
| | | | | | | | Commit a78fcfb51243 removed the last use of it. Author: Hugo Zhang, Aleksander Alekseev Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/NT0PR01MB129459E243721B954611938F9CDD2%40NT0PR01MB1294.CHNPR01.prod.partner.outlook.cn
* libpq: Fix minor TOCTOU violationPeter Eisentraut2024-08-16
| | | | | | | | | | | | | | | libpq checks the permissions of the password file before opening it. The way this is done in two separate operations, a static analyzer would flag as a time-of-check-time-of-use violation. In practice, you can't do anything with that, but it still seems better style to fix it. To fix it, open the file first and then check the permissions on the opened file handle. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Reviewed-by: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/a3356054-14ae-4e7a-acc6-249d19dac20b%40eisentraut.org
* Remove dependence on -fwrapv semantics in a few places.Nathan Bossart2024-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | This commit attempts to update a few places, such as the money, numeric, and timestamp types, to no longer rely on signed integer wrapping for correctness. This is intended to move us closer towards removing -fwrapv, which may enable some compiler optimizations. However, there is presently no plan to actually remove that compiler option in the near future. Besides using some of the existing overflow-aware routines in int.h, this commit introduces and makes use of some new ones. Specifically, it adds functions that accept a signed integer and return its absolute value as an unsigned integer with the same width (e.g., pg_abs_s64()). It also adds functions that accept an unsigned integer, store the result of negating that integer in a signed integer with the same width, and return whether the negation overflowed (e.g., pg_neg_u64_overflow()). Finally, this commit adds a couple of tests for timestamps near POSTGRES_EPOCH_JDATE. Author: Joseph Koshakow Reviewed-by: Tom Lane, Heikki Linnakangas, Jian He Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
* Clean up indentation and whitespace inconsistencies in ecpg.Tom Lane2024-08-15
| | | | | | | | | | | | ecpg's lexer and parser files aren't normally processed by pgindent, and unsurprisingly there's a lot of code in there that doesn't really match project style. I spent some time running pgindent over the fragments of these files that are C code, and this is the result. This is in the same spirit as commit 30ed71e42, though apparently Peter used a different method for that one, since it didn't find these problems. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
* Do not hardcode PG_PROTOCOL_LATEST in NegotiateProtocolVersionRobert Haas2024-08-15
| | | | | | | | | | | | | | | | | | We shouldn't ask the client to use a protocol version later than the one that they requested. To avoid that, if the client requests a version newer than the latest one we support, set FrontendProtocol to the latest version we support, not the requested version. Then, use that value when building the NegotiateProtocolVersion message. (It seems good on general principle to avoid setting FrontendProtocol to a version we don't support, anyway.) None of this really matters right now, because we only support a single protocol version, but if that ever changes, we'll need this. Jelte Fennema-Nio, reviewed by me and incorporating some of my proposed wording Discussion: https://postgr.es/m/CAGECzQTyXDNtMXdq2L-Wp=OvOCPa07r6+U_MGb==h90MrfT+fQ@mail.gmail.com
* Optimise numeric multiplication using base-NBASE^2 arithmetic.Dean Rasheed2024-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently mul_var() uses the schoolbook multiplication algorithm, which is O(n^2) in the number of NBASE digits. To improve performance for large inputs, convert the inputs to base NBASE^2 before multiplying, which effectively halves the number of digits in each input, theoretically speeding up the computation by a factor of 4. In practice, the actual speedup for large inputs varies between around 3 and 6 times, depending on the system and compiler used. In turn, this significantly reduces the runtime of the numeric_big regression test. For this to work, 64-bit integers are required for the products of base-NBASE^2 digits, so this works best on 64-bit machines, on which it is faster whenever the shorter input has more than 4 or 5 NBASE digits. On 32-bit machines, the additional overheads, especially during carry propagation and the final conversion back to base-NBASE, are significantly higher, and it is only faster when the shorter input has more than around 50 NBASE digits. When the shorter input has more than 6 NBASE digits (so that mul_var_short() cannot be used), but fewer than around 50 NBASE digits, there may be a noticeable slowdown on 32-bit machines. That seems to be an acceptable tradeoff, given the performance gains for other inputs, and the effort that would be required to maintain code specifically targeting 32-bit machines. Joel Jacobson and Dean Rasheed. Discussion: https://postgr.es/m/9d8a4a42-c354-41f3-bbf3-199e1957db97%40app.fastmail.com
* Extend mul_var_short() to 5 and 6-digit inputs.Dean Rasheed2024-08-15
| | | | | | | | | | | | | | | | | | | Commit ca481d3c9a introduced mul_var_short(), which is used by mul_var() whenever the shorter input has 1-4 NBASE digits and the exact product is requested. As speculated on in that commit, it can be extended to work for more digits in the shorter input. This commit extends it up to 6 NBASE digits (up to 24 decimal digits), for which it also gives a significant speedup. This covers more cases likely to occur in real-world queries, for which using base-NBASE^2 arithmetic provides little benefit. To avoid code bloat and duplication, refactor it a bit using macros and exploiting the fact that some portions of the code are shared between the different cases. Dean Rasheed, reviewed by Joel Jacobson. Discussion: https://postgr.es/m/9d8a4a42-c354-41f3-bbf3-199e1957db97%40app.fastmail.com
* Variable renaming in dbcommands.cPeter Eisentraut2024-08-15
| | | | | | | | | | There were several sets of very similar local variable names, such as "downer" and "dbowner", which was very confusing and error-prone. Rename the former to "ownerEl" and so on, similar to collationcmds.c and typecmds.c. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://www.postgresql.org/message-id/flat/e5bce225-ee04-40c7-a280-ea7214318048%40eisentraut.org
* Improve ALTER PUBLICATION validation and error messagesDavid Rowley2024-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | Attempting to add a system column for a table to an existing publication would result in the not very intuitive error message of: ERROR: negative bitmapset member not allowed Here we improve that to have it display the same error message as a user would see if they tried adding a system column for a table when adding it to the publication in the first place. Doing this requires making the function which validates the list of columns an extern function. The signature of the static function wasn't an ideal external API as it made the code more complex than it needed to be. Here we adjust the function to have it populate a Bitmapset of attribute numbers. Doing it this way allows code simplification. There was no particular bug here other than the weird error message, so no backpatch. Bug: #18558 Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Peter Smith, David Rowley Discussion: https://postgr.es/m/18558-411bc81b03592125@postgresql.org
* libpq: Trace responses to SSLRequest and GSSENCRequestAlvaro Herrera2024-08-14
| | | | | | | | | | Since these are single bytes instead of v2 or v3 messages they need custom tracing logic. These "messages" don't even have official names in the protocol specification, so I (Jelte) called them SSLResponse and GSSENCResponse here. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
* Apply PGDLLIMPORT markings to some GUC variablesPeter Eisentraut2024-08-14
| | | | | | | | | | | | According to the commit message in 8ec569479, we must have all variables in header files marked with PGDLLIMPORT. In commit d3cc5ffe81f6 some variables were moved from launch_backend.c file to several header files. This adds PGDLLIMPORT to moved variables. Author: Sofia Kopikova <s.kopikova@postgrespro.ru> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/e0b17014-5319-4dd6-91cd-93d9c8fc9539%40postgrespro.ru
* Remove TRACE_SORT macroPeter Eisentraut2024-08-14
| | | | | | | | | | | | The TRACE_SORT macro guarded the availability of the trace_sort GUC setting. But it has been enabled by default ever since it was introduced in PostgreSQL 8.1, and there have been no reports that someone wanted to disable it. So just remove the macro to simplify things. (For the avoidance of doubt: The trace_sort GUC is still there. This only removes the rarely-used macro guarding it.) Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/be5f7162-7c1d-44e3-9a78-74dcaa6529f2%40eisentraut.org
* Harmonize MinGW CODESET lookup with MSVC.Thomas Munro2024-08-14
| | | | | | | | | | | | | | | | | | | | | | Historically, MinGW environments lacked some Windows API calls, so we took a different code path in win32_langinfo(). Somehow, the code change in commit 35eeea62 (removing setlocale() calls) caused one particular 001_initdb.pl test to fail on MinGW + ICU builds, because pg_import_system_collations() found no collations. It might take a MinGW user to discover the exact reason. Updating that function to use the same code as MSVC seems to fix that test, so lets do that. (There are plenty more places that test for MSVC unnecessarily, to be investigated later.) While here, also rename the helper function win32_langinfo() to win32_get_codeset(), to explain what it does less confusingly; it's not really a general langinfo() substitute. Noticed by triggering the optional MinGW CI task; no build farm animals failed. Discussion: https://postgr.es/m/CA%2BhUKGKBWfhXQ3J%2B2Lj5PhKvQnGD%3DsywA0XQcb7boTCf%3DerVLg%40mail.gmail.com
* Add resource statistics reporting to ANALYZE VERBOSE.Masahiko Sawada2024-08-13
| | | | | | | | | | | | | | Previously, log_autovacuum_min_duration utilized dedicated code for logging resource statistics, such as system and buffer usage during autoanalyze. However, this logging functionality was not utilized by ANALYZE VERBOSE. This commit adds resource statistics reporting to ANALYZE VERBOSE by reusing the same logging code as autoanalyze. Author: Anthonin Bonnefoy Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA%40mail.gmail.com
* Use pgBufferUsage for buffer usage tracking in analyze.Masahiko Sawada2024-08-13
| | | | | | | | | | | | | | | | | | | | Previously, (auto)analyze used global variables VacuumPageHit, VacuumPageMiss, and VacuumPageDirty to track buffer usage. However, pgBufferUsage provides a more generic way to track buffer usage with support functions. This change replaces those global variables with pgBufferUsage in analyze. Since analyze was the sole user of those variables, it removes their declarations. Vacuum previously used those variables but replaced them with pgBufferUsage as part of a bug fix, commit 5cd72cc0c. Additionally, it adjusts the buffer usage message in both vacuum and analyze for better consistency. Author: Anthonin Bonnefoy Reviewed-by: Masahiko Sawada, Michael Paquier Discussion: https://postgr.es/m/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA%40mail.gmail.com
* Include <xlocale.h> for macOS, take II.Thomas Munro2024-08-13
| | | | | | Fix typo in macro name. Discussion: https://postgr.es/m/CA%2BhUKG%2Bk-o3N_SyNJNJpAcdtMo_HheN30miAeXehk9yw%3D9WYzA%40mail.gmail.com
* Include <xlocale.h> for older macOS.Thomas Munro2024-08-13
| | | | | | | Commit 35eeea62 forgot to include <xlocale.h> when using locale_t (which didn't seem to be required on newer Apple SDK as used by CI, hence mistake). Let's see if this fixes build farm animals longfin and sifika.
* Use thread-safe nl_langinfo_l(), not nl_langinfo().Thomas Munro2024-08-13
| | | | | | | | | | | | | This gets rid of some setlocale() calls. The remaining call to setlocale() in pg_get_encoding_from_locale() is a query of the name of the current locale when none was provided (in a multi-threaded future that would need more work). All known non-Windows targets have nl_langinfo_l(), from POSIX 2008, and for Windows we already do something thread-safe. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CA%2BhUKGJqVe0%2BPv9dvC9dSums_PXxGo9SWcxYAMBguWJUGbWz-A%40mail.gmail.com
* All POSIX systems have langinfo.h and CODESET.Thomas Munro2024-08-13
| | | | | | | | | | We don't need configure probes for HAVE_LANGINFO_H (it is implied by !WIN32), and we don't need to consider systems that have it but don't define CODESET (that was for OpenBSD in commit 81cca218, but it has now had it for 19 years). Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CA%2BhUKGJqVe0%2BPv9dvC9dSums_PXxGo9SWcxYAMBguWJUGbWz-A%40mail.gmail.com
* Use errmsg_internal for debug messagesPeter Eisentraut2024-08-13
| | | | Some newer code was applying this inconsistently.
* Rename C23 keywordPeter Eisentraut2024-08-13
| | | | | | | | constexpr is a keyword in C23. Rename a conflicting identifier for future-proofing. Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/08abc832-1384-4aca-a535-1a79765b565e%40eisentraut.org
* libpq: Trace frontend authentication challengesAlvaro Herrera2024-08-12
| | | | | | | | | | | | | | | | | | If tracing was enabled during connection startup, these messages would previously be listed in the trace output as something like this: F 54 Unknown message: 70 mismatched message length: consumed 4, expected 54 With this commit their type and contents are now correctly listed: F 36 StartupMessage 3 0 "user" "foo" "database" "alvherre" F 54 SASLInitialResponse "SCRAM-SHA-256" 32 'n,,n=,r=nq5zEPR/VREHEpOAZzH8Rujm' F 108 SASLResponse 'c=biws,r=nq5zEPR/VREHEpOAZzH8RujmVtWZDQ8glcrvy9OMNw7ZqFUn,p=BBwAKe0WjSvigB6RsmmArAC+hwucLeuwJrR5C/HQD5M=' Author: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
* Fix nls.mk to reflect astreamer files relocationAlvaro Herrera2024-08-12
| | | | | | | | | In the recent commit f80b09bac8, astreamer files were moved to another directory, but this change was not reflected in nls.mk. This commit corrects that oversight. Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/20240806.102123.648178476296575604.horikyota.ntt@gmail.com
* Fix creation of partition descriptor during concurrent detach+dropAlvaro Herrera2024-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If a partition undergoes DETACH CONCURRENTLY immediately followed by DROP, this could cause a problem for a concurrent transaction recomputing the partition descriptor when running a prepared statement, because it tries to dereference a pointer to a tuple that's not found in a catalog scan. The existing retry logic added in commit dbca3469ebf8 is sufficient to cope with the overall problem, provided we don't try to dereference a non-existant heap tuple. Arguably, the code in RelationBuildPartitionDesc() has been wrong all along, since no check was added in commit 898e5e3290a7 against receiving a NULL tuple from the catalog scan; that bug has only become user-visible with DETACH CONCURRENTLY which was added in branch 14. Therefore, even though there's no known mechanism to cause a crash because of this, backpatch the addition of such a check to all supported branches. In branches prior to 14, this would cause the code to fail with a "missing relpartbound for relation XYZ" error instead of crashing; that's okay, because there are no reports of such behavior anyway. Author: Kuntal Ghosh <kuntalghosh.2007@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/18559-b48286d2eacd9a4e@postgresql.org
* Remove unnecessary check for NULL locale, per Coverity.Jeff Davis2024-08-12
| | | | | Discussion: https://postgr.es/m/3804933.1723394010@sss.pgh.pa.us Reported-by: Tom Lane
* Give nbtree move right function internal linkage.Peter Geoghegan2024-08-12
| | | | | | | | Declare _bt_moveright() static. This is a minor modularity win; the routine was already private to nbtsearch.c for all practical purposes. Author: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAEze2WgWVzCNEXQB_op5MMZMDgJ3fg3AhVm6bq2iZPpJNXGhWw@mail.gmail.com
* Log more info when wait-for-catchup tests time out.Tom Lane2024-08-12
| | | | | | | | | | | Cluster.pm's wait_for_catchup and allied subroutines don't provide enough information to diagnose the problem when a wait times out. In hopes of debugging some intermittent buildfarm failures, let's dump the ending state of the relevant system view when that happens. Add this to v17 too, but not stable branches. Discussion: https://postgr.es/m/352068.1723422725@sss.pgh.pa.us
* Add user-callable CRC functions.Nathan Bossart2024-08-12
| | | | | | | | | | | | | | We've had code for CRC-32 and CRC-32C for some time (for WAL records, etc.), but there was no way for users to call it, despite apparent popular demand. The new crc32() and crc32c() functions accept bytea input and return bigint (to avoid returning negative values). Bumps catversion. Author: Aleksander Alekseev Reviewed-by: Peter Eisentraut, Tom Lane Discussion: https://postgr.es/m/CAJ7c6TNMTGnqnG%3DyXXUQh9E88JDckmR45H2Q%2B%3DucaCLMOW1QQw%40mail.gmail.com
* Fix outdated commentsDavid Rowley2024-08-12
| | | | | | | | A few fields in ResultRelInfo are now also used for MERGE. Update the comments to mention that. Reported-by: jian he <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxH8-NvFhLcSZZTTW+1M9AfS4+SOTKmyPG7ZhzNvN=+NkA@mail.gmail.com:wq
* Fix a series of typos and outdated referencesDavid Rowley2024-08-12
| | | | | Author: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/c1d63754-cb85-2d8a-8409-bde2c4d2d04b@gmail.com
* Fix bad indentation introduced in commit f011e82c2cHeikki Linnakangas2024-08-12
|
* Consolidate postmaster code to launch background processesHeikki Linnakangas2024-08-12
| | | | | | | | | | | | | | Much of the code in process_pm_child_exit() to launch replacement processes when one exits or when progressing to next postmaster state was unnecessary, because the ServerLoop will launch any missing background processes anyway. Remove the redundant code and let ServerLoop handle it. In ServerLoop, move the code to launch all the processes to a new subroutine, to group it all together. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/8f2118b9-79e3-4af7-b2c9-bd5818193ca4@iki.fi
* Remove dead codePeter Eisentraut2024-08-12
| | | | | | | | | After e9931bfb751, the locale argument of SB_lower_char() is never NULL, so the branch that deals with NULL can be removed (similar to how e9931bfb751 for example removed those branches in str_tolower()). Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://www.postgresql.org/message-id/4f562d84-87f4-44dc-8946-01d6c437936f@eisentraut.org
* Remove fe_memutils from libpgcommon_shlibPeter Eisentraut2024-08-12
| | | | | | | | | | | | libpq must not use palloc/pfree. It's not allowed to exit on allocation failure, and mixing the frontend pfree with malloc is architecturally unsound. Remove fe_memutils from the shlib build entirely, to keep devs from accidentally depending on it in the future. Author: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/CAOYmi+=pg=W5L1h=3MEP_EB24jaBu2FyATrLXqQHGe7cpuvwyg@mail.gmail.com
* Remove support for old realpath() APIPeter Eisentraut2024-08-12
| | | | | | | | | | | | | The now preferred way to call realpath() is by passing NULL as the second argument and get a malloc'ed result. We still supported the old way of providing our own buffer as a second argument, for some platforms that didn't support the new way yet. Those were only Solaris less than version 11 and some older AIX versions (7.1 and newer appear to support the new variant). We don't support those platforms versions anymore, so we can remove this extra code. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/9e638b49-5c3f-470f-a392-2cbedb2f7855%40eisentraut.org
* Remove "parent" column from pg_backend_memory_contextsDavid Rowley2024-08-12
| | | | | | | | | | | | | | | | | | | | | | | | 32d3ed816 added the "path" column to pg_backend_memory_contexts to allow a stable method of obtaining the parent MemoryContext of a given row in the view. Using the "path" column is now the preferred method of obtaining the parent row. Previously, any queries which were self-joining to this view using the "name" and "parent" columns could get incorrect results due to the fact that names are not unique. Here we aim to explicitly break such queries so that they can be corrected and use the "path" column instead. It is possible that there are more innocent users of the parent column that just need an indication of the parent and having to write out a self-joining CTE may be an unnecessary hassle for those cases. Let's remove the column for now and see if anyone comes back with any complaints. This does seem like a good time to attempt to get rid of the column as we still have around 1 year to revert this if someone comes back with a valid complaint. Plus this view is new to v14 and is quite niche, so perhaps not many people will be affected. Author: Melih Mutlu <m.melihmutlu@gmail.com> Discussion: https://postgr.es/m/CAGPVpCT7NOe4fZXRL8XaoxHpSXYTu6GTpULT_3E-HT9hzjoFRA@mail.gmail.com
* Avoid unneeded nbtree backwards scan buffer locks.Peter Geoghegan2024-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach nbtree backwards scans to avoid relocking a just-read leaf page to read its current left sibling link when it isn't truly necessary. This happened inside _bt_readnextpage whenever _bt_readpage had already determined that there'll be no further matches to the left (or at least none for the current primitive index scan, for a scan with array keys). A new precheck inside _bt_readnextpage is all that we need to avoid these useless lock acquisitions. Arguably, using a precheck like this was a missed opportunity for commit 2ed5b87f96, which taught nbtree to drop leaf page pins early to avoid blocking cleanup by VACUUM. Forwards scans already managed to avoid relocking the page like this. The optimization added by this commit is particularly helpful with backwards scans that use array keys where the scan must perform multiple primitive index scans. Such backwards scans will now avoid a useless leaf page re-lock at the end of each primitive index scan. Note that this commit does not attempt to avoid needlessly re-locking a leaf page that was just read when the scan must follow the leaf page's left link. That more ambitious optimization could work by stashing the left link when the page is first read by a backwards scan, allowing the subsequent _bt_readnextpage call to optimistically skip re-reading the original page just to get a new copy of its left link. For now we only address cases where we don't care about our original page's left link. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-Wz=xgs7PojG=EUvhgadwENzu_mY_riNh-w9wFPsaS717ew@mail.gmail.com
* Initialize HASHCTL differently, to suppress Coverity warningHeikki Linnakangas2024-08-11
| | | | | | | | Coverity complained that the hash_create() call might access hash_table_ctl->hctl. That's a false alarm, hash_create() only accesses that field when passed the HASH_SHARED_MEM flag. Try to silence it by using a plain local variable instead of a const. That's how the HASHCTL is initialized in all the other hash_create() calls.
* Suppress Coverity warnings about Asserts in get_name_for_var_field.Tom Lane2024-08-11
| | | | | | | | | Coverity thinks dpns->plan could be null at these points. That shouldn't really be possible, but it's easy enough to modify the Asserts so they'd not core-dump if it were true. These are new in b919a97a6. Back-patch to v13; the v12 version of the patch didn't have these Asserts.
* Allow adjusting session_authorization and role in parallel workers.Tom Lane2024-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | The code intends to allow GUCs to be set within parallel workers via function SET clauses, but not otherwise. However, doing so fails for "session_authorization" and "role", because the assign hooks for those attempt to set the subsidiary "is_superuser" GUC, and that call falls foul of the "not otherwise" prohibition. We can't switch to using GUC_ACTION_SAVE for this, so instead add a new GUC variable flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set anyway. (This is okay because is_superuser has context PGC_INTERNAL and thus only hard-wired calls can change it. We'd need more thought before applying the flag to other GUCs; but maybe there are other use-cases.) This isn't the prettiest fix perhaps, but other alternatives we thought of would be much more invasive. While here, correct a thinko in commit 059de3ca4: when rejecting a GUC setting within a parallel worker, we should return 0 not -1 if the ereport doesn't longjmp. (This seems to have no consequences right now because no caller cares, but it's inconsistent.) Improve the comments to try to forestall future confusion of the same kind. Despite the lack of field complaints, this seems worth back-patching. Thanks to Nathan Bossart for the idea to invent a new flag, and for review. Discussion: https://postgr.es/m/2833457.1723229039@sss.pgh.pa.us
* Add tests for pg_wal_replay_wait() errorsAlexander Korotkov2024-08-10
| | | | | Improve test coverage for pg_wal_replay_wait() procedure by adding test cases when it errors out.
* Improve header comment for WaitLSNSetLatches()Alexander Korotkov2024-08-10
| | | | | Reflect the fact that we remove waiters from the heap, not just set their latches.
* Adjust pg_wal_replay_wait() procedure behavior on promoted standbyAlexander Korotkov2024-08-10
| | | | | | | | | | | | pg_wal_replay_wait() is intended to be called on standby. However, standby can be promoted to primary at any moment, even concurrently with the pg_wal_replay_wait() call. If recovery is not currently in progress that doesn't mean the wait was unsuccessful. Thus, we always need to recheck if the target LSN is replayed. Reported-by: Kevin Hale Boyes Discussion: https://postgr.es/m/CAPpHfdu5QN%2BZGACS%2B7foxmr8_nekgA2PA%2B-G3BuOUrdBLBFb6Q%40mail.gmail.com Author: Alexander Korotkov
* Lower minimum maintenance_work_mem to 64kBJohn Naylor2024-08-10
| | | | | | | | | | | | | | | | | | Since the introduction of TID store, vacuum uses far less memory in the common case than in versions 16 and earlier. Invoking multiple rounds of index vacuuming in turn requires a much larger table. It'd be a good idea anyway to cover this case in regression testing, and a lower limit is less painful for slow buildfarm animals. The reason to do it now is to re-enable coverage of the bugfix in commit 83c39a1f7f. For consistency, give autovacuum_work_mem the same treatment. Suggested by Andres Freund Tested by Melanie Plageman Backpatch to v17, where TID store was introduced Discussion: https://postgr.es/m/20240516205458.ohvlzis5b5tvejru@awork3.anarazel.de Discussion: https://postgr.es/m/20240722164745.fvaoh6g6zprisqgp%40awork3.anarazel.de
* Fix inappropriate uses of atol()Peter Eisentraut2024-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | Some code using atol() would not work correctly if sizeof(long)==4: - src/bin/pg_basebackup/pg_basebackup.c: Would miscount size of a tablespace over 2 TB. - src/bin/pg_basebackup/streamutil.c: Would truncate a timeline ID beyond INT32_MAX. - src/bin/pg_rewind/libpq_source.c: Would miscount size of files larger than 2 GB (but this currently cannot happen). Replace these with atoll(). In one case, the use of atol() did not result in incorrect behavior but seems inconsistent with related code: - src/interfaces/ecpg/ecpglib/execute.c: Gratuitous, since it processes a value from pg_type.typlen, which is int16. Replace this with atoi(). Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/a52738ad-06bc-4d45-b59f-b38a8a89de49%40eisentraut.org
* libpq: Trace StartupMessage/SSLRequest/GSSENCRequest correctlyAlvaro Herrera2024-08-09
| | | | | | | | | | | libpq tracing via PQtrace would uselessly print the wrong thing for these types of messages. With this commit, their type and contents would be correctly listed. (This can be verified with PQconnectStart(), but we don't use that in libpq_pipeline, so I (Álvaro) haven't bothered to add any tests.) Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
* Fix comment on processes being kept over a restartHeikki Linnakangas2024-08-10
| | | | | | | | | | | | | All child processes except the syslogger are killed on a restart. The archiver might be already running though, if it was started during recovery. The split in the comments between "other special children" and the first group of "background tasks" seemed really arbitrary, so I just merged them all into one group. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/8f2118b9-79e3-4af7-b2c9-bd5818193ca4@iki.fi