Re: [PATCH] reduce page overlap of GiST indexes built using sorted method

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, "sergei sh(dot)" <sshoulbakov(at)kontur(dot)io>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] reduce page overlap of GiST indexes built using sorted method
Date: 2026-08-10 06:03:50
Message-ID: 0A0E9181-548F-4076-8798-051655BD0884@yandex-team.ru
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the note!

On Sun, Aug 9, 2026 at 10:37 PM Tom Lane wrote:
> On its face, this complaint is correct: f1ea98a79's addition of
> "|| levelstate->current_page != 0" to the while condition means there
> is a path by which levelstate can become set to NULL at the bottom of
> this loop, and then the next while-condition evaluation crashes.

I think the current_page test is necessary and reachable. It handles a
topmost level that has accumulated more than one page but does not have a
parent yet. Flushing it creates the new root.

An empty index does not reach the loop: both parent and current_page are
zero. When flush is called, the split result is nonempty, and its first
partition creates the parent if needed. Thus flush has an implicit
postcondition that levelstate->parent is not NULL. Coverity seems to have
missed that.

We could make this more apparent by moving parent initialization before
the loop over the split result. I don't see a reason for that larger
refactoring in the back branches, though. The attached patch adds an
Assert for the postcondition instead.

Best regards, Andrey Borodin.

PS. I've just saw Alexander Lakhin's message too, but that's a bit bigger
chunk of work. I'll think what can be done about this.

Attachment Content-Type Size
v1-0001-Document-GiST-sorted-build-s-parent-level-invaria.patch application/octet-stream 1.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-08-10 06:05:38 Re: A new C function `get_partition_root`.
Previous Message Michael Paquier 2026-08-10 06:01:45 Re: Use EVP_MAC for HMAC with OpenSSL 3.0 and later