On Wed, Jul 2, 2025 at 12:41 PM Arseniy Mukhin
<[email protected]> wrote:
>
> Hi!
>
> Here is a new version. I added a commit message. I will add it to PG19-2.

Thank you for the patch.

I think the proposed change is reasonable; if we fail to compress all
ItemPointers, it doesn't make sense to try to form a tuple from it.

Here are some review comments:

---
-       compressedList = ginCompressPostingList(newItems, newNPosting,
GinMaxItemSize,
-
                 NULL);
+       compressedList = ginCompressPostingList(newItems, newNPosting,
GinMaxItemSize - GinGetPostingOffset(old),
+
                 &nwritten);

Why does it need to subtract GinGetPostingOffset(old) from the maxsize?

---
        pfree(newItems);
-       if (compressedList)
+       if (nwritten == newNPosting)
        {
                res = GinFormTuple(ginstate, attnum, key, category,
                                                   (char *) compressedList,

SizeOfGinPostingList(compressedList),
                                                   newNPosting,
                                                   false);
-               pfree(compressedList);
        }
+       pfree(compressedList);

I think it would be cleaner if we move 'pfree(newItems)' to before
'pfree(compressedList)'.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com


Reply via email to