Subject: Re: [BUG] false positive in bt_index_check in case of short 4B varlena datum

From: Andrey Borodin <[email-masked]>
Date: Sun, 3 May 2026 23:17:19 +0500

> On 2 May 2026, at 00:41, Andres Freund <[email-masked]> 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

---

From: Alexander Korotkov <[email-masked]>
Date: Sun, 3 May 2026 22:44:32 +0300

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

AFAICS, this is correct.  However, I propose an alternative approach:
use VARSIZE_ANY().  This might be a bit slower, but looks more
intuitive to me.

------
Regards,
Alexander Korotkov
Supabase

---

From: Andrey Borodin <[email-masked]>
Date: Mon, 4 May 2026 10:20:04 +0500

> On 4 May 2026, at 00:44, Alexander Korotkov <[email-masked]> wrote:
> 
Works for me.
However, I'd like to note that (VARSIZE_1B() < TOAST_INDEX_TARGET) is
constantly true for 8Kb+ pages.

---

From: Michael Paquier <[email-masked]>
Date: Sat, 9 May 2026 08:07:06 +0900

> constantly true for 8Kb+ pages.

How much slower?  I cannot imagine that it matters much in this code
path, but you are getting me worried.
--
Michael

---

From: Andrey Borodin <[email-masked]>
Date: Tue, 12 May 2026 14:17:27 +0500

> On 9 May 2026, at 04:07, Michael Paquier <[email-masked]> wrote:
> 

I think there will be no performance difference.

Change proposed by Alexander only prevents use of VARSIZE() against datum
that is VARSIZE_1B. AFAICS no actual behavior would change.

On some occasions we would have to normilize less tuples.

---

From: Alexander Korotkov <[email-masked]>
Date: Tue, 12 May 2026 13:22:59 +0300

> On some occasions we would have to normilize less tuples.

Any objections if I push and backpatch this?

------
Regards,
Alexander Korotkov
Supabase

---

From: Andrey Borodin <[email-masked]>
Date: Thu, 11 Jun 2026 10:57:32 +0300

> On 12 May 2026, at 13:22, Alexander Korotkov <[email-masked]> wrote:
> 

Kind reminder, let's resolve this thread. Thank you!

---

From: Michael Paquier <[email-masked]>
Date: Fri, 12 Jun 2026 10:32:10 +0900

> Kind reminder, let's resolve this thread. Thank you!

Thanks for the reminder.  FWIW, I had this thread on my radar for some
time, but I am refraining as Alexander seems to be already on it.
--
Michael