Re: [PATCH] minor optimization for ineq_histogram_selectivity()

On 11/23/22 16:59, Tom Lane wrote:
> =?UTF-8?Q?Fr=c3=a9d=c3=a9ric_Yhuel?= <frederic.yhuel@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