Thread: Re: Planner regression in 9.1: min(x) cannot use partial index with NOT NULL

Robert Haas  wrote:
> Tom Lane  wrote:
>> I don't think that suppressing nulls from an index this way is
>> really very useful. Using a partial index probably eats more
>> planner cycles than you'll save, overall.
> 
> If only 1% of the table has non-NULL values in that column, maybe
> not.
We definitely have indexes with less than 1% non-NULL, and we've
found partial indexes to be efficient for them.  On the other hand,
I can't think where we do min/max on any of them; so as long as this
regression only affects those aggregates, it won't hurt our shop.
The use case doesn't seem all that far-fetched to me, though.
-Kevin

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Robert Haas  wrote:
>> Tom Lane  wrote:
>>> I don't think that suppressing nulls from an index this way is
>>> really very useful. Using a partial index probably eats more
>>> planner cycles than you'll save, overall.

>> If only 1% of the table has non-NULL values in that column, maybe
>> not.
> We definitely have indexes with less than 1% non-NULL, and we've
> found partial indexes to be efficient for them.  On the other hand,
> I can't think where we do min/max on any of them; so as long as this
> regression only affects those aggregates, it won't hurt our shop.
> The use case doesn't seem all that far-fetched to me, though.

Hmm.  We could possibly fix this by having planagg.c do a completely
separate planner run for each aggregate, wherein it actually does build
the "equivalent" querySELECT col FROM tab WHERE existing-quals AND col IS NOT NULLORDER BY col ASC/DESC LIMIT 1
and plan that.  That'd be less efficient than the current way,
especially for cases where there are multiple aggregates, because there
would be some duplication of processing between the per-aggregate
planner runs and the main one.  But since we can only do this
optimization for rather simple queries anyway, maybe it wouldn't matter
much.
        regards, tom lane

So it's a clever hack that we used to allow the partial indexes to be
used. It relied on the implicit assumption that min(x) and max(x)
where the only values of x where NULL were both NULL.

It would be nice if we were clever enough to support *any* strict
aggregate using partial indexes on WHERE NOT NULL since they'll all
have that property.

Greg Stark <gsstark@mit.edu> writes:
> So it's a clever hack that we used to allow the partial indexes to be
> used. It relied on the implicit assumption that min(x) and max(x)
> where the only values of x where NULL were both NULL.

> It would be nice if we were clever enough to support *any* strict
> aggregate using partial indexes on WHERE NOT NULL since they'll all
> have that property.

Huh?  The point of the min/max optimization is to not scan the whole
index but just fetch the endpoint value.  For general aggregates, you
have to scan the table anyway.  If an index is useful for that, it'll
get picked up in the normal planning process.
        regards, tom lane

On Mon, Mar 21, 2011 at 5:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Greg Stark <gsstark@mit.edu> writes:
>> So it's a clever hack that we used to allow the partial indexes to be
>> used. It relied on the implicit assumption that min(x) and max(x)
>> where the only values of x where NULL were both NULL.
>
>> It would be nice if we were clever enough to support *any* strict
>> aggregate using partial indexes on WHERE NOT NULL since they'll all
>> have that property.
>
> Huh?  The point of the min/max optimization is to not scan the whole
> index but just fetch the endpoint value.

But in the case where the index has no records it doesn't know whether
there were no records in the table or they were just all NULL. As it
happens min() and max() return NULL in both cases so it doesn't
matter. My point was that this is a clever hack and a non-obvious
deduction the planner is making.


> For general aggregates, you
> have to scan the table anyway.  If an index is useful for that, it'll
> get picked up in the normal planning process.

if I do "SELECT count(col) from tab" with no WHERE clauses on a table
with 1% non-null values in col will the planner correctly find the
partial index? If so why doesn't the min/max planning find it?



--
greg

Greg Stark <gsstark@mit.edu> writes:
> On Mon, Mar 21, 2011 at 5:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> For general aggregates, you
>> have to scan the table anyway. �If an index is useful for that, it'll
>> get picked up in the normal planning process.

> if I do "SELECT count(col) from tab" with no WHERE clauses on a table
> with 1% non-null values in col will the planner correctly find the
> partial index? If so why doesn't the min/max planning find it?

It will not.  The hard part of doing something with that is that there
could be more than one aggregate.  I did think about whether we could
just push the IS NOT NULL into the main query, but that falls down on
cases like this:
select min(x), max(y) from tab;

If we try to modify that to
select min(x), max(y) from tab where x is not null and y is not null;

