Message-ID: <e3a78e44-ec6c-5924-7832-63de3aad3e9b@gmail.com>
Date: Wed, 20 Mar 2024 19:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [BUG] false positive in bt_index_check in case of short 4B
 varlena datum
Content-Language: en-US
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: "Andrey M. Borodin" <x4mmm@yandex-team.ru>,
 Michael Zhilin <m.zhilin@postgrespro.ru>, pgsql-bugs@postgresql.org,
 Yura Sokolov <y.sokolov@postgrespro.ru>
References: <7bdbe559-d61a-4ae4-a6e1-48abdf3024cc@postgrespro.ru>
 <0FDE2089-D306-4CBB-AD1F-EC4B419E3B33@yandex-team.ru>
 <e6d20179-ddbd-3376-a7f5-4831df7d6b60@gmail.com>
 <49BBA065-4C9A-4E50-9048-B457907FF219@yandex-team.ru>
 <76bc0dc9-4e43-4cd8-8eec-249b254ed1c9@postgrespro.ru>
 <8C83FCCA-2548-499A-8B1C-96C3D8ADB787@yandex-team.ru>
 <c6f376bf-9896-111f-ccb1-a19dc1381ad0@gmail.com>
 <C8DD7CB5-CFE8-4AF5-88D7-B9F441BB5A8C@yandex-team.ru>
 <0b535249-a00c-a38a-85f6-d5a38c62dd55@gmail.com>
 <CAPpHfdsAs-B7O2_=jGbF+BQzuW3kGSboY9CcxzEOWUgxxZCr5Q@mail.gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <CAPpHfdsAs-B7O2_=jGbF+BQzuW3kGSboY9CcxzEOWUgxxZCr5Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/e3a78e44-ec6c-5924-7832-63de3aad3e9b%40gmail.com>
Precedence: bulk

Hi Alexander,

20.03.2024 13:24, Alexander Korotkov wrote:
> BTW, 0001 and 0002 look good to me. I'm going to push them if no objections. 

Maybe these patches should be polished before committing:
+-- directory paths are passed to us in environment variables
looks like an irrelevant change (perhaps it was relevant in v1/v2, but
that's not so now.)

I'm also not sure about:
+-- BUG: must support different header size of short varlena datum
+-- https://postgr.es/m/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc@postgrespro.ru

AFAICS, for most similar bug fixes, the bug report referenced in a commit
message only (there is no such comment in 0002, either). I also suspect
that the comment:
* Also tuple had short varlena datums with 4B header. ...
might looks incorrect for native English speakers.

This patch also adds a couple of empty lines, which may be not needed.
@@ -2973,6 +2973,7 @@ bt_normalize_tuple(BtreeCheckState *state, IndexTuple itup)
          * index without further processing, so an external varlena header
          * should never be encountered here
          */
 > +
         if (VARATT_IS_EXTERNAL(DatumGetPointer(normalized[i])))
             ereport(ERROR,
                     (errcode(ERRCODE_INDEX_CORRUPTED),
---
         }
+       /*
...
+           need_free[i] = true;
+       }
 > +
     }

Best regards,
Alexander