pgsql: Fix a few more redundant calls of GetLatestSnapshot()
Fix a few more redundant calls of GetLatestSnapshot() Commit 2367503177 fixed this in RelationFindReplTupleByIndex(), but I missed two other similar cases. Per report from Ranier Vilela. Discussion: https://www.postgresql.org/message-id/caeudqarut1de45wn87f-gb7xmy_hw6x1dfd3sqdhhxp-rmd...@mail.gmail.com Backpatch-through: 13 Branch -- REL_17_STABLE Details --- https://git.postgresql.org/pg/commitdiff/f1ef111a0940fedace997fe740fb221b068cf6ee Modified Files -- src/backend/executor/execReplication.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Fix ARRAY_SUBLINK and ARRAY[] for int2vector and oidvector input
Fix ARRAY_SUBLINK and ARRAY[] for int2vector and oidvector input. If the given input_type yields valid results from both get_element_type and get_array_type, initArrayResultAny believed the former and treated the input as an array type. However this is inconsistent with what get_promoted_array_type does, leading to situations where the output of an ARRAY() subquery is labeled with the wrong type: it's labeled as oidvector[] but is really a 2-D array of OID. That at least results in strange output, and can result in crashes if further processing such as unnest() is applied. AFAIK this is only possible with the int2vector and oidvector types, which are special-cased to be treated mostly as true arrays even though they aren't quite. Fix by switching the logic to match get_promoted_array_type by testing get_array_type not get_element_type, and remove an Assert thereby made pointless. (We need not introduce a symmetrical check for get_element_type in the other if-branch, because initArrayResultArr will check it.) This restores the behavior that existed before bac27394a introduced initArrayResultAny: the output really is int2vector[] or oidvector[]. Comparable confusion exists when an input of an ARRAY[] construct is int2vector or oidvector: transformArrayExpr decides it's dealing with a multidimensional array constructor, and we end up with something that's a multidimensional OID array but is alleged to be of type oidvector. I have not found a crashing case here, but it's easy to demonstrate totally-wrong results. Adjust that code so that what you get is an oidvector[] instead, for consistency with ARRAY() subqueries. (This change also makes these types work like domains-over-arrays in this context, which seems correct.) Bug: #18840 Reported-by: yang lei Author: Tom Lane Discussion: https://postgr.es/m/18840-fbc9505f066e5...@postgresql.org Backpatch-through: 13 Branch -- REL_16_STABLE Details --- https://git.postgresql.org/pg/commitdiff/0405982c7cbab6e16245df848e9d110bd0284fa0 Modified Files -- src/backend/parser/parse_expr.c | 14 +++- src/backend/utils/adt/arrayfuncs.c | 12 ++-- src/test/regress/expected/arrays.out | 126 +++ src/test/regress/sql/arrays.sql | 22 ++ 4 files changed, 166 insertions(+), 8 deletions(-)
pgsql: Doc: remove obsolete comment.
Doc: remove obsolete comment. This para should have been removed by 2f9661311, which made it both false and irrelevant. Noted while looking at SQL function plancache patch. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1c9242b2cdc358b8be9c4e9967823a24a7807525 Modified Files -- src/include/utils/plancache.h | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-)
pgsql: Silence perl critic
Silence perl critic Commit 27bdec06841 uses a loop variable that is not strictly local to the loop. Perlcritic disapproves, and there's really no reason as the variable is not used outside the loop. Per buildfarm animals koel and crake. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/5eabd91a83adae75f53b61857343660919fef4c7 Modified Files -- src/common/unicode/generate-unicode_case_table.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
pgsql: Fix a few more redundant calls of GetLatestSnapshot()
Fix a few more redundant calls of GetLatestSnapshot() Commit 2367503177 fixed this in RelationFindReplTupleByIndex(), but I missed two other similar cases. Per report from Ranier Vilela. Discussion: https://www.postgresql.org/message-id/caeudqarut1de45wn87f-gb7xmy_hw6x1dfd3sqdhhxp-rmd...@mail.gmail.com Backpatch-through: 13 Branch -- REL_15_STABLE Details --- https://git.postgresql.org/pg/commitdiff/d765226cb9d2e1f4a1efcf45f8e729aafc0e0ddf Modified Files -- src/backend/executor/execReplication.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: pgsql: pg_logicalinspect: Fix possible crash when passing a directory p
On 12.03.25 03:08, David Rowley wrote: On Wed, 12 Mar 2025 at 05:57, Masahiko Sawada wrote: contrib/pg_logicalinspect/pg_logicalinspect.c | 55 --- This introduces a new compiler warning for compilers that don't know the ereport(ERROR) does not return. Which compiler is that?
pgsql: Fix ARRAY_SUBLINK and ARRAY[] for int2vector and oidvector input
Fix ARRAY_SUBLINK and ARRAY[] for int2vector and oidvector input. If the given input_type yields valid results from both get_element_type and get_array_type, initArrayResultAny believed the former and treated the input as an array type. However this is inconsistent with what get_promoted_array_type does, leading to situations where the output of an ARRAY() subquery is labeled with the wrong type: it's labeled as oidvector[] but is really a 2-D array of OID. That at least results in strange output, and can result in crashes if further processing such as unnest() is applied. AFAIK this is only possible with the int2vector and oidvector types, which are special-cased to be treated mostly as true arrays even though they aren't quite. Fix by switching the logic to match get_promoted_array_type by testing get_array_type not get_element_type, and remove an Assert thereby made pointless. (We need not introduce a symmetrical check for get_element_type in the other if-branch, because initArrayResultArr will check it.) This restores the behavior that existed before bac27394a introduced initArrayResultAny: the output really is int2vector[] or oidvector[]. Comparable confusion exists when an input of an ARRAY[] construct is int2vector or oidvector: transformArrayExpr decides it's dealing with a multidimensional array constructor, and we end up with something that's a multidimensional OID array but is alleged to be of type oidvector. I have not found a crashing case here, but it's easy to demonstrate totally-wrong results. Adjust that code so that what you get is an oidvector[] instead, for consistency with ARRAY() subqueries. (This change also makes these types work like domains-over-arrays in this context, which seems correct.) Bug: #18840 Reported-by: yang lei Author: Tom Lane Discussion: https://postgr.es/m/18840-fbc9505f066e5...@postgresql.org Backpatch-through: 13 Branch -- REL_14_STABLE Details --- https://git.postgresql.org/pg/commitdiff/f7ae51312f3d5366ab958c43239ca142aeb11a70 Modified Files -- src/backend/parser/parse_expr.c | 14 +++- src/backend/utils/adt/arrayfuncs.c | 12 ++-- src/test/regress/expected/arrays.out | 126 +++ src/test/regress/sql/arrays.sql | 22 ++ 4 files changed, 166 insertions(+), 8 deletions(-)
pgsql: Simplify and generalize PrepareSortSupportFromIndexRel()
Simplify and generalize PrepareSortSupportFromIndexRel() PrepareSortSupportFromIndexRel() was accepting btree strategy numbers purely for the purpose of comparing it later against btree strategies to determine if the sort direction was forward or reverse. Change that. Instead, pass a bool directly, to indicate the same without an unfortunate assumption that a strategy number refers specifically to a btree strategy. (This is similar in spirit to commits 0d2aa4d4937 and c594f1ad2ba.) (This could arguably be simplfied further by having the callers fill in ssup_reverse directly. But this way, it preserves consistency by having all PrepareSortSupport*() variants be responsible for filling in ssup_reverse.) Moreover, remove the hardcoded check against BTREE_AM_OID, and check against amcanorder instead, which is the actual requirement. Co-authored-by: Mark Dilger Discussion: https://www.postgresql.org/message-id/flat/e72eaa49-354d-4c2e-8eb9-255197f55...@enterprisedb.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/a359d3701991d040bf7b66cfa7052546eb923c38 Modified Files -- src/backend/access/nbtree/nbtsort.c| 7 +++ src/backend/utils/sort/sortsupport.c | 15 ++- src/backend/utils/sort/tuplesortvariants.c | 14 ++ src/include/utils/sortsupport.h| 2 +- 4 files changed, 16 insertions(+), 22 deletions(-)
pgsql: Remove table AM callback scan_bitmap_next_block
Remove table AM callback scan_bitmap_next_block After pushing the bitmap iterator into table-AM specific code (as part of making bitmap heap scan use the read stream API in 2b73a8cd33b7), scan_bitmap_next_block() no longer returns the current block number. Since scan_bitmap_next_block() isn't returning any relevant information to bitmap table scan code, it makes more sense to get rid of it. Now, bitmap table scan code only calls table_scan_bitmap_next_tuple(), and the heap AM implementation of scan_bitmap_next_block() is a local helper in heapam_handler.c. Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/flat/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c3953226a07527a1e2f7f410b83e1a7021f42888 Modified Files -- src/backend/access/heap/heapam_handler.c | 372 -- src/backend/access/table/tableamapi.c | 3 - src/backend/executor/nodeBitmapHeapscan.c | 73 ++ src/backend/optimizer/util/plancat.c | 2 +- src/include/access/tableam.h | 90 ++-- 5 files changed, 244 insertions(+), 296 deletions(-)
pgsql: Simplify distance heuristics in read_stream.c.
Simplify distance heuristics in read_stream.c. Make the distance control heuristics simpler and more aggressive in preparation for asynchronous I/O. The v17 version of read_stream.c made a conservative choice to limit the look-ahead distance when streaming sequential blocks, because it couldn't benefit very much from looking ahead further yet. It had a three-behavior model where only random I/O would rapidly increase the look-ahead distance, to support read-ahead advice. Sequential I/O would move it towards the io_combine_limit setting, just enough to build one full-sized synchronous I/O at a time, and then expect kernel read-ahead to avoid I/O stalls. That already left I/O performance on the table with advice-based I/O concurrency, since sequential blocks could be followed by random jumps, eg with the proposed streaming Bitmap Heap Scan patch. It is time to delete the cautious middle option and adjust the distance based on recent I/O needs only, since asynchronous reads will need to be started ahead of time whether random or sequential. It is still limited by io_combine_limit, *_io_concurrency, buffer availability and strategy ring size, as before. Reviewed-by: Andres Freund (earlier version) Tested-by: Melanie Plageman Discussion: https://postgr.es/m/CA%2BhUKGK_%3D4CVmMHvsHjOVrK6t4F%3DLBpFzsrr3R%2BaJYN8kcTfWg%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/799959dc7cf0e2462601bea8d07b6edec3fa0c4f Modified Files -- src/backend/storage/aio/read_stream.c | 68 +-- 1 file changed, 16 insertions(+), 52 deletions(-)
pgsql: BitmapHeapScan uses the read stream API
BitmapHeapScan uses the read stream API Make Bitmap Heap Scan use the read stream API instead of invoking ReadBuffer() for each block indicated by the bitmap. The read stream API handles prefetching, so remove all of the explicit prefetching from bitmap heap scan code. Now, heap table AM implements a read stream callback which uses the bitmap iterator to return the next required block to the read stream code. Tomas Vondra conducted extensive regression testing of this feature. Andres Freund, Thomas Munro, and I analyzed regressions and Thomas Munro patched the read stream API. Author: Melanie Plageman Reviewed-by: Tomas Vondra Tested-by: Tomas Vondra Tested-by: Andres Freund Tested-by: Thomas Munro Tested-by: Nazir Bilal Yavuz Discussion: https://postgr.es/m/flat/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/2b73a8cd33b745c5b8a7f44322f86642519e3a40 Modified Files -- src/backend/access/heap/heapam.c | 80 +++ src/backend/access/heap/heapam_handler.c | 90 src/backend/executor/nodeBitmapHeapscan.c | 341 +- src/include/access/tableam.h | 25 +-- src/include/nodes/execnodes.h | 23 +- 5 files changed, 124 insertions(+), 435 deletions(-)
pgsql: Separate TBM[Shared|Private]Iterator and TBMIterateResult
Separate TBM[Shared|Private]Iterator and TBMIterateResult Remove the TBMIterateResult member from the TBMPrivateIterator and TBMSharedIterator and make tbm_[shared|private_]iterate() take a TBMIterateResult as a parameter. This allows tidbitmap API users to manage multiple TBMIterateResults per scan. This is required for bitmap heap scan to use the read stream API, with which there may be multiple I/Os in flight at once, each one with a TBMIterateResult. Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/d4bb26c9-fe07-439e-ac53-c0e244387e01%40vondra.me Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/944e81bf99db2b5b70b8a389d4f273534da73f74 Modified Files -- src/backend/access/gin/ginget.c | 33 ++- src/backend/access/gin/ginscan.c | 2 +- src/backend/access/heap/heapam_handler.c | 28 + src/backend/executor/nodeBitmapHeapscan.c | 39 ++--- src/backend/nodes/tidbitmap.c | 96 +-- src/include/access/gin_private.h | 7 ++- src/include/nodes/tidbitmap.h | 6 +- 7 files changed, 110 insertions(+), 101 deletions(-)
pgsql: CREATE INDEX: do update index stats if autovacuum=off.
CREATE INDEX: do update index stats if autovacuum=off. This fixes a thinko from commit d611f8b15. The intent was to prevent updating the stats of the pre-existing heap if autovacuum is off, but it also disabled updating the stats of the just-created index. There is AFAICS no good reason to do the latter, since there could not be any pre-existing stats to refrain from overwriting, and the zeroed stats that are there to begin with are very unlikely to be useful. Moreover, the change broke our cross-version upgrade tests again. Author: Tom Lane Discussion: https://postgr.es/m/1116282.1741374...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/29d6808edebb3eced76e2acbbcf453dd693ac6bf Modified Files -- src/backend/catalog/index.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-)
pgsql: Organize and deduplicate statistics import tests.
Organize and deduplicate statistics import tests. Author: Corey Huinker Reported-by: Melanie Plageman Discussion: https://postgr.es/m/CAAKRu_bWEqUfxhODfJ-XbZC75vq=P6DYOKK6biyey=ym1ah...@mail.gmail.com Discussion: https://postgr.es/m/CADkLM=f1n2_Vomq0gKab7xdxDHmJGgn=de48p8fzqop3mrs...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1d33de9d683722bfa14e436e30a8f5b999a4e2e7 Modified Files -- src/test/regress/expected/stats_import.out | 681 +++-- src/test/regress/sql/stats_import.sql | 555 +-- 2 files changed, 454 insertions(+), 782 deletions(-)
pgsql: Optimization for lower(), upper(), casefold() functions.
Optimization for lower(), upper(), casefold() functions. Improve performance and reduce table sizes for case mapping. The main case mapping table stores only 16-bit offsets, which can be used to look up the mapped code point in any of the case tables (fold, lower, upper, or title case). Simple case pairs point to the same offsets. Generate a function in generate-unicode_case_table.pl that consists of a nested branches to test for specific codepoint ranges that determine the offset in the main table. Other approaches were considered, such as representing these ranges as another structure (rather than branches in a generated function), or a different approach such as a radix tree, or perfect hashing. The author implemented and tested these alternatives and settled on the generated branches. Author: Alexander Borisov Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/7cac7e66-9a3b-4e3f-a997-42aa0c401f80%40gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/27bdec06841d1bb004ca7627eac97808b08a7ac7 Modified Files -- src/common/unicode/generate-unicode_case_table.pl | 388 +- src/common/unicode_case.c |80 +- src/include/common/unicode_case_table.h | 16418 3 files changed, 13671 insertions(+), 3215 deletions(-)
pgsql: Fix snapshot used in logical replication index lookup
Fix snapshot used in logical replication index lookup The function calls GetLatestSnapshot() to acquire a fresh snapshot, makes it active, and was meant to pass it to table_tuple_lock(), but instead called GetLatestSnapshot() again to acquire yet another snapshot. It was harmless because the heap AM and all other known table AMs ignore the 'snapshot' argument anyway, but let's be tidy. In the long run, this perhaps should be redesigned so that snapshot was not needed in the first place. The table AM API uses TID + snapshot as the unique identifier for the row version, which is questionable when the row came from an index scan with a Dirty snapshot. You might lock a different row version when you use a different snapshot in the table_tuple_lock() call (a fresh MVCC snapshot) than in the index scan (DirtySnapshot). However, in the heap AM and other AMs where the TID alone identifies the row version, it doesn't matter. So for now, just fix the obvious albeit harmless bug. This has been wrong ever since the table AM API was introduced in commit 5db6df0c01, so backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/83d243d6-ad8d-4307-8b51-2ee5844f6...@iki.fi Backpatch-through: 13 Branch -- REL_14_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2ef048855fdc770302a2c5897c6bbeef8af4ad62 Modified Files -- src/backend/executor/execReplication.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: localbuf: Fix dangerous coding pattern in GetLocalVictimBuffer()
localbuf: Fix dangerous coding pattern in GetLocalVictimBuffer() If PinLocalBuffer() were to modify the buf_state, the buf_state in GetLocalVictimBuffer() would be out of date. Currently that does not happen, as PinLocalBuffer() only modifies the buf_state if adjust_usagecount=true and GetLocalVictimBuffer() passes false. However, it's easy to make this not the case anymore - it cost me a few hours to debug the consequences. The minimal fix would be to just refetch the buf_state after after calling PinLocalBuffer(), but the same danger exists in later parts of the function. Instead, declare buf_state in the narrower scopes and re-read the state in conditional branches. Besides being safer, it also fits well with an upcoming set of cleanup patches that move the contents of the conditional branches in GetLocalVictimBuffer() into helper functions. I "broke" this in 794f2594479. Arguably this should be backpatched, but as the relevant functions are not exported and there is no actual misbehaviour, I chose to not backpatch, at least for now. Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/caakru_b9anbwzes5aaf9wcvcevmgz-1akhsq-clly-p7whz...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/fa6af9b25e4b449e6e006d9b3434315c0e8e4402 Modified Files -- src/backend/storage/buffer/localbuf.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-)
pgsql: localbuf: Introduce InvalidateLocalBuffer()
localbuf: Introduce InvalidateLocalBuffer() Previously, there were three copies of this code, two of them identical. There's no good reason for that. This change is nice on its own, but the main motivation is the AIO patchset, which needs to add extra checks the deduplicated code, which of course is easier if there is only one version. Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/caakru_b9anbwzes5aaf9wcvcevmgz-1akhsq-clly-p7whz...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0762a151b0e018944694ccac07e521adcdf7a06f Modified Files -- src/backend/storage/buffer/localbuf.c | 92 +-- 1 file changed, 44 insertions(+), 48 deletions(-)
pgsql: localbuf: Introduce TerminateLocalBufferIO()
localbuf: Introduce TerminateLocalBufferIO() Previously TerminateLocalBufferIO() was open-coded in multiple places, which doesn't seem like a great idea. While TerminateLocalBufferIO() currently is rather simple, an upcoming patch requires additional code to be added to TerminateLocalBufferIO(), making this modification particularly worthwhile. For some reason FlushRelationBuffers() previously cleared BM_JUST_DIRTIED, even though that's never set for temporary buffers. This is not carried over as part of this change. Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/caakru_b9anbwzes5aaf9wcvcevmgz-1akhsq-clly-p7whz...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/dd6f2618f681e699cb5f2122a3f036beaa89f992 Modified Files -- src/backend/storage/buffer/bufmgr.c | 32 +--- src/backend/storage/buffer/localbuf.c | 29 ++--- src/include/storage/buf_internals.h | 2 ++ 3 files changed, 37 insertions(+), 26 deletions(-)
pgsql: localbuf: Introduce FlushLocalBuffer()
localbuf: Introduce FlushLocalBuffer() Previously we had two paths implementing writing out temporary table buffers. For shared buffers, the logic for that is centralized in FlushBuffer(). Introduce FlushLocalBuffer() to do the same for local buffers. Besides being a nice cleanup on its own, it also makes an upcoming change slightly easier. Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/caakru_b9anbwzes5aaf9wcvcevmgz-1akhsq-clly-p7whz...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4b4d33b9ea9ff6bdc813b5b7b1aa4a6a3a2a2d5c Modified Files -- src/backend/storage/buffer/bufmgr.c | 22 +--- src/backend/storage/buffer/localbuf.c | 64 --- src/include/storage/buf_internals.h | 1 + 3 files changed, 38 insertions(+), 49 deletions(-)
pgsql: localbuf: Introduce StartLocalBufferIO()
localbuf: Introduce StartLocalBufferIO() To initiate IO on a shared buffer we have StartBufferIO(). For temporary table buffers no similar function exists - likely because the code for that currently is very simple due to the lack of concurrency. However, the upcoming AIO support will make it possible to re-encounter a local buffer, while the buffer already is the target of IO. In that case we need to wait for already in-progress IO to complete. This commit makes it easier to add the necessary code, by introducing StartLocalBufferIO(). Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/caakru_b9anbwzes5aaf9wcvcevmgz-1akhsq-clly-p7whz...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/771ba90298e2b79a94678c96f7960274a7896feb Modified Files -- src/backend/storage/buffer/bufmgr.c | 8 ++-- src/backend/storage/buffer/localbuf.c | 36 +++ src/include/storage/buf_internals.h | 1 + 3 files changed, 39 insertions(+), 6 deletions(-)
