Re: [PATCH] minor optimization for ineq_histogram_selectivity()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Frédéric Yhuel <frederic(dot)yhuel(at)dalibo(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] minor optimization for ineq_histogram_selectivity()
Date: 2022-11-23 15:59:15
Message-ID: 3710862.1669219155@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

=?UTF-8?Q?Fr=c3=a9d=c3=a9ric_Yhuel?= <frederic(dot)yhuel(at)dalibo(dot)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. It also
looks quite weird compared to the normal coding of binary search.

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.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2022-11-23 16:03:22 Re: Prefetch the next tuple's memory during seqscans
Previous Message Tom Lane 2022-11-23 15:32:47 Re: Bug in MERGE's test for tables with rules