Re: PATCH: index-only scans with partial indexes

Hello,

> The attached patch applies on the latest v5 patch and will
> address above issues. (And modifies expected files, which are the
> manifestation of this improovement).

As you see, it is a quite bad choice. Ugly and unreadable and
fragile.

The cause of this seeming mismatch would be the place to hold
indexrinfos. It is determined only by baserestrictinfo and
indpred. Any other components are not involved. So IndexClauseSet
is found not to be the best place after all, I suppose.

Instead, I came to think that the better place is
IndexOptInfo. Partial indexes are examined in check_partial_index
and it seems to be the most proper place to check this so far.

Thougts?

regards,


At Tue, 06 Oct 2015 15:12:02 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20151006.151202.58051959.horiguchi.kyotaro@lab.ntt.co.jp>
> Hello,
> 
> At Thu, 01 Oct 2015 01:36:51 +0200, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote in
<560C7213.3010203@2ndquadrant.com>
> > > Good point. I think we may simply point indexrinfos to the existing
> > > list
> > > of restrictions in that case - we don't need to copy it. So no
> > > additional memory / CPU consumption, and it should make the code
> > > working
> > > with it a bit simpler.
> > 
> > Attached is v5 of the patch improving the comments (rephrasing, moving
> > before function etc.).
> 
> Looks good (for me).
> 
> > I also attempted to fix the issue with empty list for non-partial
> > indexes by simply setting it to rel->baserestrictinfo in
> > match_restriction_clauses_to_index (for non-partial indexes),
> 
> Although it is not the cause of these errors (or any errors on
> regress), build_paths_for_OR (which doesn't seem to be called in
> regression tests) doesn't set indexrinfos
> properly. match_clauses_to_index can commonize(?) the process in
> return for additional list copy and concat.  The attached diff is
> doing in the latter method. Avoiding the copies will make the
> code a bit complicated.
> 
> But anyway regtests doesn't say whether it is sane or not:( (TODO)
> 
> > but that
> > results in a bunch of
> > 
> >    ERROR:  variable not found in subplan target list
> > 
> > during "make installcheck". I can't quite wrap my head around why.
> 
> During considerartion on parameterized joins in
> get_join_index_paths, caluseset fed to get_index_paths is
> generated from given join (j|e)clausesets. So completing the
> clauseset with necessary restrictinfo from rcaluseset fixes the
> problem.
> 
> The attached patch applies on the latest v5 patch and will
> address above issues. (And modifies expected files, which are the
> manifestation of this improovement).

-- 
Kyotaro Horiguchi
NTT Open Source Software Center