then we get the wrong answers, since x and y are probably nonnull in
different subsets of the table.

In the case of min/max, the endpoint hack makes the aggregates so cheap
that we can afford to perform a separate indexscan for each aggregate,
and thus having a NOT NULL qual that is different for each aggregate
isn't a problem (as long as we make sure it only affects that
aggregate's subquery and not the whole query).  This approach doesn't
scale to aggregates that will scan the whole table, though.

I suppose we might be able to do what you're suggesting for the case of
only one aggregate, but that isn't going to meet the desire of not
having a regression from what 9.0 could do with min/max.
        regards, tom lane

I wrote:
> Hmm.  We could possibly fix this by having planagg.c do a completely
> separate planner run for each aggregate, wherein it actually does build
> the "equivalent" query
>     SELECT col FROM tab WHERE existing-quals AND col IS NOT NULL
>     ORDER BY col ASC/DESC LIMIT 1
> and plan that.  That'd be less efficient than the current way,
> especially for cases where there are multiple aggregates, because there
> would be some duplication of processing between the per-aggregate
> planner runs and the main one.  But since we can only do this
> optimization for rather simple queries anyway, maybe it wouldn't matter
> much.

I studied the code some more, and I think this probably can be made to
work.  The basic idea is to have preprocess_minmax_aggregates build
simplified queries like the above (working by modifying the query tree
that exists at the point where it's called) and call query_planner on
them.  Save aside the resulting path data, then let the regular planning
process continue.  When optimize_minmax_aggregates is called, see
whether the regular plan is cheaper than the sum of the path costs.
If not, use the paths to construct a replacement plan, same as now.

The reason this should work is that query_planner() embodies pretty
much all the useful processing that happens between
preprocess_minmax_aggregates and optimize_minmax_aggregates --- the
other code in that stretch is mostly about grouping, which would disable
the minmax optimization anyway.  So no important steps will get left
out.  Of course, this introduces still more coupling between planagg.c
and planner.c, but I think that's probably tolerable.  The main
objection to this approach is having to do all the index analysis
N+1 times for an N-aggregate query.  I don't see any practical
alternative though if we want to make use of indexes that wouldn't be
used without the IS NOT NULL clause.
        regards, tom lane

On Tue, Mar 22, 2011 at 4:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Reimplement planner's handling of MIN/MAX aggregate optimization (again).
>

I'm just curious, Why is this no longer an interesting special case?


--- this is an interesting special case as of 9.1
-explain (costs off)
-  select min(unique2) from tenk1 where unique2 = 42;
-                  QUERY PLAN
------------------------------------------------
- Aggregate
-   ->  Index Scan using tenk1_unique2 on tenk1
-         Index Cond: (unique2 = 42)



-- 
greg

Greg Stark <gsstark@mit.edu> writes:
> On Tue, Mar 22, 2011 at 4:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Reimplement planner's handling of MIN/MAX aggregate optimization (again).

> I'm just curious, Why is this no longer an interesting special case?

> --- this is an interesting special case as of 9.1
> -explain (costs off)
> -  select min(unique2) from tenk1 where unique2 = 42;
> -                  QUERY PLAN
> ------------------------------------------------
> - Aggregate
> -   ->  Index Scan using tenk1_unique2 on tenk1
> -         Index Cond: (unique2 = 42)

In the pathkey-based implementation, that resulted in an empty pathkey
list, which that implementation couldn't deal with.  I figured that was
okay because the default plan isn't bad in such a case, but I put in a
test case (probably because the code failed before I put in a defense
against it, but I don't recall for sure).  It's not particularly a
corner case for the new code, though, and the resulting plan changed
(because the new code will in fact turn this into a LIMIT subselect
anyway).  So I debated whether to change the expected output or just
take it out, and I chose the latter.
        regards, tom lane

On Tue, Mar 22, 2011 at 01:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I studied the code some more, and I think this probably can be made to
> work.  The basic idea is to have preprocess_minmax_aggregates build
> simplified queries like the above (working by modifying the query tree
> that exists at the point where it's called) and call query_planner on
> them.  Save aside the resulting path data, then let the regular planning
> process continue.  When optimize_minmax_aggregates is called, see
> whether the regular plan is cheaper than the sum of the path costs.
> If not, use the paths to construct a replacement plan, same as now.

Thanks a lot! I can confirm that this is fixed now in git version, and
now also works with partitioned tables, which is great news.

Regards,
Marti