summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
4 daysRemove no longer needed casts to PointerPeter Eisentraut
These casts used to be required when Pointer was char *, but now it's void * (commit 1b2bb5077e9), so they are not needed anymore. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/4154950a-47ae-4223-bd01-1235cc50e933%40eisentraut.org
4 daysheaderscheck ccache supportPeter Eisentraut
Currently, headerscheck and cpluspluscheck are very slow, and they defeat use of ccache. This fixes that, and now they are much faster. The problem was that the test files are created in a randomly-named directory (`mktemp -d /tmp/$me.XXXXXX`), and this directory is mentioned on the compiler command line, which is part of the cache key. The solution is to create the test files in the build directory. For example, for src/include/storage/ipc.h, we generate tmp_headerscheck_c/src_include_storage_ipc_h.c (or .cpp) Now ccache works. (And it's also a bit easier to debug everything with this naming.) (The subdirectory is used to keep the cleanup trap simple.) The observed speedup on Cirrus CI for headerscheck plus cpluspluscheck is from about 1min 20s to only 20s. In local use, the speedups are similar. Co-authored-by: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/b49e74d4-3cf9-4d1c-9dce-09f75e55d026%40eisentraut.org
4 daysheaderscheck: Use LLVM_CPPFLAGSPeter Eisentraut
Otherwise, headerscheck will fail if the LLVM headers are in a location not reached by the normal CFLAGS/CPPFLAGS. Discussion: https://www.postgresql.org/message-id/flat/b49e74d4-3cf9-4d1c-9dce-09f75e55d026%40eisentraut.org
4 daysFix incorrect assertion bound in WaitForLSN()Alexander Korotkov
The assertion checking MyProcNumber used MaxBackends as the upper bound, but the procInfos array is allocated with size MaxBackends + NUM_AUXILIARY_PROCS. This inconsistency would cause a false assertion failure if an auxiliary process calls WaitForLSN(). Author: Xuneng Zhou <xunengzhou@gmail.com>
5 daysRename BUFFERPIN wait event class to BUFFERAndres Freund
In an upcoming patch more wait events will be added to the wait event class (for buffer locking), making the current name too specific. Alternatively we could introduce a dedicated wait event class for those, but it seems somewhat confusing to have a BUFFERPIN and a BUFFER wait event class. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff
5 daysAdd pg_atomic_unlocked_write_u64Andres Freund
The 64bit equivalent of pg_atomic_unlocked_write_u32(), to be used in an upcoming patch converting BufferDesc.state into a 64bit atomic. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff
5 daysbufmgr: Turn BUFFER_LOCK_* into an enumAndres Freund
It seems cleaner to use an enum to tie the different values together. It also helps to have a more descriptive type in the argument to various functions. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff
5 daysMake stats_ext test faster under cache-clobbering test conditions.Tom Lane
Commit 1eccb9315 added a test case that will cause a large number of evaluations of a plpgsql function. With -DCLOBBER_CACHE_ALWAYS, that takes an unreasonable amount of time (hours) because the function's cache entries are repeatedly deleted and rebuilt. That doesn't add any useful test coverage --- other test cases already exercise plpgsql well enough --- and it's not part of what this test intended to cover. We can get the same planner coverage, if not more, by making the test directly invoke numeric_lt(). Reported-by: Tomas Vondra <tomas@vondra.me> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/baf1ae02-83bd-4f5d-872a-1d04f11a9073@vondra.me
5 daysAdd test for multixid wraparoundHeikki Linnakangas
Author: Andrey Borodin <amborodin@acm.org> Discussion: https://www.postgresql.org/message-id/7de697df-d74d-47db-9f73-e069b7349c4b@iki.fi
5 daysSet next multixid's offset when creating a new multixidHeikki Linnakangas
With this commit, the next multixid's offset will always be set on the offsets page, by the time that a backend might try to read it, so we no longer need the waiting mechanism with the condition variable. In other words, this eliminates "corner case 2" mentioned in the comments. The waiting mechanism was broken in a few scenarios: - When nextMulti was advanced without WAL-logging the next multixid. For example, if a later multixid was already assigned and WAL-logged before the previous one was WAL-logged, and then the server crashed. In that case the next offset would never be set in the offsets SLRU, and a query trying to read it would get stuck waiting for it. Same thing could happen if pg_resetwal was used to forcibly advance nextMulti. - In hot standby mode, a deadlock could happen where one backend waits for the next multixid assignment record, but WAL replay is not advancing because of a recovery conflict with the waiting backend. The old TAP test used carefully placed injection points to exercise the old waiting code, but now that the waiting code is gone, much of the old test is no longer relevant. Rewrite the test to reproduce the IPC/MultixactCreation hang after crash recovery instead, and to verify that previously recorded multixids stay readable. Backpatch to all supported versions. In back-branches, we still need to be able to read WAL that was generated before this fix, so in the back-branches this includes a hack to initialize the next offsets page when replaying XLOG_MULTIXACT_CREATE_ID for the last multixid on a page. On 'master', bump XLOG_PAGE_MAGIC instead to indicate that the WAL is not compatible. Author: Andrey Borodin <amborodin@acm.org> Reviewed-by: Dmitry Yurichev <dsy.075@yandex.ru> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Reviewed-by: Ivan Bykov <i.bykov@modernsys.ru> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/172e5723-d65f-4eec-b512-14beacb326ce@yandex.ru Backpatch-through: 14
5 daysUse "foo(void)" for definitions of functions with no parameters.Nathan Bossart
Standard practice in PostgreSQL is to use "foo(void)" instead of "foo()", as the latter looks like an "old-style" function declaration. Similar changes were made in commits cdf4b9aff2, 0e72b9d440, 7069dbcc31, f1283ed6cc, 7b66e2c086, e95126cf04, and 9f7c527af3. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/aTBObQPg%2Bps5I7vl%40ip-10-97-1-34.eu-west-3.compute.internal
5 daysPut back alternative-output expected filesÁlvaro Herrera
These were removed in 5dee7a603f66, but that was too optimistic, per buildfarm member prion as reported by Tom Lane. Mea (Álvaro's) culpa. Author: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Discussion: https://postgr.es/m/570630.1764737028@sss.pgh.pa.us
5 daysFix stray references to SubscriptRefPeter Eisentraut
This type never existed. SubscriptingRef was meant instead. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/2eaa45e3-efc5-4d75-b082-f8159f51445f%40eisentraut.org
5 daysChange Pointer to void *Peter Eisentraut
The comment for the Pointer type said 'XXX Pointer arithmetic is done with this, so it can't be void * under "true" ANSI compilers.'. This has been fixed in the previous commit 756a4368932. This now changes the definition of the type from char * to void *, as envisaged by that comment. Extension code that relies on using Pointer for pointer arithmetic will need to make changes similar to commit 756a4368932, but those changes would be backward compatible. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/4154950a-47ae-4223-bd01-1235cc50e933%40eisentraut.org
5 daysDon't rely on pointer arithmetic with Pointer typePeter Eisentraut
The comment for the Pointer type says 'XXX Pointer arithmetic is done with this, so it can't be void * under "true" ANSI compilers.'. This fixes that. Change from Pointer to use char * explicitly where pointer arithmetic is needed. This makes the meaning of the code clearer locally and removes a dependency on the actual definition of the Pointer type. (The definition of the Pointer type is not changed in this commit.) Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/4154950a-47ae-4223-bd01-1235cc50e933%40eisentraut.org
5 daysUse more appropriate DatumGet* functionPeter Eisentraut
Use DatumGetCString() instead of DatumGetPointer() for returning a C string. Right now, they are the same, but that doesn't always have to be so. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/4154950a-47ae-4223-bd01-1235cc50e933%40eisentraut.org
5 daysRemove useless casts to PointerPeter Eisentraut
in arguments of memcpy() and memmove() calls Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/4154950a-47ae-4223-bd01-1235cc50e933%40eisentraut.org
5 daysFix shadow variable warning in subscriptioncmds.c.Amit Kapila
Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Author: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Reviewed-by: Vignesh C <vignesh21@gmail.com> Discussion: https://postgr.es/m/CAHut+PsF8R0Bt4J3c92+T2F0mun0rRfK=-GH+iBv2s-O8ahJJw@mail.gmail.com
6 daysUse LW_SHARED in dsa.c where possible.Nathan Bossart
Both dsa_get_total_size() and dsa_get_total_size_from_handle() take an exclusive lock just to read a variable. This commit reduces the lock level to LW_SHARED in those functions. Reviewed-by: Sami Imseih <samimseih@gmail.com> Discussion: https://postgr.es/m/aS8fMzWs9e8iHxk2%40nathan
6 daysFix amcheck's handling of half-dead B-tree pagesHeikki Linnakangas
amcheck incorrectly reported the following error if there were any half-dead pages in the index: ERROR: mismatch between parent key and child high key in index "amchecktest_id_idx" It's expected that a half-dead page does not have a downlink in the parent level, so skip the test. Reported-by: Konstantin Knizhnik <knizhnik@garret.ru> Reviewed-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Discussion: https://www.postgresql.org/message-id/33e39552-6a2a-46f3-8b34-3f9f8004451f@garret.ru Backpatch-through: 14
6 daysAdd a test for half-dead pages in B-tree indexesHeikki Linnakangas
To increase our test coverage in general, and because I will use this in the next commit to test a bug we currently have in amcheck. Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://www.postgresql.org/message-id/33e39552-6a2a-46f3-8b34-3f9f8004451f@garret.ru
6 daysFix amcheck's handling of incomplete root splits in B-treeHeikki Linnakangas
When the root page is being split, it's normal that root page according to the metapage is not marked BTP_ROOT. Fix bogus error in amcheck about that case. Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://www.postgresql.org/message-id/abd65090-5336-42cc-b768-2bdd66738404@iki.fi Backpatch-through: 14
6 daysAdd a test for incomplete splits in B-tree indexesHeikki Linnakangas
To increase our test coverage in general, and because I will add onto this in the next commit to also test amcheck with incomplete splits. This is copied from the similar test we had for GIN indexes. B-tree's incomplete splits work similarly to GIN's, so with small changes, the same test works for B-tree too. Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://www.postgresql.org/message-id/abd65090-5336-42cc-b768-2bdd66738404@iki.fi
6 daysShow size of DSAs and dshashes in pg_dsm_registry_allocations.Nathan Bossart
Presently, this view reports NULL for the size of DSAs and dshash tables because 1) the current backend might not be attached to them and 2) the registry doesn't save the pointers to the dsa_area or dshash_table in local memory. Also, the view doesn't show partially-initialized entries to avoid ambiguity, since those entries would report a NULL size as well. This commit introduces a function that looks up the size of a DSA given its handle (transiently attaching to the control segment if needed) and teaches pg_dsm_registry_allocations to use it to show the size of successfully-initialized DSA and dshash entries. Furthermore, the view now reports partially-initialized entries with a NULL size. Reviewed-by: Rahila Syed <rahilasyed90@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/aSeEDeznAsHR1_YF%40nathan
6 daysRemove doc and code comments about ON CONFLICT deficienciesÁlvaro Herrera
They have been fixed, so we don't need this text anymore. This reverts commit 8b18ed6dfbb8. Author: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Discussion: https://postgr.es/m/CADzfLwWo+FV9WSeOah9F1r=4haa6eay1hNvYYy_WfziJeK+aLQ@mail.gmail.com
6 daysAvoid use of NOTICE to wait for snapshot invalidationÁlvaro Herrera
This idea (implemented in commits and bc32a12e0db2 and 9e8fa05d3412) of using notices to detect that a session is sleeping was unreliable, so simplify the concurrency controller session to just look at pg_stat_activity for a process sleeping on the injection point we want it to hit. This change allows us to remove a secondary injection point and the alternative expected output files. Reproduced by Alexander Lakhin following a report in buildfarm member skink (which runs the server under valgrind). Author: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Discussion: https://postgr.es/m/3e302c96-cdd2-45ec-af84-03dbcdccde4a@gmail.com
6 daysFix ON CONFLICT with REINDEX CONCURRENTLY and partitionsÁlvaro Herrera
When planning queries with ON CONFLICT on partitioned tables, the indexes to consider as arbiters for each partition are determined based on those found in the parent table. However, it's possible for an index on a partition to be reindexed, and in that case, the auxiliary indexes created on the partition must be considered as arbiters as well; failing to do that may result in spurious "duplicate key" errors given sufficient bad luck. We fix that in this commit by matching every index that doesn't have a parent to each initially-determined arbiter index. Every unparented matching index is considered an additional arbiter index. Closely related to the fixes in bc32a12e0db2 and 2bc7e886fc1b, and for identical reasons, not backpatched (for now) even though it's a longstanding issue. Author: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Discussion: https://postgr.es/m/CANtu0ojXmqjmEzp-=aJSxjsdE76iAsRgHBoK0QtYHimb_mEfsg@mail.gmail.com
6 daysRemove useless casting to same typePeter Eisentraut
This removes some casts where the input already has the same type as the type specified by the cast. Their presence could cause risks of hiding actual type mismatches in the future or silently discarding qualifiers. It also improves readability. Same kind of idea as 7f798aca1d5 and ef8fe693606. (This does not change all such instances, but only those hand-picked by the author.) Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://www.postgresql.org/message-id/flat/aSQy2JawavlVlEB0%40ip-10-97-1-34.eu-west-3.compute.internal
6 daysSimplify hash_xlog_split_allocate_page()Peter Eisentraut
Instead of complicated pointer arithmetic, overlay a uint32 array and just access the array members. That's safe thanks to XLogRecGetBlockData() returning a MAXALIGNed buffer. Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/aSQy2JawavlVlEB0%40ip-10-97-1-34.eu-west-3.compute.internal
6 daysReplace pointer comparisons and assignments to literal zero with NULLPeter Eisentraut
While 0 is technically correct, NULL is the semantically appropriate choice for pointers. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://www.postgresql.org/message-id/aS1AYnZmuRZ8g%2B5G%40ip-10-97-1-34.eu-west-3.compute.internal
6 daysUpdate comment related to C99Peter Eisentraut
One could do more work here to eliminate the Windows difference described in the comment, but that can be a separate project. The purpose of this change is to update comments that might confusingly indicate that C99 is not required. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/170308e6-a7a3-4484-87b2-f960bb564afa%40eisentraut.org
6 daysUpdate some timestamp[tz] functions to use soft-error reportingMichael Paquier
This commit updates two functions that convert "timestamptz" to "timestamp", and vice-versa, to use the soft error reporting rather than a their own logic to do the same. These are now named as follows: - timestamp2timestamptz_safe() - timestamptz2timestamp_safe() These functions were suffixed with "_opt_overflow", previously. This shaves some code, as it is possible to detect how a timestamp[tz] overflowed based on the returned value rather than a custom state. It is optionally possible for the callers of these functions to rely on the error generated internally by these functions, depending on the error context. Similar work has been done in d03668ea0566 and 4246a977bad6. Reviewed-by: Amul Sul <sulamul@gmail.com> Discussion: https://postgr.es/m/aS09YF2GmVXjAxbJ@paquier.xyz
7 daysMake regex "max_chr" depend on encoding, not provider.Jeff Davis
The regex mechanism scans through the first "max_chr" character values to cache character property ranges (isalpha, etc.). For single-byte encodings, there's no sense in scanning beyond UCHAR_MAX; but for UTF-8 it makes sense to cache higher code point values (though not all of them; only up to MAX_SIMPLE_CHR). Prior to 5a38104b36, the logic about how many character values to scan was based on the pg_regex_strategy, which was dependent on the provider. Commit 5a38104b36 preserved that logic exactly, allowing different providers to define the "max_chr". Now, change it to depend only on the encoding and whether ctype_is_c. For this specific calculation, distinguishing between providers creates more complexity than it's worth. Discussion: https://postgr.es/m/450ceb6260cad30d7afdf155d991a9caafee7c0d.camel@j-davis.com Reviewed-by: Chao Li <li.evan.chao@gmail.com>
7 daysChange some callers to use pg_ascii_toupper().Jeff Davis
The input is ASCII anyway, so it's better to be clear that it's not locale-dependent. Discussion: https://postgr.es/m/450ceb6260cad30d7afdf155d991a9caafee7c0d.camel@j-davis.com
7 daysFix ON CONFLICT ON CONSTRAINT during REINDEX CONCURRENTLYÁlvaro Herrera
When REINDEX CONCURRENTLY is processing the index that supports a constraint, there are periods during which multiple indexes match the constraint index's definition. Those must all be included in the set of inferred index for INSERT ON CONFLICT, in order to avoid spurious "duplicate key" errors. To fix, we set things up to match all indexes against attributes, expressions and predicates of the constraint index, then return all indexes that match those, rather than just the one constraint index. This is more onerous than before, where we would just test the named constraint for validity, but it's not more onerous than processing "conventional" inference (where a list of attribute names etc is given). This is closely related to the misbehaviors fixed by bc32a12e0db2, for a different situation. We're not backpatching this one for now either, for the same reasons. Author: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Discussion: https://postgr.es/m/CANtu0ojXmqjmEzp-=aJSxjsdE76iAsRgHBoK0QtYHimb_mEfsg@mail.gmail.com
7 daysFix a strict aliasing violationPeter Eisentraut
This one is almost a textbook example of an aliasing violation, and it is straightforward to fix, so clean it up. (The warning only shows up if you remove the -fno-strict-aliasing option.) Also, move the code after the error checking. Doesn't make a difference technically, but it seems strange to do actions before errors are checked. Reported-by: Tatsuo Ishii <ishii@postgresql.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/20240724.155525.366150353176322967.ishii%40postgresql.org
7 daysMove WAL sequence code into its own fileMichael Paquier
This split exists for most of the other RMGRs, and makes cleaner the separation between the WAL code, the redo code and the record description code (already in its own file) when it comes to the sequence RMGR. The redo and masking routines are moved to a new file, sequence_xlog.c. All the RMGR routines are now located in a new header, sequence_xlog.h. This separation is useful for a different patch related to sequences that I have been working on, where it makes a refactoring of sequence.c easier if its RMGR routines and its core routines are split. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Discussion: https://postgr.es/m/aSfTxIWjiXkTKh1E@paquier.xyz
7 daysSwitch some date/timestamp functions to use the soft error reportingMichael Paquier
This commit changes some functions related to the data types date and timestamp to use the soft error reporting rather than a custom boolean flag called "overflow", used to let the callers of these functions know if an overflow happens. This results in the removal of some boilerplate code, as it is possible to rely on an error context rather than a custom state, with the possibility to use the error generated inside the functions updated here, if necessary. These functions were suffixed with "_opt_overflow". They are now renamed to use "_safe" as suffix. This work is similar to 4246a977bad6. Author: Amul Sul <sulamul@gmail.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAAJ_b95HEmFyzHZfsdPquSHeswcopk8MCG1Q_vn4tVkZ+xxofw@mail.gmail.com
9 daysDon't call simplify_aggref with a NULL PlannerInfoDavid Rowley
42473b3b3 added prosupport infrastructure to allow simplification of Aggrefs during constant-folding. In some cases the context->root that's given to eval_const_expressions_mutator() can be NULL. 42473b3b3 failed to take that into account, which could result in a crash. To fix, add a check and only call simplify_aggref() when the PlannerInfo is set. Author: David Rowley <dgrowleyml@gmail.com> Reported-by: Birler, Altan <altan.birler@tum.de> Discussion: https://postgr.es/m/132d4da23b844d5ab9e352d34096eab5@tum.de
9 daysUpdate obsolete row compare preprocessing comments.Peter Geoghegan
We have some limited ability to detect redundant and contradictory conditions involving an nbtree row comparison key following commits f09816a0 and bd3f59fd: we can do so in simple cases involving IS NULL and IS NOT NULL keys on a row compare key's first column. We can likewise determine that a scan's qual is unsatisfiable given a row compare whose first subkey's arg is NULL. Update obsolete comments that claimed that we merely copied row compares into the output key array "without any editorialization". Also update another _bt_preprocess_keys header comment paragraph: add a parenthetical remark that points out that preprocessing will generate a skip array for the preceding example qual. That will ultimate lead to preprocessing marking the example's lower-order y key required -- which is exactly what the example supposes cannot happen. Keep the original comment, though, since it accurately describes the mechanical rules that determine which keys get marked required in the absence of skip arrays (which can occasionally still matter). This fixes an oversight in commit 92fe23d9, which added the nbtree skip scan optimization. Author: Peter Geoghegan <pg@bowt.ie> Backpatch-through: 18
9 daysAvoid rewriting data-modifying CTEs more than once.Dean Rasheed
Formerly, when updating an auto-updatable view, or a relation with rules, if the original query had any data-modifying CTEs, the rewriter would rewrite those CTEs multiple times as RewriteQuery() recursed into the product queries. In most cases that was harmless, because RewriteQuery() is mostly idempotent. However, if the CTE involved updating an always-generated column, it would trigger an error because any subsequent rewrite would appear to be attempting to assign a non-default value to the always-generated column. This could perhaps be fixed by attempting to make RewriteQuery() fully idempotent, but that looks quite tricky to achieve, and would probably be quite fragile, given that more generated-column-type features might be added in the future. Instead, fix by arranging for RewriteQuery() to rewrite each CTE exactly once (by tracking the number of CTEs already rewritten as it recurses). This has the advantage of being simpler and more efficient, but it does make RewriteQuery() dependent on the order in which rewriteRuleAction() joins the CTE lists from the original query and the rule action, so care must be taken if that is ever changed. Reported-by: Bernice Southey <bernice.southey@gmail.com> Author: Bernice Southey <bernice.southey@gmail.com> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Discussion: https://postgr.es/m/CAEDh4nyD6MSH9bROhsOsuTqGAv_QceU_GDvN9WcHLtZTCYM1kA@mail.gmail.com Backpatch-through: 14
10 daysGenerate translator comments for GUC parameter descriptionsPeter Eisentraut
Automatically generate comments like /* translator: GUC parameter "client_min_messages" short description */ in the generated guc_tables.inc.c. This provides translators more context. Reviewed-by: Pavlo Golub <pavlo.golub@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Stéphane Schildknecht <sas.postgresql@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/1a89b3f0-e588-41ef-b712-aba766143cad%40eisentraut.org
10 daysFix pg_isblank()Peter Eisentraut
There was a pg_isblank() function that claimed to be a replacement for the standard isblank() function, which was thought to be "not very portable yet". We can now assume that it's portable (it's in C99). But pg_isblank() actually diverged from the standard isblank() by also accepting '\r', while the standard one only accepts space and tab. This was added to support parsing pg_hba.conf under Windows. But the hba parsing code now works completely differently and already handles line endings before we get to pg_isblank(). The other user of pg_isblank() is for ident protocol message parsing, which also handles '\r' separately. So this behavior is now obsolete and confusing. To improve clarity, I separated those concerns. The ident parsing now gets its own function that hardcodes the whitespace characters mentioned by the relevant RFC. pg_isblank() is now static in hba.c and is a wrapper around the standard isblank(), with some extra logic to ensure robust treatment of non-ASCII characters. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/170308e6-a7a3-4484-87b2-f960bb564afa%40eisentraut.org
10 daysAdd slotsync_skip_reason column to pg_replication_slots view.Amit Kapila
Introduce a new column, slotsync_skip_reason, in the pg_replication_slots view. This column records the reason why the last slot synchronization was skipped. It is primarily relevant for logical replication slots on standby servers where the 'synced' field is true. The value is NULL when synchronization succeeds. Author: Shlok Kyal <shlok.kyal.oss@gmail.com> Reviewed-by: shveta malik <shveta.malik@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Ashutosh Sharma <ashu.coek88@gmail.com> Reviewed-by: Hou Zhijie <houzj.fnst@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/CAE9k0PkhfKrTEAsGz4DjOhEj1nQ+hbQVfvWUxNacD38ibW3a1g@mail.gmail.com
11 daysFix possibly uninitialized HeapScanDesc.rs_startblockDavid Rowley
The solution used in 0ca3b1697 to determine the Parallel TID Range Scan's start location was to modify the signature of table_block_parallelscan_startblock_init() to allow the startblock to be passed in as a parameter. This allows the scan limits to be adjusted before that function is called so that the limits are picked up when the parallel scan starts. The commit made it so the call to table_block_parallelscan_startblock_init uses the HeapScanDesc's rs_startblock to pass the startblock to the parallel scan. That all works ok for Parallel TID Range scans as the HeapScanDesc rs_startblock gets set by heap_setscanlimits(), but for Parallel Seq Scans, initscan() does not initialize rs_startblock, and that results in passing an uninitialized value to table_block_parallelscan_startblock_init() as noted by the buildfarm member skink, running Valgrind. To fix this issue, make it so initscan() sets the rs_startblock for parallel scans unless we're doing a rescan. This makes it so table_block_parallelscan_startblock_init() will be called with the startblock set to InvalidBlockNumber, and that'll allow the syncscan code to find the correct start location (when enabled). For Parallel TID Range Scans, this InvalidBlockNumber value will be overwritten in the call to heap_setscanlimits(). initscan() is a bit light on documentation on what's meant to get initialized where for parallel scans. From what I can tell, it looks like it just didn't matter prior to 0ca3b1697 that rs_startblock was left uninitialized for parallel scans. To address the light documentation, I've also added some comments to mention that the syncscan location for parallel scans is figured out in table_block_parallelscan_startblock_init. I've also taken the liberty to adjust the if/else if/else code in initscan() to make it clearer which parts apply to parallel scans and which parts are for the serial scans. Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAApHDvqALm+k7FyfdQdCw1yF_8HojvR61YRrNhwRQPE=zSmnQA@mail.gmail.com
11 daysAdd routines for marking buffers dirty efficientlyMichael Paquier
This commit introduces new internal bufmgr routines for marking shared buffers as dirty: * MarkDirtyUnpinnedBuffer() * MarkDirtyRelUnpinnedBuffers() * MarkDirtyAllUnpinnedBuffers() These functions provide an efficient mechanism to respectively mark one buffer, all the buffers of a relation, or the entire shared buffer pool as dirty, something that can be useful to force patterns for the checkpointer. MarkDirtyUnpinnedBufferInternal(), an extra routine, is used by these three, to mark as dirty an unpinned buffer. They are intended as developer tools to manipulate buffer dirtiness in bulk, and will be used in a follow-up commit. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Aidar Imamov <a.imamov@postgrespro.ru> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Joseph Koshakow <koshy44@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Yuhang Qiu <iamqyh@gmail.com> Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com> Discussion: https://postgr.es/m/CAN55FZ0h_YoSqqutxV6DES1RW8ig6wcA8CR9rJk358YRMxZFmw@mail.gmail.com
11 daysAllow indexscans on partial hash indexes with implied quals.Tom Lane
Normally, if a WHERE clause is implied by the predicate of a partial index, we drop that clause from the set of quals used with the index, since it's redundant to test it if we're scanning that index. However, if it's a hash index (or any !amoptionalkey index), this could result in dropping all available quals for the index's first key, preventing us from generating an indexscan. It's fair to question the practical usefulness of this case. Since hash only supports equality quals, the situation could only arise if the index's predicate is "WHERE indexkey = constant", implying that the index contains only one hash value, which would make hash a really poor choice of index type. However, perhaps there are other !amoptionalkey index AMs out there with which such cases are more plausible. To fix, just don't filter the candidate indexquals this way if the index is !amoptionalkey. That's a bit hokey because it may result in testing quals we didn't need to test, but to do it more accurately we'd have to redundantly identify which candidate quals are actually usable with the index, something we don't know at this early stage of planning. Doesn't seem worth the effort. Reported-by: Sergei Glukhov <s.glukhov@postgrespro.ru> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/e200bf38-6b45-446a-83fd-48617211feff@postgrespro.ru Backpatch-through: 14
11 daysFix new test for CATCACHE_FORCE_RELEASE buildsÁlvaro Herrera
Two of the isolation tests introduce by commit bc32a12e0db2 had a problem under CATCACHE_FORCE_RELEASE, as evidenced by buildfarm member prion. An injection point is hit ahead of what the test spec expects, so a session goes to sleep and there's no one there to wait it up. Fix in the simplest possible way, which is to conditionally wake the process up if it's waiting. An alternative output file is necessary to cover both cases. This suggests a couple of possible improvements to the injection points infrastructure: a conditional wakeup (doing nothing if no one is sleeping, as opposed to throwing an error), as well as a way to attach to a point in "deactivated" mode, activated later. Author: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Mihail Nikalayeu <mihailnikalayeu@gmail.com> Discussion: https://postgr.es/m/202511261817.fyixgtt3hqdr@alvherre.pgsql
11 daysUse C11 alignas in pg_atomic_uint64 definitionsPeter Eisentraut
They were already using pg_attribute_aligned. This replaces that with alignas and moves that into the required syntactic position. This ends up making these three atomics implementations appear a bit more consistent, but shouldn't change anything otherwise. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/46f05236-d4d4-4b4e-84d4-faa500f14691%40eisentraut.org
11 daysFix error reporting for SQL/JSON path type mismatchesAmit Langote
transformJsonFuncExpr() used exprType()/exprLocation() on the possibly coerced path expression, which could be NULL when coercion to jsonpath failed, leading to "cache lookup failed for type 0" errors. Preserve the original expression node so that type and location in the "must be of type jsonpath" error are reported correctly. Add regression tests to cover these cases. Reported-by: Jian He <jian.universality@gmail.com> Author: Jian He <jian.universality@gmail.com> Reviewed-by: Kirill Reshke <reshkekirill@gmail.com> Discussion: https://postgr.es/m/CACJufxHunVg81JMuNo8Yvv_hJD0DicgaVN2Wteu8aJbVJPBjZA@mail.gmail.com Backpatch-through: 17