Hi,
On 09/26/2015 01:28 PM, Tomas Vondra wrote:
> The patch does not change the check_index_only implementation - it
> still needs to check the clauses, just like in v1 of the patch. To
> make this re-check unnecessary, we'd have to stick the remaining
> clauses somewhere, so that check_index_only can use only the filtered
> list (instead of walking through the complete list of restrictions).
After thinking about this a bit more, I realized we already have a good
place for keeping this list - IndexClauseSet. So I've done that, and
removed most of the code from check_index_only - it still needs to
decide whether to use the full list of restrictions (regular indexes) or
the filtered list (for partial indexes).
Calling predicate_implied_by in match_clause_to_index makes the check a
bit earlier, compared to the v1 of the patch. So theoretically there
might be cases where we'd interrupt the execution between those places,
and the v1 would not pay the price for the call. But those places are
fairly close together, so I find that unlikely and I've been unable to
reproduce a plausible example of such regression despite trying.
The only regression test this breaks is "aggregates", and I believe
that's actually correct because it changes the plans like this:
-> Index Only Scan Backward using minmaxtest2i on minmaxtest2
Index Cond: (f1 IS NOT NULL)
-> Index Only Scan using minmaxtest3i on minmaxtest3
- Index Cond: (f1 IS NOT NULL)
i.e. it removes the Index Condition from scans of minmaxtest3i, which is
perfectly sensible because the index is defined like this:
create index minmaxtest3i on minmaxtest3(f1) where f1 is not null;
So it's partial index and that condition is implied by the predicate
(it's actually exactly the same).
I haven't fixed those regression tests for now.
>
> Or perhaps we can do the check in match_clause_to_index, pretty much
> for free? If the clause is not implied by the index predicate (which
> we know thanks to the fix), and we don't assign it to any of the
> index columns, it means we can'd use IOS, no?
After thinking about this a bit more, this is clearly nonsense. It fails
on conditions referencing multiple columns (WHERE a=b) which can't be
assigned to a single index column. So the logic would have to be much
more complicated, effectively doing what check_index_only is doing just
a tiny bit later.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services