aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Don't expose Windows' mbstowcs_l() and wcstombs_l().Thomas Munro2023-07-11
| | | | | | | | | | | | | | | | | | | | Windows has similar functions with leading underscores. Previously, we provided the rename via a macro in win32_port.h. In fact its functions are not always good replacements for the Unix functions, since they can't deal with UTF-8. They are only currently used by pg_locale.c, which is careful to redirect to other Windows routines for UTF-8. Given that portability hazard, it seem unlikely to be a good idea to encourage any other code to think of these functions as being available outside pg_locale.c. Any code that thinks it wants these functions probably wants our wchar2char() or char2wchar() routines instead, or it won't actually work on Windows in UTF-8 databases. Furthermore, some major libc implementations including glibc don't have them (they only have the standard variants without _l), so external code is very unlikely to require them to exist. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/CA%2BhUKG%2Bt_CHPzEoPnKyARJBJgE9-GxNajJo6ZuSfRK_KWFO%2B6w%40mail.gmail.com
* Be more rigorous about local variables in PostgresMain().Tom Lane2023-07-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since PostgresMain calls sigsetjmp, any local variables that are not marked "volatile" have a risk of unspecified behavior. In practice this means that when control returns via longjmp, such variables might get reset to their values as of the time of sigsetjmp, depending on whether the compiler chose to put them in registers or on the stack. We were careful about this for "send_ready_for_query", but not the other local variables. In the case of the timeout_enabled flags, resetting them to their initial "false" states is actually good, since we do "disable_all_timeouts()" in the longjmp cleanup code path. If that does not happen, we risk uselessly calling "disable_timeout()" later, which is harmless but a little bit expensive. Let's explicitly reset these flags so that the behavior is correct and platform-independent. (This change means that we really don't need the new "volatile" markings after all, but let's install them anyway since any change in this logic could re-introduce a problem.) There is no issue for "firstchar" and "input_message" because those are explicitly reinitialized each time through the query processing loop. To make that clearer, move them to be declared inside the loop. That leaves us with all the function-lifespan locals except the sigjmp_buf itself marked as volatile, which seems like a good policy to have going forward. Because of the possibility of extra disable_timeout() calls, this seems worth back-patching. Sergey Shinderuk and Tom Lane Discussion: https://postgr.es/m/2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru
* Fix pgindentPeter Eisentraut2023-07-10
| | | | for commit e53a611523
* Message wording improvementsPeter Eisentraut2023-07-10
|
* Add more sanity checks with callers of changeDependencyFor()Michael Paquier2023-07-10
| | | | | | | | | | | | | | | | | | changeDependencyFor() returns the number of pg_depend entries changed, or 0 if there is a problem. The callers of this routine expect only one dependency to change, but they did not check for the result returned. The following code paths gain checks: - Namespace for extensions. - Namespace for various object types (see AlterObjectNamespace). - Planner support function for a function. Some existing error messages related to all that are reworded to be more consistent with the project style, and the new error messages added follow the same style. This change has exposed one bug fixed a bit earlier with bd5ddbe. Reviewed-by: Heikki Linnakangas, Akshat Jaimini Discussion: https://postgr.es/m/ZJzD/rn+UbloKjB7@paquier.xyz
* Fix ALTER EXTENSION SET SCHEMA with objects outside an extension's schemaMichael Paquier2023-07-10
| | | | | | | | | | | | | | | | | | | | | | | | As coded, the code would use as a base comparison the namespace OID from the first object scanned in pg_depend when switching its namespace dependency entry to the new one, and use it as a base of comparison for any follow-up checks. It would also be used as the old namespace OID to switch *from* for the extension's pg_depend entry. Hence, if the first object scanned has a namespace different than the one stored in the extension, we would finish by: - Not checking that the extension objects map with the extension's schema. - Not switching the extension -> namespace dependency entry to the new namespace provided by the user, making ALTER EXTENSION ineffective. This issue exists since this command has been introduced in d9572c4 for relocatable extension, so backpatch all the way down to 11. The test case has been provided by Heikki, that I have tweaked a bit to show the effects on pg_depend for the extension. Reported-by: Heikki Linnakangas Author: Michael Paquier, Heikki Linnakangas Discussion: https://postgr.es/m/20eea594-a05b-4c31-491b-007b6fceef28@iki.fi Backpatch-through: 11
* Remove unnecessary unbind in LDAP search+bind modePeter Eisentraut2023-07-09
| | | | | | | | | | | | | | | | | | | | | | | | Comments in src/backend/libpq/auth.c say: (after successfully finding the final DN to check the user-supplied password against) /* Unbind and disconnect from the LDAP server */ and later /* * Need to re-initialize the LDAP connection, so that we can bind to * it with a different username. */ But the protocol actually permits multiple subsequent authentications ("binds") over a single connection. So, it seems like the whole connection re-initialization thing was just a confusion and can be safely removed, thus saving quite a few network round-trips, especially for the case of ldaps/starttls. Author: Anatoly Zaretsky <anatoly.zaretsky@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CALbq6kmJ-1+58df4B51ctPfTOSyPbY8Qi2=ct8oR=i4TamkUoQ@mail.gmail.com
* All supported systems have locale_t.Thomas Munro2023-07-09
| | | | | | | | | | | | | | | | | | | | locale_t is defined by POSIX.1-2008 and SUSv4, and available on all targeted systems. For Windows, win32_port.h redirects to a partial implementation called _locale_t. We can now remove a lot of compile-time tests for HAVE_LOCALE_T, and associated comments and dead code branches that were needed for older computers. Since configure + MinGW builds didn't detect locale_t but now we assume that all systems have it, further inconsistencies among the 3 Windows build systems were revealed. With this commit, we no longer define HAVE_WCSTOMBS_L and HAVE_MBSTOWCS_L on any Windows build system, but we have logic to deal with that so that replacements are available where appropriate. Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Tristan Partin <tristan@neon.tech> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/CA%2BhUKGLg7_T2GKwZFAkEf0V7vbnur-NfCjZPKZb%3DZfAXSV1ORw%40mail.gmail.com
* Make some indentation in gram.y consistentPeter Eisentraut2023-07-08
| | | | | Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Revert MAINTAIN privilege and pg_maintain predefined role.Nathan Bossart2023-07-07
| | | | | | | | | | | | | | | | This reverts the following commits: 4dbdb82513, c2122aae63, 5b1a879943, 9e1e9d6560, ff9618e82a, 60684dd834, 4441fc704d, and b5d6382496. A role with the MAINTAIN privilege may be able to use search_path tricks to escalate privileges to the table owner. Unfortunately, it is too late in the v16 development cycle to apply the proposed fix, i.e., restricting search_path when running maintenance commands. Bumps catversion. Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org Backpatch-through: 16
* Document relaxed HOT for summarizing indexesTomas Vondra2023-07-07
| | | | | | | | | | | | | Commit 19d8e2308b allowed a weaker check for HOT with summarizing indexes, but it did not update README.HOT. So do that now. Patch by Matthias van de Meent, minor changes by me. Backpatch to 16, where the optimization was introduced. Author: Matthias van de Meent Reviewed-by: Tomas Vondra Backpatch-through: 16 Discussion: https://postgr.es/m/CAEze2WiEOm8V+c9kUeYp2BPhbEc5s473fUf51xNeqvSFGv44Ew@mail.gmail.com
* WAL-log the creation of the init fork of unlogged indexes.Heikki Linnakangas2023-07-06
| | | | | | | | | | | | | | | | | | | We create a file, so we better WAL-log it. In practice, all the built-in index AMs and all extensions that I'm aware of write a metapage to the init fork, which is WAL-logged, and replay of the metapage implicitly creates the fork too. But if ambuildempty() didn't write any page, we would miss it. This can be seen with dummy_index_am. Set up replication, create a 'dummy_index_am' index on an unlogged table, and look at the files created in the replica: the init fork is not created on the replica. Dummy_index_am doesn't do anything with the relation files, however, so it doesn't lead to any user-visible errors. Backpatch to all supported versions. Reviewed-by: Robert Haas Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9%40iki.fi
* Fix code indentation vioaltion introduced in commit cc32ec24fd.Amit Kapila2023-07-06
| | | | | | Per buildfarm member koel Discussion: https://postgr.es/m/CAA4eK1K2Y_iegdXRUNbsghY9+b-4cSOrxYt9T8TtwXkkdWMVJA@mail.gmail.com
* Add GUC parameter "huge_pages_status"Michael Paquier2023-07-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is useful to show the allocation state of huge pages when setting up a server with "huge_pages = try", where allocating huge pages would be attempted but the server would continue its startup sequence even if the allocation fails. The effective status of huge pages is not easily visible without OS-level tools (or for instance, a lookup at /proc/N/smaps), and the environments where Postgres runs may not authorize that. Like the other GUCs related to huge pages, this works for Linux and Windows. This GUC can report as values: - "on", if huge pages were allocated. - "off", if huge pages were not allocated. - "unknown", a special state that could only be seen when using for example postgres -C because it is only possible to know if the shared memory allocation worked after we can check for the GUC values, even if checking a runtime-computed GUC. This value should never be seen when querying for the GUC on a running server. An assertion is added to check that. The discussion has also turned around having a new function to grab this status, but this would have required more tricks for -DEXEC_BACKEND, something that GUCs already handle. Noriyoshi Shinoda has initiated the thread that has led to the result of this commit. Author: Justin Pryzby Reviewed-by: Nathan Bossart, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/TU4PR8401MB1152EBB0D271F827E2E37A01EECC9@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM
* Add newline at the end of header generated by generate-wait_event_types.plMichael Paquier2023-07-06
| | | | | | | | The header file wait_event_types.h was generated without a newline at its end, which was inconsistent with all the other things generated automatically. Per offline gripe from Nathan Bossart.
* Revert the commits related to allowing page lock to conflict among parallel ↵Amit Kapila2023-07-06
| | | | | | | | | | | | | | | | | | | | group members. This commit reverts the work done by commits 3ba59ccc89 and 72e78d831a. Those commits were incorrect in asserting that we never acquire any other heavy-weight lock after acquring page lock other than relation extension lock. We can acquire a lock on catalogs while doing catalog look up after acquring page lock. This won't impact any existing feature but we need to think some other way to achieve this before parallelizing other write operations or even improving the parallelism in vacuum (like allowing multiple workers for an index). Reported-by: Jaime Casanova Author: Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAJKUy5jffnRKNvRHKQ0LynRb0RJC-o4P8Ku3x9vGAVLwDBWumQ@mail.gmail.com
* Handle \v as a whitespace character in parsersMichael Paquier2023-07-06
| | | | | | | | | | | | | | | | | | | | | This commit comes as a continuation of the discussion that has led to d522b05, as \v was handled inconsistently when parsing array values or anything going through the parsers, and changing a parser behavior in stable branches is a scary thing to do. The parsing of array values now uses the more central scanner_isspace() and array_isspace() is removed. As pointing out by Peter Eisentraut, fix a confusing reference to horizontal space in the parsers with the term "horiz_space". \f was included in this set since 3cfdd8f from 2000, but it is not horizontal. "horiz_space" is renamed to "non_newline_space", to refer to all whitespace characters except newlines. The changes impact the parsers for the backend, psql, seg, cube, ecpg and replication commands. Note that JSON should not escape \v, as per RFC 7159, so these are not touched. Reviewed-by: Peter Eisentraut, Tom Lane Discussion: https://postgr.es/m/ZJKcjNwWHHvw9ksQ@paquier.xyz
* Fix leak of LLVM "fatal-on-oom" section counter.Heikki Linnakangas2023-07-05
| | | | | | | | | | | | | | | | llvm_release_context() called llvm_enter_fatal_on_oom(), but was missing the corresponding llvm_leave_fatal_on_oom() call. As a result, if JIT was used at all, we were almost always in the "fatal-on-oom" state. It only makes a difference if you use an extension written in C++, and run out of memory in a C++ 'new' call. In that case, you would get a PostgreSQL FATAL error, instead of the default behavior of throwing a C++ exception. Back-patch to all supported versions. Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/54b78cca-bc84-dad8-4a7e-5b56f764fab5@iki.fi
* Change example in pgindent README on "/*-----" comments.Heikki Linnakangas2023-07-05
| | | | | | | | | | | | | | | | | Most, but not all, of our "/*----" style comments have an end-guard line with dashes at the end of the comment. However, pgindent doesn't care about the end-guards, so they mostly just waste screen space. Going forward, let's not require end-guards. Remove a broken end-guard in a comment in walsender.c that led me to think about this. Remove the end guard from another comment in walsender.c for consistency, so that we use the same style in all comments in the file. However, we have thousands of existing "/*----" comments the repository, so it's not worth the code churn to change them all. Discussion: https://www.postgresql.org/message-id/fb083c91-d490-3b65-25f3-05e9118b6b0d%40iki.fi
* Rename EVT cache hash to make context name uniqueDaniel Gustafsson2023-07-05
| | | | | | | | | | | | | | | BuildEventTriggerCache sets up a context "EventTriggerCache" which house a hash named "Event Trigger Cache", which in turn creates a context with the table name. This generated log output for memory context dumps like below: LOG: level: 2; EventTriggerCache: 8192 total in 1 blocks; 7928 free (4 chunks); 264 used LOG: level: 3; Event Trigger Cache: 8192 total in 1 blocks; 2616 free (0 chunks); 5576 used This renames the hash to ensure that the hash context has a unique name for easier log reading and debugging. Discussion: https://postgr.es/m/5EDC969E-CAE3-4CBD-965E-3B8A1294CFA4@yesql.se
* pgstat: fix subscription stats entry leak.Masahiko Sawada2023-07-05
| | | | | | | | | | | | | | | | | | Commit 7b64e4b3 taught DropSubscription() to drop stats entry of subscription that is not associated with a replication slot for apply worker at DROP SUBSCRIPTION but missed covering the case where the subscription is not associated with replication slots for both apply worker and tablesync worker. Also add a test to verify that the stats for slot-less subscription is removed at DROP SUBSCRIPTION time. Backpatch down to 15. Author: Masahiko Sawada Reviewed-by: Nathan Bossart, Hayato Kuroda, Melih Mutlu, Amit Kapila Discussion: https://postgr.es/m/CAD21AoB71zkP7uPT7JDPsZcvp0749ExEQnOJxeNKPDFisHar+w@mail.gmail.com Backpatch-through: 15
* Generate automatically code and documentation related to wait eventsMichael Paquier2023-07-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The documentation and the code is generated automatically from a new file called wait_event_names.txt, formatted in sections dedicated to each wait event class (Timeout, Lock, IO, etc.) with three tab-separated fields: - C symbol in enums - Format in the system views - Description in the docs Using this approach has several advantages, as we have proved to be rather bad in maintaining this area of the tree across the years: - The order of each item in the documentation and the code, which should be alphabetical, has become incorrect multiple times, and the script generating the code and documentation has a few rules to enforce that, making the maintenance a no-brainer. - Some wait events were added to the code, but not documented, so this cannot be missed now. - The order of the tables for each wait event class is enforced in the documentation (the input .txt file does so as well for clarity, though this is not mandatory). - Less code, shaving 1.2k lines from the tree, with 1/3 of the savings coming from the code, the rest from the documentation. The wait event types "Lock" and "LWLock" still have their own code path for their code, hence only the documentation is created for them. These classes are listed with a special marker called WAIT_EVENT_DOCONLY in the input file. Adding a new wait event now requires only an update of wait_event_names.txt, with "Lock" and "LWLock" treated as exceptions. This commit has been tested with configure/Makefile, the CI and VPATH build. clean, distclean and maintainer-clean were working fine. Author: Bertrand Drouvot, Michael Paquier Discussion: https://postgr.es/m/77a86b3a-c4a8-5f5d-69b9-d70bbf2e9b98@gmail.com
* Fix assertion failure in snapshot buildingDaniel Gustafsson2023-07-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Clear any potential stale next_phase_at value from the snapshot builder which otherwise may trip an assertion check ensuring that there is no next_phase_at value. This can be reproduced by running 80 concurrent sessions like the below where $c is a loop counter (assumes there has been 1..$c databases created) : echo " CREATE TABLE replication_example(id SERIAL PRIMARY KEY, somedata int, text varchar(120)); SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_$c', 'test_decoding'); SELECT data FROM pg_logical_slot_get_changes('regression_slot_$c', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); " | psql -d regress_$c >>psql.log & Backpatch down to v16. Bug: #17695 Author: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Reported-by: bowenshi <zxwsbg@qq.com> Discussion: https://postgr.es/m/17695-6be9277c9295985f@postgresql.org Backpatch-through: v16
* Ensure that creation of an empty relfile is fsync'd at checkpoint.Heikki Linnakangas2023-07-04
| | | | | | | | | | | | | | | | | If you create a table and don't insert any data into it, the relation file is never fsync'd. You don't lose data, because an empty table doesn't have any data to begin with, but if you crash and lose the file, subsequent operations on the table will fail with "could not open file" error. To fix, register an fsync request in mdcreate(), like we do for mdwrite(). Per discussion, we probably should also fsync the containing directory after creating a new file. But that's a separate and much wider issue. Backpatch to all supported versions. Reviewed-by: Andres Freund, Thomas Munro Discussion: https://www.postgresql.org/message-id/d47d8122-415e-425c-d0a2-e0160829702d%40iki.fi
* Allow Incremental Sorts on GiST and SP-GiST indexesDavid Rowley2023-07-04
| | | | | | | | | | | | | | | Previously an "amcanorderbyop" index would only be used when the index could provide sorted results which satisfied all query_pathkeys. Here we relax this so that we also allow these indexes to be considered by the planner when they only provide partially sorted results. This allows the planner to later consider making use of an Incremental Sort to satisfy the remaining pathkeys. This change is particularly useful for KNN-type queries which contain a LIMIT clause and an additional ORDER BY clause for a non-indexed column. Author: Miroslav Bendik Reviewed-by: Richard Guo, David Rowley Discussion: https://postgr.es/m/CAPoEpV0QYDtzjwamwWUBqyWpaCVbJV2d6qOD7Uy09bWn47PJtw%40mail.gmail.com
* Re-bin segment when memory pages are freed.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | | | | | It's OK to be lazy about re-binning memory segments when allocating, because that can only leave segments in a bin that's too high. We'll search higher bins if necessary while allocating next time, and also eventually re-bin, so no memory can become unreachable that way. However, when freeing memory, the largest contiguous range of free pages might go up, so we should re-bin eagerly to make sure we don't leave the segment in a bin that is too low for get_best_segment() to find. The re-binning code is moved into a function of its own, so it can be called whenever free pages are returned to the segment's free page map. Back-patch to all supported releases. Author: Dongming Liu <ldming101@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier version) Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CAL1p7e8LzB2LSeAXo2pXCW4%2BRya9s0sJ3G_ReKOU%3DAjSUWjHWQ%40mail.gmail.com
* Remove trailing zero words from BitmapsetsDavid Rowley2023-07-04
| | | | | | | | | | | | | | | | | | | | | Prior to this, Bitmapsets could contain trailing words which had no members set. Many places in bitmapset.c had to loop over trailing words to check for empty words. If we ensure we always remove these trailing zero words then we can perform various optimizations such as fast pathing bms_is_subset to return false when 'a' has more words than 'b'. A similar optimization is possible in bms_equal. Both of these together can yield quite significant performance increases in the query planner when querying a partitioned table with around 100 or more partitions. While we're at it, since the minimum number of words a Bitmapset can contain is 1, we can make use of do/while loops instead of for loops when looping over all words in a set. This means checking the loop condition 1 less time, which for single-word sets cuts the loop condition checks in half. Author: David Rowley Reviewed-by: Yuya Watari Discussion: https://postgr.es/m/CAApHDvr5O41MuUjw0DQKqmAnv7QqvmLqXReEd5o4nXTzWp8-+w@mail.gmail.com
* Increase size of bgw_library_name.Nathan Bossart2023-07-03
| | | | | | | | | | | This commit increases the size of the bgw_library_name member of the BackgroundWorker struct from BGW_MAXLEN (96) bytes to MAXPGPATH (default of 1024) bytes so that it can store longer file names (e.g., absolute paths). Author: Yurii Rashkovskii Reviewed-by: Daniel Gustafsson, Aleksander Alekseev Discussion: https://postgr.es/m/CA%2BRLCQyjFV5Y8tG5QgUb6gjteL4S3p%2B1gcyqWTqigyM93WZ9Pg%40mail.gmail.com
* Fix race in SSI interaction with gin fast path.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | | The ginfast.c code previously checked for conflicts in before locking the relevant buffer, leaving a window where a RW conflict could be missed. Re-order. There was also a place where buffer ID and block number were confused while trying to predicate-lock a page, noted by visual inspection. Back-patch to all supported releases. Fixes one more problem discovered with the reproducer from bug #17949, in this case when Dmitry tried other index types. Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com> Reported-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
* Fix race in SSI interaction with bitmap heap scan.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When performing a bitmap heap scan, we don't want to miss concurrent writes that occurred after we observed the heap's rs_nblocks, but before we took predicate locks on index pages. Therefore, we can't skip fetching any heap tuples that are referenced by the index, because we need to test them all with CheckForSerializableConflictOut(). The old optimization that would ignore any references to blocks >= rs_nblocks gets in the way of that requirement, because it means that concurrent writes in that window are ignored. Removing that optimization shouldn't affect correctness at any isolation level, because any new tuples shouldn't be visible to an MVCC snapshot. There also shouldn't be any error-causing references to heap blocks past the end, because we should have held at least an AccessShareLock on the table before the index scan. It can't get smaller while our transaction is running. For now, though, we'll keep the optimization at lower levels to avoid making unnecessary changes in a bug fix. Back-patch to all supported releases. In release 11, the code is in a different place but not fundamentally different. Fixes one aspect of bug #17949. Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
* Fix race in SSI interaction with empty btrees.Thomas Munro2023-07-04
| | | | | | | | | | | | | | | | | When predicate-locking btrees, we have a special case for completely empty btrees, since there is no page to lock. This was racy, because, without buffer lock held, a matching key could be inserted between the _bt_search() and the PredicateLockRelation() calls. Fix, by rechecking _bt_search() after taking the relation-level SIREAD lock, if using SERIALIZABLE isolation and an empty btree is discovered. Back-patch to all supported releases. Fixes one aspect of bug #17949. Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org
* Don't truncate database and user names in startup packets.Nathan Bossart2023-07-03
| | | | | | | | | | | | | | | | | | | | Unlike commands such as CREATE DATABASE, ProcessStartupPacket() does not perform multibyte-aware truncation of overlength names. This means that connection attempts might fail even if the user provides the same overlength names that were used in CREATE DATABASE, CREATE ROLE, etc. Ideally, we'd do the same multibyte- aware truncation in both code paths, but it doesn't seem worth the added complexity of trying to discover the encoding of the names. Instead, let's simply skip truncating the names in the startup packet and let the user/database lookup fail later on. With this change, users must provide the exact names stored in the catalogs, even if the names were truncated. This reverts commit d18c1d1f51. Author: Bertrand Drouvot Reviewed-by: Kyotaro Horiguchi, Tom Lane Discussion: https://postgr.es/m/07436793-1426-29b2-f924-db7422a05fb7%40gmail.com
* Consider fillfactor when estimating relation sizeTomas Vondra2023-07-03
| | | | | | | | | | | | | | | | | When table_block_relation_estimate_size() estimated the number of tuples in a relation without statistics (e.g. right after load), it did not consider fillfactor when calculating density. With non-default fillfactor values, this may result in significant overestimate of the number of tuples - up to 10x with the minimum 10% fillfactor. This may have unexpected consequences, e.g. when creating hash indexes. This considers the current fillfactor value in the "no statistics" code path. If the fillfactor changes after loading data into the table, the estimate may be off. But that seems much less likely than changing the fillfactor before the data load. Reviewed-by: Corey Huinker, Peter Eisentraut Discussion: https://postgr.es/m/cf154ef9-6bac-d268-b735-67a3443debba@enterprisedb.com
* Fix code indentation violationsTomas Vondra2023-07-03
| | | | | | | Commits ce5aaea8cd, 2b8b2852bb and 28d03feac3 violated the expected code indentation rules, upsetting the new buildfarm member "koel." Discussion: https://postgr.es/m/ZKIU4mhWpgJOM0W0%40paquier.xyz
* A minor simplification for List manipulationPeter Eisentraut2023-07-03
| | | | | | | | | Fix one place that was using lfirst(list_head(list)) by using linitial(list) instead. They are equivalent but the latter is simpler. We did the same in 9d299a49. Author: Richard Guo <guofenglinux@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAMbWs49dJnpezDQDDxCPKq7+=_3NyqLqGqnhqCjd+dYe4MS15w@mail.gmail.com
* Take pg_attribute out of VacAttrStatsPeter Eisentraut2023-07-03
| | | | | | | | | | | | | The VacAttrStats structure contained the whole Form_pg_attribute for a column, but it actually only needs attstattarget from there. So remove the Form_pg_attribute field and make a separate field for attstattarget. This simplifies some code for extended statistics that doesn't deal with a column but an expression, which had to fake up pg_attribute rows to satisfy internal APIs. Also, we can remove some comments that essentially said "don't look at pg_attribute directly". Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org
* Add macro for maximum statistics targetPeter Eisentraut2023-07-03
| | | | | | | | The number of places where 10000 was hardcoded had grown a bit beyond the comfort level. Introduce a macro MAX_STATISTICS_TARGET instead. Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org
* Change type of pg_statistic_ext.stxstattargetPeter Eisentraut2023-07-03
| | | | | | | | Change from int32 to int16, to match attstattarget (changed in 90189eefc1). Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org
* Remove support for OpenSSL 1.0.1Michael Paquier2023-07-03
| | | | | | | | | | | | | | Here are some notes about this change: - As X509_get_signature_nid() should always exist (OpenSSL and LibreSSL), hence HAVE_X509_GET_SIGNATURE_NID is now gone. - OPENSSL_API_COMPAT is bumped to 0x10002000L. - One comment related to 1.0.1e introduced by 74242c2 is removed. Upstream OpenSSL still provides long-term support for 1.0.2 in a closed fashion, so removing it is out of scope for a few years, at least. Reviewed-by: Jacob Champion, Daniel Gustafsson Discussion: https://postgr.es/m/ZG3JNursG69dz1lr@paquier.xyz
* Refactor some code related to wait events "BufferPin" and "Extension"Michael Paquier2023-07-03
| | | | | | | | | | | | | | | | | | The following changes are done: - Addition of WaitEventBufferPin and WaitEventExtension, that hold a list of wait events related to each category. - Addition of two functions that encapsulate the list of wait events for each category. - Rename BUFFER_PIN to BUFFERPIN (only this wait event class used an underscore, requiring a specific rule in the automation script). These changes make a bit easier the automatic generation of all the code and documentation related to wait events, as all the wait event categories are now controlled by consistent structures and functions. Author: Bertrand Drouvot Discussion: https://postgr.es/m/c6f35117-4b20-4c78-1df5-d3056010dcf5@gmail.com Discussion: https://postgr.es/m/77a86b3a-c4a8-5f5d-69b9-d70bbf2e9b98@gmail.com
* Remove redundant PARTITION BY columns from WindowClausesDavid Rowley2023-07-03
| | | | | | | | | | | | | | | | | | | Here we adjust the query planner to have it remove items from a window clause's PARTITION BY clause in cases where the pathkey for a column in the PARTITION BY clause is redundant. Doing this allows the optimization added in 9d9c02ccd to stop window aggregation early rather than going into "pass-through" mode to find tuples belonging to the next partition. Also, when we manage to remove all PARTITION BY columns, we now no longer needlessly check that the current tuple belongs to the same partition as the last tuple in nodeWindowAgg.c. If the pathkey was redundant then all tuples must contain the same value for the given redundant column, so there's no point in checking that during execution. Author: David Rowley Reviewed-by: Richard Guo Discussion: https://postgr.es/m/CAApHDvo2ji+hdxrxfXtRtsfSVw3to2o1nCO20qimw0dUGK8hcQ@mail.gmail.com
* Silence "missing contrecord" error.Thomas Munro2023-07-03
| | | | | | | | | | | | | | | | | Commit dd38ff28ad added a new error message "missing contrecord" when we fail to reassemble a record. Unfortunately that caused noisy messages to be logged by pg_waldump at end of segment, and by walsender when asked to shut down on a segment boundary. Remove the new error message, so that this condition signals end-of- WAL without a message. It's arguably a reportable condition that should not be silenced while performing crash recovery, but fixing that without introducing noise in the other cases will require more research. Back-patch to 15. Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Discussion: https://postgr.es/m/6a1df56e-4656-b3ce-4b7a-a9cb41df8189%40enterprisedb.com
* Fix oversight in handling of modifiedCols since f24523672dTomas Vondra2023-07-02
| | | | | | | | | | | | | | | | | | | Commit f24523672d fixed a memory leak by moving the modifiedCols bitmap into the per-row memory context. In the case of AFTER UPDATE triggers, the bitmap is however referenced from an event kept until the end of the query, resulting in a use-after-free bug. Fixed by copying the bitmap into the AfterTriggerEvents memory context, which is the one where we keep the trigger events. There's only one place that needs to do the copy, but the memory context may not exist yet. Doing that in a separate function seems more readable. Report by Alexander Pyhalov, fix by me. Backpatch to 13, where the bitmap was added to the event by commit 71d60e2aa0. Reported-by: Alexander Pyhalov Backpatch-through: 13 Discussion: https://postgr.es/m/acddb17c89b0d6cb940eaeda18c08bbe@postgrespro.ru
* Fix memory leak in Incremental Sort rescansTomas Vondra2023-07-02
| | | | | | | | | | | | | | | | | | | | | | | | The Incremental Sort had a couple issues, resulting in leaking memory during rescans, possibly triggering OOM. The code had a couple of related flaws: 1. During rescans, the sort states were reset but then also set to NULL (despite the comment saying otherwise). ExecIncrementalSort then sees NULL and initializes a new sort state, leaking the memory used by the old one. 2. Initializing the sort state also automatically rebuilt the info about presorted keys, leaking the already initialized info. presorted_keys was also unnecessarily reset to NULL. Patch by James Coleman, based on patches by Laurenz Albe and Tom Lane. Backpatch to 13, where Incremental Sort was introduced. Author: James Coleman, Laurenz Albe, Tom Lane Reported-by: Laurenz Albe, Zu-Ming Jiang Backpatch-through: 13 Discussion: https://postgr.es/m/b2bd02dff61af15e3526293e2771f874cf2a3be7.camel%40cybertec.at Discussion: https://postgr.es/m/db03c582-086d-e7cd-d4a1-3bc722f81765%40inf.ethz.ch
* Improve BRIN minmax-multi opclass test coverageTomas Vondra2023-07-02
| | | | | | | | | | | | | | | | | | | | | | Per the code coverage report, the existing regression tests did not exercice some a couple important BRIN minmax-multi code paths. - The tests focused on testing planning with a range of scan key strategies, but not the execution. Fixed by adding queries that actually test query execution for both equality and inequality. - All tests created indexes after inserting data, but this only exercises the CREATE INDEX strategy that sees all values at once, not incremental summary updates. The new tests flip the order and create the index before adding data. - The assert check(s) validating correctness of expanded ranges were present only in the "union" code path, which is not covered by regression tests at all (as it requires concurrency etc.). Fixed by adding the asserts to a couple more places. Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/57020b2e-d9c9-9bc7-4892-b36d9bb07563%40enterprisedb.com
* Introduce bloom_filter_size for BRIN bloom opclassTomas Vondra2023-07-02
| | | | | | | | | Move the calculation of Bloom filter parameters (for BRIN indexes) into a separate function to make reuse easier. At the moment we only call it from one place, but that may change and it's easier to read anyway. Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/0e1f3350-c9cf-ab62-43a5-5dae314de89c%40enterprisedb.com
* Minor cleanups in the BRIN codeTomas Vondra2023-07-02
| | | | | | | | | | | | BRIN bloom and minmax-multi opclasses were somewhat inconsistent when dealing with bool variables, assigning to them Datum values etc. While not a bug, it makes the code harder to understand, so fix that. While at it, update an incorrect comment copied to bloom opclass from minmax, talking about strategies not supported by bloom. Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/0e1f3350-c9cf-ab62-43a5-5dae314de89c%40enterprisedb.com
* Trust signalfd on illumos, again.Thomas Munro2023-07-02
| | | | | | | | | | | | | | | | | Commit 3ab4fc5d avoided choosing signalfd by default on illumos, because it triggered kernel panics. That was fixed, so we can remove a kludge from our code. Users/packagers can still override the default choice at compile time if desired, and we'll leave the back-branches unchanged so they keep choosing self-pipe by default, but we'll default to signalfd (like we do for Linux) in 17. Fixed kernels should be everywhere by the time 17 ships. The illumos issues were: * https://www.illumos.org/issues/13700 * https://www.illumos.org/issues/14892 Discussion: https://postgr.es/m/CA+hUKG+NK-K_G_i1H3OpDTwYPEsiwQi_jw58PGcW2H+-N2eVCA@mail.gmail.com
* Remove redundant check for fast_forward.Heikki Linnakangas2023-06-30
| | | | | | | We already checked for it earlier in the function. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/1ba2899e-77f8-7866-79e5-f3b7d1251a3e@iki.fi
* Improve comment on why we need ctid->(cmin,cmax) mapping.Heikki Linnakangas2023-06-30
| | | | | | | Combocids are only part of the problem. Explain the problem in more detail. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/1ba2899e-77f8-7866-79e5-f3b7d1251a3e@iki.fi