On 11/23/22 16:59, Tom Lane wrote:
=?UTF-8?Q?Fr=c3=a9d=c3=a9ric_Yhuel?= <frederic.yh...@dalibo.com> writes:
On 10/24/22 17:26, Frédéric Yhuel wrote:
When studying the weird planner issue reported here [1], I came up with
the attached patch. It reduces the probability of calling
get_actual_variable_range().
This isn't very useful anymore thanks to this patch:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9c6ad5eaa957bdc2132b900a96e0d2ec9264d39c
I hadn't looked at this patch before, but now that I have, I'm inclined
to reject it anyway. It just moves the problem around: now, instead of
possibly doing an unnecessary index probe at the right end, you're
possibly doing an unnecessary index probe at the left end.
Indeed... it seemed to me that both versions would do an unnecessary
index probe at the left end, but I wasn't careful enough :-/
It also
looks quite weird compared to the normal coding of binary search.
That's right.
I wonder if there'd be something to be said for leaving the initial
probe calculation alone and doing this:
else if (probe == sslot.nvalues - 1 && sslot.nvalues > 2)
+ {
+ /* Don't probe the endpoint until we have to. */
+ if (probe > lobound)
+ probe--;
+ else
have_end = get_actual_variable_range(root,
vardata,
sslot.staop,
collation,
NULL,
&sslot.values[probe]);
+ }
On the whole though, it seems like a wart.
Yeah... it's probably wiser not risking introducing a bug, only to save
an index probe in rare cases (and only 100 reads, thanks to 9c6ad5ea).
Thank you for having had a look at it.
Best regards,
Frédéric
