public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andrey Borodin <[email protected]>
To: Andres Freund <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: Michael Zhilin <[email protected]>
Cc: [email protected]
Cc: Yura Sokolov <[email protected]>
Subject: Re: [BUG] false positive in bt_index_check in case of short 4B varlena datum
Date: Sun, 3 May 2026 23:17:19 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <c64tbgvkqu6v5y66lkitiy6dd32ksrwz3nysbclqnyztstx2lj@ymw4bqsf7zpv>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CAPpHfdsAs-B7O2_=jGbF+BQzuW3kGSboY9CcxzEOWUgxxZCr5Q@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtZxhgjZTpaLd--dCxzUOL6tN+aAZPBeTsGKHLkMQAZpw@mail.gmail.com>
	<CAPpHfdu412Z+jh5Oyc1yzWFM0+52h0kSGQKV4=xQ0aT0UeY2BA@mail.gmail.com>
	<CAPpHfdspqdzVdEU2qHMibKn8OwVuLoZZYvKm-m5Ffqy2aMXgGQ@mail.gmail.com>
	<7ckc7oka4bvafkf5bwlqs6ygrhlsbhz25ppozfch7zbuxcx3rf@e4pr4oqenalc>
	<[email protected]>
	<c64tbgvkqu6v5y66lkitiy6dd32ksrwz3nysbclqnyztstx2lj@ymw4bqsf7zpv>



> On 2 May 2026, at 00:41, Andres Freund <[email protected]> wrote:
> 
> This is checking (as you noted) !VARATT_IS_EXTENDED, whereas the
> bt_normalize_tuple() code is checking !VARATT_IS_COMPRESSED.
> 
> VARATT_IS_EXTENDED() will return true for short varlenas (because it's not a
> standard 4 byte uncompressed varlena), whereas VARATT_IS_COMPRESSED() will
> return false for a short varlena (since it's not compressed).
> 
> I didn't find other instanes of similar code that uses !VARATT_IS_COMPRESSED.

As far as I understand 

!VARATT_IS_COMPRESSED(DatumGetPointer(normalized[i])) && !VARATT_IS_SHORT(DatumGetPointer(normalized[i]))

is exactly

!VARATT_IS_EXTENDED(DatumGetPointer(untoasted_values[i]))

Which is what was proposed in v2 patch. But later was changed to !VARATT_IS_COMPRESSED().
As I understood it was done to further strengthen normalization.

So the intent might be that short varatts need normalization in some cases. But we have no tests that show such a case.
I tried to build a problematic storage alternation like [0], but everything works nicely.

So I propose something in a line with attached patch.


Best regards, Andrey Borodin.

[0] https://github.com/postgres/postgres/blob/master/contrib/amcheck/sql/check_btree.sql#L170-L172


Attachments:

  [application/octet-stream] 0001-amcheck-avoid-using-VARSIZE-for-VARATT_IS_SHORT.patch (1.4K, 2-0001-amcheck-avoid-using-VARSIZE-for-VARATT_IS_SHORT.patch)
  download | inline diff:
From 276482c76491b1aa18a729dd637342b374dcd7fa Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Sun, 3 May 2026 23:13:59 +0500
Subject: [PATCH] amcheck: avoid using VARSIZE() for VARATT_IS_SHORT()

---
 contrib/amcheck/verify_nbtree.c | 2 +-
 src/include/varatt.h            | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index b74ab5f7a05..a3394d88a4e 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -2888,7 +2888,7 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup)
 							ItemPointerGetBlockNumber(&(itup->t_tid)),
 							ItemPointerGetOffsetNumber(&(itup->t_tid)),
 							RelationGetRelationName(state->rel))));
-		else if (!VARATT_IS_COMPRESSED(DatumGetPointer(normalized[i])) &&
+		else if (!VARATT_IS_EXTENDED(DatumGetPointer(normalized[i])) &&
 				 VARSIZE(DatumGetPointer(normalized[i])) > TOAST_INDEX_TARGET &&
 				 (att->attstorage == TYPSTORAGE_EXTENDED ||
 				  att->attstorage == TYPSTORAGE_MAIN))
diff --git a/src/include/varatt.h b/src/include/varatt.h
index 000bdf33b92..321342cab67 100644
--- a/src/include/varatt.h
+++ b/src/include/varatt.h
@@ -297,6 +297,7 @@ typedef struct
 static inline Size
 VARSIZE(const void *PTR)
 {
+	Assert(VARATT_IS_4B(PTR));
 	return VARSIZE_4B(PTR);
 }
 
-- 
2.50.1 (Apple Git-155)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [BUG] false positive in bt_index_check in case of short 4B varlena datum
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox