| 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 |
| 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 |