From: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
---|---|
To: | Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> |
Cc: | Peter Geoghegan <pg(at)bowt(dot)ie>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Bug in nbtree optimization to skip > operator comparisons (or < comparisons in backwards scans) |
Date: | 2023-12-26 19:35:37 |
Message-ID: | CAPpHfdv=BwxRJvBEKkPhvJ_0LbWqkiYOLz-v+m8JrmFLgDKdbA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Pavel,
On Mon, Dec 25, 2023 at 8:32 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
> I've reviewed both patches:
> 0001 - is a pure refactoring replacing argument transfer from via struct member to transfer explicitly as a function argument. It's justified by the fact firstPage is localized only to several places. The patch looks simple and good enough.
>
> 0002:
> continuescanPrechecked is semantically much better than previous requiredMatchedByPrecheck which confused me earlier. Thanks!
>
> From the new comments, it looks a little bit hard to understand who does what. Semantics "if caller told" in comments looks more clear to me. Could you especially give attention to the comments:
>
> "If they wouldn't be matched, then the *continuescan flag would be set for the current item and the last item on the page accordingly."
> "If the key is required for the opposite direction scan, we need to know there was already at least one matching item on the page. For those keys."
>
> > Prechecking the value of the continuescan flag for the last item on the
> >+ * page (according to the scan direction).
> Maybe, in this case, it would be more clear like: "...(for backwards scan it will be the first item on a page)"
>
> Otherwise the patch 0002 looks like a good fix for the bug to be pushed.
Thank you for your review. I've revised comments to meet your suggestions.
------
Regards,
Alexander Korotkov
Attachment | Content-Type | Size |
---|---|---|
0001-Remove-BTScanOpaqueData.firstPage-v4.patch | application/octet-stream | 5.4 KB |
0002-Improvements-and-fixes-for-e0b1ee17dc-v4.patch | application/octet-stream | 10.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2023-12-26 19:58:55 | Re: [meson] expose buildtype debug/optimization info to pg_config |
Previous Message | Tom Lane | 2023-12-26 19:19:04 | Re: Statistics Import and Export |