public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3] Allow HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED combination. 2+ messages / 2 participants [nested] [flat]
* [PATCH v3] Allow HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED combination. @ 2021-01-21 16:06 Julien Rouhaud <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Julien Rouhaud @ 2021-01-21 16:06 UTC (permalink / raw) This combination of hint bits was previously detected as a form of corruption, but it can be obtained with some mix of SELECT ... FOR UPDATE and UPDATE queries. Discussion: https://postgr.es/m/20210124061758.GA11756@nol --- contrib/amcheck/t/001_verify_heapam.pl | 14 +++++++++++++- contrib/amcheck/verify_heapam.c | 7 ------- src/backend/access/heap/README.tuplock | 7 ++++--- src/backend/access/heap/hio.c | 8 +++----- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/contrib/amcheck/t/001_verify_heapam.pl b/contrib/amcheck/t/001_verify_heapam.pl index a2f65b826d..6050feb712 100644 --- a/contrib/amcheck/t/001_verify_heapam.pl +++ b/contrib/amcheck/t/001_verify_heapam.pl @@ -4,7 +4,7 @@ use warnings; use PostgresNode; use TestLib; -use Test::More tests => 79; +use Test::More tests => 80; my ($node, $result); @@ -47,6 +47,9 @@ detects_heap_corruption( # fresh_test_table('test'); $node->safe_psql('postgres', q(VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) test)); +detects_no_corruption( + "verify_heapam('test')", + "all-frozen not corrupted table"); corrupt_first_page('test'); detects_heap_corruption("verify_heapam('test')", "all-frozen corrupted table"); @@ -92,6 +95,15 @@ sub fresh_test_table ALTER TABLE $relname ALTER b SET STORAGE external; INSERT INTO $relname (a, b) (SELECT gs, repeat('b',gs*10) FROM generate_series(1,1000) gs); + BEGIN; + SAVEPOINT s1; + SELECT 1 FROM $relname WHERE a = 42 FOR UPDATE; + UPDATE $relname SET b = b WHERE a = 42; + RELEASE s1; + SAVEPOINT s1; + SELECT 1 FROM $relname WHERE a = 42 FOR UPDATE; + UPDATE $relname SET b = b WHERE a = 42; + COMMIT; )); } diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 88ab32490c..49f5ca0ef2 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -608,13 +608,6 @@ check_tuple_header_and_visibilty(HeapTupleHeader tuphdr, HeapCheckContext *ctx) ctx->tuphdr->t_hoff, ctx->lp_len)); header_garbled = true; } - if ((ctx->tuphdr->t_infomask & HEAP_XMAX_LOCK_ONLY) && - (ctx->tuphdr->t_infomask2 & HEAP_KEYS_UPDATED)) - { - report_corruption(ctx, - pstrdup("tuple is marked as only locked, but also claims key columns were updated")); - header_garbled = true; - } if ((ctx->tuphdr->t_infomask & HEAP_XMAX_COMMITTED) && (ctx->tuphdr->t_infomask & HEAP_XMAX_IS_MULTI)) diff --git a/src/backend/access/heap/README.tuplock b/src/backend/access/heap/README.tuplock index d03ddf6cdc..6441e8baf0 100644 --- a/src/backend/access/heap/README.tuplock +++ b/src/backend/access/heap/README.tuplock @@ -146,9 +146,10 @@ The following infomask bits are applicable: FOR UPDATE; this is implemented by the HEAP_KEYS_UPDATED bit. - HEAP_KEYS_UPDATED - This bit lives in t_infomask2. If set, indicates that the XMAX updated - this tuple and changed the key values, or it deleted the tuple. - It's set regardless of whether the XMAX is a TransactionId or a MultiXactId. + This bit lives in t_infomask2. If set, indicates that the operation(s) done + by the XMAX compromise the tuple key, such as a SELECT FOR UPDATE, an UPDATE + that modifies the columns of the key, or a DELETE. It's set regardless of + whether the XMAX is a TransactionId or a MultiXactId. We currently never set the HEAP_XMAX_COMMITTED when the HEAP_XMAX_IS_MULTI bit is set. diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c index fb7ad0bab4..75223c9bea 100644 --- a/src/backend/access/heap/hio.c +++ b/src/backend/access/heap/hio.c @@ -49,12 +49,10 @@ RelationPutHeapTuple(Relation relation, /* * Do not allow tuples with invalid combinations of hint bits to be placed - * on a page. These combinations are detected as corruption by the - * contrib/amcheck logic, so if you disable one or both of these - * assertions, make corresponding changes there. + * on a page. This combination is detected as corruption by the + * contrib/amcheck logic, so if you disable this assertion, make + * corresponding changes there. */ - Assert(!((tuple->t_data->t_infomask & HEAP_XMAX_LOCK_ONLY) && - (tuple->t_data->t_infomask2 & HEAP_KEYS_UPDATED))); Assert(!((tuple->t_data->t_infomask & HEAP_XMAX_COMMITTED) && (tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI))); -- 2.20.1 --0ntfKIWw70PvrIHh-- ^ permalink raw reply [nested|flat] 2+ messages in thread
* Fix outdated code comments in nodeAgg.c @ 2025-02-09 15:14 Zhang Mingli <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Zhang Mingli @ 2025-02-09 15:14 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> Hi, By reading the codes in nodeAgg.c, I found that some comments are outdated. The comments referred to the function lookup_hash_entry(), which has been removed in commit b563594. I've adjusted the comments to refer to the correct function to make it clear. -- Zhang Mingli HashData Attachments: [application/octet-stream] v0-0001-fix-update-outdated-comments-in-nodeAgg.c.patch (2.3K, ../../198a8d1e-0792-4e7f-828e-902aa342f36e@Spark/3-v0-0001-fix-update-outdated-comments-in-nodeAgg.c.patch) download | inline diff: From 892252cfb311d6fec8520136d6f4739b0645be85 Mon Sep 17 00:00:00 2001 From: Zhang Mingli <[email protected]> Date: Sun, 9 Feb 2025 23:08:44 +0800 Subject: [PATCH] fix: update outdated comments in nodeAgg.c Replaced references to the removed function lookup_hash_entry() in comments with the appropriate current function. Authored-by: Zhang Mingli [email protected] --- src/backend/executor/nodeAgg.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 3005b5c0e3..aa8de946ed 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1453,7 +1453,7 @@ find_cols_walker(Node *node, FindColsContext *context) * To implement hashed aggregation, we need a hashtable that stores a * representative tuple and an array of AggStatePerGroup structs for each * distinct set of GROUP BY column values. We compute the hash key from the - * GROUP BY columns. The per-group data is allocated in lookup_hash_entry(), + * GROUP BY columns. The per-group data is allocated in initialize_hash_entry(), * for each entry. * * We have a separate hashtable and associated perhash data structure for each @@ -1542,7 +1542,7 @@ build_hash_table(AggState *aggstate, int setno, long nbuckets) * at all. Only columns of the first two types need to be stored in the * hashtable, and getting rid of the others can make the table entries * significantly smaller. The hashtable only contains the relevant columns, - * and is packed/unpacked in lookup_hash_entry() / agg_retrieve_hash_table() + * and is packed/unpacked in lookup_hash_entries() / agg_retrieve_hash_table() * into the format of the normal input descriptor. * * Additional columns, in addition to the columns grouped by, come from two @@ -2077,8 +2077,6 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, /* * Look up hash entries for the current tuple in all hashed grouping sets. * - * Be aware that lookup_hash_entry can reset the tmpcontext. - * * Some entries may be left NULL if we are in "spill mode". The same tuple * will belong to different groups for each grouping set, so may match a group * already in memory for one set and match a group not in memory for another -- 2.34.1 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2025-02-09 15:14 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-01-21 16:06 [PATCH v3] Allow HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED combination. Julien Rouhaud <[email protected]> 2025-02-09 15:14 Fix outdated code comments in nodeAgg.c Zhang Mingli <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox