aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* doc: 1-byte varlena headers can be used for user PLAIN storageBruce Momjian2023-10-31
| | | | | | | | | | | | This also updates some C comments. Reported-by: suchithjn22@gmail.com Discussion: https://postgr.es/m/167336599095.2667301.15497893107226841625@wrigleys.postgresql.org Author: Laurenz Albe (doc patch) Backpatch-through: 11
* Adjust the order of the prechecks in pgrowlocks()David Rowley2023-10-31
| | | | | | | | | | | | | | | | | | 4b8266415 added a precheck to pgrowlocks() to ensure the given object's pg_class.relam is HEAP_TABLE_AM_OID, however, that check was put before another check which was checking if the given object was a partitioned table. Since the pg_class.relam is always InvalidOid for partitioned tables, if pgrowlocks() was called passing a partitioned table, then the "only heap AM is supported" error would be raised instead of the intended error about the given object being a partitioned table. Here we simply move the pg_class.relam check to after the check that verifies that we are in fact working with a normal (non-partitioned) table. Reported-by: jian he Discussion: https://postgr.es/m/CACJufxFaSp_WguFCf0X98951zFVX+dXFnF1mxAb-G3g1HiHOow@mail.gmail.com Backpatch-through: 12, where 4b8266415 was introduced.
* Diagnose !indisvalid in more SQL functions.Noah Misch2023-10-30
| | | | | | | | | | | | | pgstatindex failed with ERRCODE_DATA_CORRUPTED, of the "can't-happen" class XX. The other functions succeeded on an empty index; they might have malfunctioned if the failed index build left torn I/O or other complex state. Report an ERROR in statistics functions pgstatindex, pgstatginindex, pgstathashindex, and pgstattuple. Report DEBUG1 and skip all index I/O in maintenance functions brin_desummarize_range, brin_summarize_new_values, brin_summarize_range, and gin_clean_pending_list. Back-patch to v11 (all supported versions). Discussion: https://postgr.es/m/20231001195309.a3@google.com
* amcheck: Distinguish interrupted page deletion from corruption.Noah Misch2023-10-30
| | | | | | | | | | | | | This prevents false-positive reports about "the first child of leftmost target page is not leftmost of its level", "block %u is not leftmost" and "left link/right link pair". They appeared if amcheck ran before VACUUM cleaned things, after a cluster exited recovery between the first-stage and second-stage WAL records of a deletion. Back-patch to v11 (all supported versions). Reviewed by Peter Geoghegan. Discussion: https://postgr.es/m/20231005025232.c7.nmisch@google.com
* btree_gin: Fix calculation of leftmost interval value.Dean Rasheed2023-10-29
| | | | | | | | | | | | | | | | | | Formerly, the value computed by leftmostvalue_interval() was a long way short of the minimum possible interval value. As a result, an index scan on a GIN index on an interval column with < or <= operators would miss large negative interval values. Fix by setting all fields of the leftmost interval to their minimum values, ensuring that the result is less than any other possible interval. Since this only affects index searches, no index rebuild is necessary. Back-patch to all supported branches. Dean Rasheed, reviewed by Heikki Linnakangas. Discussion: https://postgr.es/m/CAEZATCV80%2BgOfF8ehNUUfaKBZgZMDfCfL-g1HhWGb6kC3rpDfw%40mail.gmail.com
* Fix intra-query memory leak when a SRF returns zero rows.Tom Lane2023-10-28
| | | | | | | | | | | | | | | | | | When looping around after finding that the set-returning function returned zero rows for the current input tuple, ExecProjectSet neglected to reset either of the two memory contexts it's responsible for cleaning out. Typically this wouldn't cause much problem, because once the SRF does return at least one row, the contexts would get reset on the next call. However, if the SRF returns no rows for many input tuples in succession, quite a lot of memory could be transiently consumed. To fix, make sure we reset both contexts while looping around. Per bug #18172 from Sergei Kornilov. Back-patch to all supported branches. Discussion: https://postgr.es/m/18172-9b8c5fc1d676ded3@postgresql.org
* Remove PHOT from our default timezone abbreviations list.Tom Lane2023-10-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Debian recently decided to split out a bunch of "obsolete" timezone names into a new tzdata-legacy package, which isn't installed by default. One of these zone names is Pacific/Enderbury, and that breaks our regression tests (on --with-system-tzdata builds) because our default timezone abbreviations list defines PHOT as Pacific/Enderbury. Pacific/Enderbury got renamed to Pacific/Kanton in tzdata 2021b, so that in distros that still have this entry it's just a symlink to Pacific/Kanton anyway. So one answer would be to redefine PHOT as Pacific/Kanton. However, then things would fail if the installed tzdata predates 2021b, which is recent enough that that seems like a real problem. Instead, let's just remove PHOT from the default list. That seems likely to affect nobody in the real world, because (a) it was an abbreviation that the tzdb crew made up in the first place, with no evidence of real-world usage, and (b) the total human population of the Phoenix Islands is less than two dozen persons, per Wikipedia. If anyone does use this zone abbreviation they can easily put it back via a custom abbreviations file. We'll keep PHOT in the Pacific.txt reference file, but change it to Pacific/Kanton there, as that definition seems more likely to be useful to future readers of that file. Per report from Victor Wagner. Back-patch to all supported branches. Discussion: https://postgr.es/m/20231027152049.4b5c8044@wagner.wagner.home
* Doc: remove misleading info about ecpg's CONNECT/DISCONNECT DEFAULT.Tom Lane2023-10-25
| | | | | | | | | | | | | | | | | | | | | | | | | As far as I can see, ecpg has no notion of a "default" open connection. You can do "CONNECT TO DEFAULT" but that just specifies letting libpq use all its default connection parameters --- the resulting connection is not special subsequently. In particular, SET CONNECTION = DEFAULT and DISCONNECT DEFAULT simply act on a connection named DEFAULT, if you've made one; they do not have special lookup rules. But the documentation of these commands makes it look like they do. Simplest fix, I think, is just to remove the paras suggesting that DEFAULT is special here. Also, SET CONNECTION *does* have one special lookup rule, which is that it recognizes CURRENT as an alias for the currently selected connection. SET CONNECTION = CURRENT is a no-op, so it's pretty useless, but nonetheless it does something different from selecting a connection by name; so we'd better document it. Per report from Sylvain Frandaz. Back-patch to all supported versions. Discussion: https://postgr.es/m/169824721149.1769274.1553568436817652238@wrigleys.postgresql.org
* doc: Fix link to catalog tableDaniel Gustafsson2023-10-25
| | | | | | | | | The link to pg_type was accidentally linking to pg_authid instead. Backpatch to 12 and 11. Author: Maxim Yablokov <m.yablokov@postgrespro.ru> Discussion: https://postgr.es/m/3289f0ff-0925-46c9-b126-7e4ab6831dad@postgrespro.ru Backpatch-through: 11, 12
* doc: Fix some typos and grammarMichael Paquier2023-10-25
| | | | | | | Author: Ekaterina Kiryanova, Elena Indrupskaya, Oleg Sibiryakov, Maxim Yablokov Discussion: https://postgr.es/m/7aad518b-3e6d-47f3-9184-b1d69cb412e7@postgrespro.ru Backpatch-through: 11
* jit: Adjust back-patch of f90b4a84 to 12 and 13.Thomas Munro2023-10-24
| | | | | | | | | | | | | | | While back-patching f90b4a84, I missed that branches before REL_14_STABLE did some (accidental?) type punning in a function parameter, and failed to adjust these two branches accordingly. That didn't seem to cause a problem for newer LLVM versions or non-debug builds, but older debug builds would fail a type cross-check assertion. Fix by supplying the correct function argument type. In REL_14_STABLE the same change was made by commit df99ddc7. Per build farm animal xenodermus, which runs a debug build of LLVM 6 with jit_above_cost=0. Discussion: https://postgr.es/m/CA%2BhUKGLQ38rgZ3bvNHXPRjsWFAg3pa%3Dtnpeq0osa%2B%3DmiFD5jAw%40mail.gmail.com
* Log LLVM library version in configure output.Thomas Munro2023-10-22
| | | | | | | | | | When scanning build farm results, it's useful to be able to see which version is in use. For the Meson build system, this information was already displayed. Back-patch to all supported branches. Discussion: https://postgr.es/m/4022690.1697852728%40sss.pgh.pa.us
* Dodge a compiler bug affecting timetz_zone/timetz_izone.Tom Lane2023-10-20
| | | | | | | | | | | | | This avoids a compiler bug occurring in AIX's xlc, even in pretty late-model revisions. Buildfarm testing has now confirmed that only 64-bit xlc is affected. Although we are contemplating dropping support for xlc in v17, it's still supported in the back branches, so we need this fix. Back-patch of code changes from HEAD commit 19fa97731. (The test cases were already back-patched, in 4a427b82c et al.) Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com
* Doc: update CREATE OPERATOR's statement about => as an operator.Tom Lane2023-10-20
| | | | | | | This doco said that use of => as an operator "is deprecated". It's been fully disallowed since 865f14a2d back in 9.5, but evidently that commit missed updating this statement. Do so now.
* jit: Changes for LLVM 17.Thomas Munro2023-10-19
| | | | | | | | | | | | Changes required by https://llvm.org/docs/NewPassManager.html. Back-patch to 12, leaving the final release of 11 unchanged, consistent with earlier decision not to back-patch LLVM 16 support either. Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BWXznXCyTgCADd%3DHWkP9Qksa6chd7L%3DGCnZo-MBgg9Lg%40mail.gmail.com
* jit: Supply LLVMGlobalGetValueType() for LLVM < 8.Thomas Munro2023-10-19
| | | | | | | | | | | Commit 37d5babb used this C API function while adding support for LLVM 16 and opaque pointers, but it's not available in LLVM 7 and older. Provide it in our own llvmjit_wrap.cpp. It just calls a C++ function that pre-dates LLVM 3.9, our minimum target. Back-patch to 12, like 37d5babb. Discussion: https://postgr.es/m/CA%2BhUKGKnLnJnWrkr%3D4mSGhE5FuTK55FY15uULR7%3Dzzc%3DwX4Nqw%40mail.gmail.com
* jit: Support opaque pointers in LLVM 16.Thomas Munro2023-10-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove use of LLVMGetElementType() and provide the type of all pointers to LLVMBuildXXX() functions when emitting IR, as required by modern LLVM versions[1]. * For LLVM <= 14, we'll still use the old LLVMBuildXXX() functions. * For LLVM == 15, we'll continue to do the same, explicitly opting out of opaque pointer mode. * For LLVM >= 16, we'll use the new LLVMBuildXXX2() functions that take the extra type argument. The difference is hidden behind some new IR emitting wrapper functions l_load(), l_gep(), l_call() etc. The change is mostly mechanical, except that at each site the correct type had to be provided. In some places we needed to do some extra work to get functions types, including some new wrappers for C++ APIs that are not yet exposed by in LLVM's C API, and some new "example" functions in llvmjit_types.c because it's no longer possible to start from the function pointer type and ask for the function type. Back-patch to 12, because it's a little tricker in 11 and we agreed not to put the latest LLVM support into the upcoming final release of 11. [1] https://llvm.org/docs/OpaquePointers.html Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Ronan Dunklau <ronan.dunklau@aiven.io> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA%2BhUKGKNX_%3Df%2B1C4r06WETKTq0G4Z_7q4L4Fxn5WWpMycDj9Fw%40mail.gmail.com
* windows: msvc: Define STDIN/OUT/ERR_FILENO.Nathan Bossart2023-10-17
| | | | | | | | | | | | | | | | | | | | | This commit (c290e79cf0) was originally back-patched to v15. Commit 97550c0711 added a new use of STDERR_FILENO, and it was back-patched all the way to v11, thus breaking MSVC builds for v11 through v14. Since STDERR_FILENO is now needed on older versions, let's back-patch c290e79cf0 down to v11, too. Here follows the original commit message describing the change: Because they are not available we've used _fileno(stdin) in some places, but that doesn't reliably work, because stdin might be closed. This is the prerequisite of the subsequent commit, fixing a failure introduced in 76e38b37a5. Author: Andres Freund <andres@anarazel.de> Reported-By: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers) Discussion: https://postgr.es/m/20231017164517.GA613565%40nathanxps13 Backpatch-through: 11
* Back-patch test cases for timetz_zone/timetz_izone.Tom Lane2023-10-17
| | | | | | | | | | | | | Per code coverage reports, we had zero regression test coverage of these functions. That came back to bite us, as apparently that's allowed us to miss discovering misbehavior of this code with AIX's xlc compiler. Install relevant portions of the test cases added in 97957fdba, 2f0472030, 19fa97731. (Assuming the expected outcome that the xlc problem does appear in back branches, a code fix will follow.) Discussion: https://postgr.es/m/CA+hUKGK=DOC+hE-62FKfZy=Ybt5uLkrg3zCZD-jFykM-iPn8yw@mail.gmail.com
* Avoid calling proc_exit() in processes forked by system().Nathan Bossart2023-10-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The SIGTERM handler for the startup process immediately calls proc_exit() for the duration of the restore_command, i.e., a call to system(). This system() call forks a new process to execute the shell command, and this child process inherits the parent's signal handlers. If both the parent and child processes receive SIGTERM, both will attempt to call proc_exit(). This can end badly. For example, both processes will try to remove themselves from the PGPROC shared array. To fix this problem, this commit adds a check in StartupProcShutdownHandler() to see whether MyProcPid == getpid(). If they match, this is the parent process, and we can proc_exit() like before. If they do not match, this is a child process, and we just emit a message to STDERR (in a signal safe manner) and _exit(), thereby skipping any problematic exit callbacks. This commit also adds checks in proc_exit(), ProcKill(), and AuxiliaryProcKill() that verify they are not being called within such child processes. Suggested-by: Andres Freund Reviewed-by: Thomas Munro, Andres Freund Discussion: https://postgr.es/m/Y9nGDSgIm83FHcad%40paquier.xyz Discussion: https://postgr.es/m/20230223231503.GA743455%40nathanxps13 Backpatch-through: 11
* Ensure we have a snapshot while dropping ON COMMIT DROP temp tables.Tom Lane2023-10-16
| | | | | | | | | | | | | | | | | | | | | | | | Dropping a temp table could entail TOAST table access to clean out toasted catalog entries, such as large pg_constraint.conbin strings for complex CHECK constraints. If we did that via ON COMMIT DROP, we triggered the assertion in init_toast_snapshot(), because there was no provision for setting up a snapshot for the drop actions. Fix that. (I assume here that the adjacent truncation actions for ON COMMIT DELETE ROWS don't have a similar problem: it doesn't seem like nontransactional truncations would need to touch any toasted fields. If that proves wrong, we could refactor a bit to have the same snapshot acquisition cover that too.) The test case added here does not fail before v15, because that assertion was added in 277692220 which was not back-patched. However, the race condition the assertion warns of surely exists further back, so back-patch to all supported branches. Per report from Richard Guo. Discussion: https://postgr.es/m/CAMbWs4-x26=_QxxgdJyNbiCDzvtr2WV5ZDso_v-CukKEe6cBZw@mail.gmail.com
* Try to handle torn reads of pg_control in frontend.Thomas Munro2023-10-16
| | | | | | | | | | | | | | | | | | | | | | | Some of our src/bin tools read the control file without any kind of interlocking against concurrent writes from the server. At least ext4 and ntfs can expose partially modified contents when you do that. For now, we'll try to tolerate this by retrying up to 10 times if the checksum doesn't match, until we get two reads in a row with the same bad checksum. This is not guaranteed to reach the right conclusion, but it seems very likely to. Thanks to Tom Lane for this suggestion. Various ideas for interlocking or atomicity were considered too complicated, unportable or expensive given the lack of field reports, but remain open for future reconsideration. Back-patch as far as 12. It doesn't seem like a good idea to put a heuristic change for a very rare problem into the final release of 11. Reviewed-by: Anton A. Melnikov <aamelnikov@inbox.ru> Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de
* Acquire ControlFileLock in relevant SQL functions.Thomas Munro2023-10-16
| | | | | | | | | | | | | | Commit dc7d70ea added functions that read the control file, but didn't acquire ControlFileLock. With unlucky timing, file systems that have weak interlocking like ext4 and ntfs could expose partially overwritten contents, and the checksum would fail. Back-patch to all supported releases. Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Anton A. Melnikov <aamelnikov@inbox.ru> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20221123014224.xisi44byq3cf5psi%40awork3.anarazel.de
* Don't spuriously report FD_SETSIZE exhaustion on Windows.Noah Misch2023-10-14
| | | | | | | | | | | | | | | | | Starting on 2023-08-03, this intermittently terminated a "pgbench -C" test in CI. It could affect a high-client-count "pgbench" without "-C". While parallel reindexdb and vacuumdb reach the same problematic check, sufficient client count and/or connection turnover is less plausible for them. Given the lack of examples from the buildfarm or from manual builds, reproducing this must entail rare operating system configurations. Also correct the associated error message, which was wrong for non-Windows. Back-patch to v12, where the pgbench check first appeared. While v11 vacuumdb has the problematic check, reaching it with typical vacuumdb usage is implausible. Reviewed by Thomas Munro. Discussion: https://postgr.es/m/CA+hUKG+JwvTNdcyJTriy9BbtzF1veSRQ=9M_ZKFn9_LqE7Kp7Q@mail.gmail.com
* Fix runtime partition pruning for HASH partitioned tablesDavid Rowley2023-10-13
| | | | | | | | | | | | | | | | | | | | | | | This could only affect HASH partitioned tables with at least 2 partition key columns. If partition pruning was delayed until execution and the query contained an IS NULL qual on one of the partitioned keys, and some subsequent partitioned key was being compared to a non-Const, then this could result in a crash due to the incorrect keyno being used to calculate the stateidx for the expression evaluation code. Here we fix this by properly skipping partitioned keys which have a nullkey set. Effectively, this must be the same as what's going on inside perform_pruning_base_step(). Sergei Glukhov also provided a patch, but that's not what's being used here. Reported-by: Sergei Glukhov Reviewed-by: tender wang, Sergei Glukhov Discussion: https://postgr.es/m/d05b26fa-af54-27e1-f693-6c31590802fa@postgrespro.ru Backpatch-through: 11, where runtime partition pruning was added.
* Doc: fix grammatical errors for enable_partitionwise_aggregateDavid Rowley2023-10-12
| | | | | | | Author: Andrew Atkinson Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/CAG6XLEnC%3DEgq0YHRic2kWWDs4xwQnQ_kBA6qhhzAq1-pO_9Tfw%40mail.gmail.com Backpatch-through: 11, where enable_partitionwise_aggregate was added
* Fix incorrect step generation in HASH partition pruningDavid Rowley2023-10-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | get_steps_using_prefix_recurse() incorrectly assumed that it could stop recursive processing of the 'prefix' list when cur_keyno was one before the step_lastkeyno. Since hash partition pruning can prune using IS NULL quals, and these IS NULL quals are not present in the 'prefix' list, then that logic could cause more levels of recursion than what is needed and lead to there being no more items in the 'prefix' list to process. This would manifest itself as a crash in some code that expected the 'start' ListCell not to be NULL. Here we adjust the logic so that instead of stopping recursion at 1 key before the step_lastkeyno, we just look at the llast(prefix) item and ensure we only recursively process up until just before whichever the last key is. This effectively allows keys to be missing in the 'prefix' list. This change does mean that step_lastkeyno is no longer needed, so we remove that from the static functions. I also spent quite some time reading this code and testing it to try to convince myself that there are no other issues. That resulted in the irresistible temptation of rewriting some comments, many of which were just not true or inconcise. Reported-by: Sergei Glukhov Reviewed-by: Sergei Glukhov, tender wang Discussion: https://postgr.es/m/2f09ce72-315e-2a33-589a-8519ada8df61@postgrespro.ru Backpatch-through: 11, where partition pruning was introduced.
* doc: clarify that SSPI and GSSAPI are interchangeableBruce Momjian2023-10-10
| | | | | | | | Reported-by: tpo_deb@sourcepole.ch Discussion: https://postgr.es/m/167846222574.1803490.15815104179136215862@wrigleys.postgresql.org Backpatch-through: 11
* doc: foreign servers with pushdown need matching collationBruce Momjian2023-10-10
| | | | | | | | Reported-by: Pete Storer Discussion: https://postgr.es/m/BL0PR05MB66283C57D72E321591AE4EB1F3CE9@BL0PR05MB6628.namprd05.prod.outlook.com Backpatch-through: 11
* doc: add SSL configuration section referenceBruce Momjian2023-10-10
| | | | | | | | Reported-by: Steve Atkins Discussion: https://postgr.es/m/B82E80DD-1452-4175-B19C-564FE46705BA@blighty.com Backpatch-through: 11
* doc: document the need to analyze partitioned tablesBruce Momjian2023-10-10
| | | | | | | | | | Autovacuum does not do it. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20210913035409.GA10647@telsasoft.com Backpatch-through: 11
* Fix bug in GenericXLogFinish().Jeff Davis2023-10-10
| | | | | | | | Mark the buffers dirty before writing WAL. Discussion: https://postgr.es/m/25104133-7df8-cae3-b9a2-1c0aaa1c094a@iki.fi Reviewed-by: Heikki Linnakangas Backpatch-through: 11
* Doc: use CURRENT_USER not USER in plpgsql trigger examples.Tom Lane2023-10-09
| | | | | | | | | | | | While these two built-in functions do exactly the same thing, CURRENT_USER seems preferable to use in documentation examples. It's easier to look up if the reader is unsure what it is. Also, this puts these examples in sync with an adjacent example that already used CURRENT_USER. Per question from Kirk Parker. Discussion: https://postgr.es/m/CANwZ8rmN_Eb0h0hoMRS8Feftaik0z89PxVsKg+cP+PctuOq=Qg@mail.gmail.com
* Avoid memory size overflow when allocating backend activity bufferMichael Paquier2023-10-03
| | | | | | | | | | | | | | | | | | | | | | The code in charge of copying the contents of PgBackendStatus to local memory could fail on memory allocation because of an overflow on the amount of memory to use. The overflow can happen when combining a high value track_activity_query_size (max at 1MB) with a large max_connections, when both multiplied get higher than INT32_MAX as both parameters treated as signed integers. This could for example trigger with the following functions, all calling pgstat_read_current_status(): - pg_stat_get_backend_subxact() - pg_stat_get_backend_idset() - pg_stat_get_progress_info() - pg_stat_get_activity() - pg_stat_get_db_numbackends() The change to use MemoryContextAllocHuge() has been introduced in 8d0ddccec636, so backpatch down to 12. Author: Jakub Wartak Discussion: https://postgr.es/m/CAKZiRmw8QSNVw2qNK-dznsatQqz+9DkCquxP0GHbbv1jMkGHMA@mail.gmail.com Backpatch-through: 12
* Fail hard on out-of-memory failures in xlogreader.cMichael Paquier2023-10-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit changes the WAL reader routines so as a FATAL for the backend or exit(FAILURE) for the frontend is triggered if an allocation for a WAL record decode fails in walreader.c, rather than treating this case as bogus data, which would be equivalent to the end of WAL. The key is to avoid palloc_extended(MCXT_ALLOC_NO_OOM) in walreader.c, relying on plain palloc() calls. The previous behavior could make WAL replay finish too early than it should. For example, crash recovery finishing earlier may corrupt clusters because not all the WAL available locally was replayed to ensure a consistent state. Out-of-memory failures would show up randomly depending on the memory pressure on the host, but one simple case would be to generate a large record, then replay this record after downsizing a host, as Ethan Mertz originally reported. This relies on bae868caf222, as the WAL reader routines now do the memory allocation required for a record only once its header has been fully read and validated, making xl_tot_len trustable. Making the WAL reader react differently on out-of-memory or bogus record data would require ABI changes, so this is the safest choice for stable branches. Also, it is worth noting that 3f1ce973467a has been using a plain palloc() in this code for some time now. Thanks to Noah Misch and Thomas Munro for the discussion. Like the other commit, backpatch down to 12, leaving out v11 that will be EOL'd soon. The behavior of considering a failed allocation as bogus data comes originally from 0ffe11abd3a0, where the record length retrieved from its header was not entirely trustable. Reported-by: Ethan Mertz Discussion: https://postgr.es/m/ZRKKdI5-RRlta3aF@paquier.xyz Backpatch-through: 12
* Fix omission of column-level privileges in selective pg_restore.Tom Lane2023-10-02
| | | | | | | | | | | | | | | | | | | | | | | | In a selective restore, ACLs for a table should be dumped if the table is selected to be dumped. However, if the table has both table-level and column-level ACLs, only the table-level ACL was restored. This happened because _tocEntryRequired assumed that an ACL could have only one dependency (the one on its table), and punted if there was more than one. But since commit ea9125304, column-level ACLs also depend on the table-level ACL if any, to ensure correct ordering in parallel restores. To fix, adjust the logic in _tocEntryRequired to ignore dependencies on ACLs. I extended a test case in 002_pg_dump.pl so that it purports to test for this; but in fact the test passes even without the fix. That's because this bug only manifests during a selective restore, while the scenarios 002_pg_dump.pl tests include only selective dumps. Perhaps somebody would like to extend the script so that it can test scenarios including selective restore, but I'm not touching that. Euler Taveira and Tom Lane, per report from Kong Man. Back-patch to all supported branches. Discussion: https://postgr.es/m/DM4PR11MB73976902DBBA10B1D652F9498B06A@DM4PR11MB7397.namprd11.prod.outlook.com
* Fix datalen calculation in tsvectorrecv().Tom Lane2023-10-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After receiving position data for a lexeme, tsvectorrecv() advanced its "datalen" value by (npos+1)*sizeof(WordEntry) where the correct calculation is (npos+1)*sizeof(WordEntryPos). This accidentally failed to render the constructed tsvector invalid, but it did result in leaving some wasted space approximately equal to the space consumed by the position data. That could have several bad effects: * Disk space is wasted if the received tsvector is stored into a table as-is. * A legal tsvector could get rejected with "maximum total lexeme length exceeded" if the extra space pushes it over the MAXSTRPOS limit. * In edge cases, the finished tsvector could be assigned a length larger than the allocated size of its palloc chunk, conceivably leading to SIGSEGV when the tsvector gets copied somewhere else. The odds of a field failure of this sort seem low, though valgrind testing could probably have found this. While we're here, let's express the calculation as "sizeof(uint16) + npos * sizeof(WordEntryPos)" to avoid the type pun implicit in the "npos + 1" formulation. It's not wrong given that WordEntryPos had better be 2 bytes to avoid padding problems, but it seems clearer this way. Report and patch by Denis Erokhin. Back-patch to all supported versions. Discussion: https://postgr.es/m/009801d9f2d9$f29730c0$d7c59240$@datagile.ru
* Remove environment sensitivity in pl/tcl regression test.Tom Lane2023-09-29
| | | | | | | | | | | | | | | Add "-gmt 1" to our test invocations of the Tcl "clock" command, so that they do not consult the timezone environment. While it doesn't really matter which timezone is used here, it does matter that the command not fall over entirely. We've now discovered that at least on FreeBSD, "clock scan" will fail if /etc/localtime is missing. It seems worth making the test insensitive to that. Per Tomas Vondras' buildfarm animal dikkop. Thanks to Thomas Munro for the diagnosis. Discussion: https://postgr.es/m/316d304a-1dcd-cea1-3d6c-27f794727a06@enterprisedb.com
* Suppress macOS warnings about duplicate libraries in link commands.Tom Lane2023-09-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As of Xcode 15 (macOS Sonoma), the linker complains about duplicate references to the same library. We see warnings about libpgport and libpgcommon being duplicated in many client executables. This is a consequence of the hack introduced in commit 6b7ef076b to list libpgport before libpq while not removing it from $(LIBS). (Commit 8396447cd later applied the same rule to libpgcommon.) The concern in 6b7ef076b was to ensure that the client executable wouldn't unintentionally depend on pgport functions from libpq. That concern is obsolete on any platform for which we can do symbol export control, because if we can then the pgport functions in libpq won't be exposed anyway. Hence, we can fix this problem by just removing libpgport and libpgcommon from $(libpq_pgport), and letting clients depend on the occurrences in $(LIBS). In the back branches, do that only on macOS (which we know has symbol export control). In HEAD, let's be more aggressive and remove the extra libraries everywhere. The only still-supported platforms that lack export control are MinGW/Cygwin, and it doesn't seem worth sweating over ABI stability details for those (or if somebody does care, it'd probably be possible to perform symbol export control for those too). As well as being simpler, this might give some microscopic improvement in build time. The meson build system is not changed here, as it doesn't have this particular disease, though it does have some related issues that we'll fix separately. Discussion: https://postgr.es/m/467042.1695766998@sss.pgh.pa.us
* Doc: improve description of dump/restore's --clean and --if-exists.Tom Lane2023-09-29
| | | | | | | Try to make these option descriptions a little clearer for novices. Per gripe from Attila Gulyás. Discussion: https://postgr.es/m/169590536647.3727336.11070254203649648453@wrigleys.postgresql.org
* doc: Change statistics function xref to the right targetDaniel Gustafsson2023-09-29
| | | | | | | | | | | | | Commit 7d3b7011b added a link to the statistics functions, which at the time were anchored under the section for statistics views. aebe989477a added a separate section for statistics functions, but the link was not updated to point to the new anchor. Fix by changing the xref. Backpatch to all supported branches. Author: Peter Smith <peter.b.smith@fujitsu.com> Discussion: https://postgr.es/m/CAHut+Ptr0jKzNNtWnssLq+3jNhbyaBseqf6NPrWHk08mQFRoTg@mail.gmail.com Backpatch-through: 11
* Fix btmarkpos/btrestrpos array key wraparound bug.Peter Geoghegan2023-09-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | nbtree's mark/restore processing failed to correctly handle an edge case involving array key advancement and related search-type scan key state. Scans with ScalarArrayScalarArrayOpExpr quals requiring mark/restore processing (for a merge join) could incorrectly conclude that an affected array/scan key must not have advanced during the time between marking and restoring the scan's position. As a result of all this, array key handling within btrestrpos could skip a required call to _bt_preprocess_keys(). This confusion allowed later primitive index scans to overlook tuples matching the true current array keys. The scan's search-type scan keys would still have spurious values corresponding to the final array element(s) -- not values matching the first/now-current array element(s). To fix, remember that "array key wraparound" has taken place during the ongoing btrescan in a flag variable stored in the scan's state, and use that information at the point where btrestrpos decides if another call to _bt_preprocess_keys is required. Oversight in commit 70bc5833, which taught nbtree to handle array keys during mark/restore processing, but missed this subtlety. That commit was itself a bug fix for an issue in commit 9e8da0f7, which taught nbtree to handle ScalarArrayOpExpr quals natively. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-WzkgP3DDRJxw6DgjCxo-cu-DKrvjEv_ArkP2ctBJatDCYg@mail.gmail.com Backpatch: 11- (all supported branches).
* Fix checking of index expressions in CompareIndexInfo().Tom Lane2023-09-28
| | | | | | | | | | | | | | | | | | | | | | | This code was sloppy about comparison of index columns that are expressions. It didn't reliably reject cases where one index has an expression where the other has a plain column, and it could index off the start of the attmap array, leading to a Valgrind complaint (though an actual crash seems unlikely). I'm not sure that the expression-vs-column sloppiness leads to any visible problem in practice, because the subsequent comparison of the two expression lists would reject cases where the indexes have different numbers of expressions overall. Maybe we could falsely match indexes having the same expressions in different column positions, but it'd require unlucky contents of the word before the attmap array. It's not too surprising that no problem has been reported from the field. Nonetheless, this code is clearly wrong. Per bug #18135 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18135-532f4a755e71e4d2@postgresql.org
* Stop using "-multiply_defined suppress" on macOS.Tom Lane2023-09-26
| | | | | | | | | We started to use this linker switch in commit 9df308697 of 2004-07-13, which was in the OS X 10.3 era. Apparently it's been a no-op since around OS X 10.9. Apple's most recent toolchain version actively complains about it, so it's time to get rid of it. Discussion: https://postgr.es/m/467042.1695766998@sss.pgh.pa.us
* doc: clarify the effect of concurrent work_mem allocationsBruce Momjian2023-09-26
| | | | | | | | Reported-by: Sami Imseih Discussion: https://postgr.es/m/66590882-F48C-4A25-83E3-73792CF8C51F@amazon.com Backpatch-through: 11
* doc: clarify handling of time zones with "time with time zone"Bruce Momjian2023-09-26
| | | | | | | | Reported-by: davecramer@postgres.rocks Discussion: https://postgr.es/m/168451942371.714.9173574930845904336@wrigleys.postgresql.org Backpatch-through: 11
* doc: clarify the behavior of unopenable listen_addressesBruce Momjian2023-09-26
| | | | | | | | Reported-by: Gurjeet Singh Discussion: https://postgr.es/m/CABwTF4WYPD9ov-kcSq1+J+ZJ5wYDQLXquY6Lu2cvb-Y7pTpSGA@mail.gmail.com Backpatch-through: 11
* doc: pg_upgrade, clarify standby servers must remain runningBruce Momjian2023-09-26
| | | | | | | | | | | Also mention that mismatching primary/standby LSNs should never happen. Reported-by: Nikolay Samokhvalov Discussion: https://postgr.es/m/CAM527d8heqkjG5VrvjU3Xjsqxg41ufUyabD9QZccdAxnpbRH-Q@mail.gmail.com Backpatch-through: 11
* doc: mention GROUP BY columns can reference target col numbersBruce Momjian2023-09-26
| | | | | | | | Reported-by: hape <postgres-hape@gmx.de> Discussion: https://postgr.es/m/168871536004.379168.9352636188330923805@wrigleys.postgresql.org Backpatch-through: 11
* Fix another bug in parent page splitting during GiST index build.Heikki Linnakangas2023-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Yet another bug in the ilk of commits a7ee7c851 and 741b88435. In 741b88435, we took care to clear the memorized location of the downlink when we split the parent page, because splitting the parent page can move the downlink. But we missed that even *updating* a tuple on the parent can move it, because updating a tuple on a gist page is implemented as a delete+insert, so the updated tuple gets moved to the end of the page. This commit fixes the bug in two different ways (belt and suspenders): 1. Clear the downlink when we update a tuple on the parent page, even if it's not split. This the same approach as in commits a7ee7c851 and 741b88435. I also noticed that gistFindCorrectParent did not clear the 'downlinkoffnum' when it stepped to the right sibling. Fix that too, as it seems like a clear bug even though I haven't been able to find a test case to hit that. 2. Change gistFindCorrectParent so that it treats 'downlinkoffnum' merely as a hint. It now always first checks if the downlink is still at that location, and if not, it scans the page like before. That's more robust if there are still more cases where we fail to clear 'downlinkoffnum' that we haven't yet uncovered. With this, it's no longer necessary to meticulously clear 'downlinkoffnum', so this makes the previous fixes unnecessary, but I didn't revert them because it still seems nice to clear it when we know that the downlink has moved. Also add the test case using the same test data that Alexander posted. I tried to reduce it to a smaller test, and I also tried to reproduce this with different test data, but I was not able to, so let's just include what we have. Backpatch to v12, like the previous fixes. Reported-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/18129-caca016eaf0c3702@postgresql.org