Hello, 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 consistent". The following codelet would be more clearer than my crumsy words:) 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. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers