At Tue, 29 Sep 2015 16:57:03 +0200, Tomas Vondra <tomas.von...@2ndquadrant.com> 
wrote in <560aa6bf.5030...@2ndquadrant.com>
> >>> 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,
> >
> > The place looks fine but it might be better that rclauses have
> > baserestrictinfo itself when indpred == NIL. It would make the
> > semantics of rclauses more consistent.
> >
> >> 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).
> >
> > The change above allows to reduce the modification still left in
> > check_index_only.
> I'm not sure I understand what change you suggest? Could you explain?
> The change in check_index_only is effectively just (a) comment update
> and (b) choice of the right list of clauses. I'd say both changes are
> needed, although (b) could happen outside check_index_only (i.e. we
> could do the check elsewhere). Is that what you mean?

I meant that the (b) could be done earlier in
match_clause_to_index. Currently clauseset->rclauses stores given
(restrict) clauses which are not implied by indpred *only when*
indpred is present. But it's no problem that rclauses *always*
stores such clauses. rclauses is still storing clauses "not
implied by the index" for the case. It is what I meant by "more

The following codelet would be more clearer than my crumsy

in match_clause_to_index()
>       if (index->indpred != NIL &&
>           predicate_implied_by(list_make1(rinfo->clause), index->indpred))
>               return;  /* ignore clauses implied by index */
>       /* track all clauses not implied by indpred */
>       clauseset->rclauses = lappend(clauseset->rclauses, rinfo);
>       for (indexcol = 0; indexcol < index->ncolumns; indexcol++)

in check_index_only()
-        * For partial indexes use the filtered clauses, otherwise use the
-        * baserestrictinfo directly for non-partial indexes.
-        */
-       rclauses = (index->indpred != NIL) ? clauses : rel->baserestrictinfo;
-       /*
         * Add all the attributes used by restriction clauses (only those not
         * implied by the index predicate for partial indexes).
-       foreach(lc, rclauses)
+       foreach(lc, clauses)

> > cost_index() seems to need to be fixed. It would count excluded
> > clauses in estimate.
> Hmm, good point. The problem is that extract_nonindex_conditions uses
> baserel->baserestrictinfo again, i.e. it does not skip the implied
> clauses. So we may either stick the filtered clauses somewhere (for
> example in the IndexPath), teach extract_nonindex_conditions to use
> predicate_implied_by. I'd say the first option is better. Agreed?

I'll mention this in a separate reply for the following mail.


Kyotaro Horiguchi
NTT Open Source Software Center

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to