Hello, Tomas. my cerebral cortext gets squeezed by jumping from parser to planner.
At Wed, 24 Feb 2016 01:13:22 +0100, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote in <56ccf5a2.5040...@2ndquadrant.com> > Hi, > > On 12/06/2015 11:48 PM, Tomas Vondra wrote: > > /* > > * Frequently, there will be no partial indexes, so first check to > > * make sure there's something useful to do here. > > */ > > have_partial = false; > > foreach(lc, rel->indexlist) > > { > > IndexOptInfo *index = (IndexOptInfo *) lfirst(lc); > > > > /* > > * index rinfos are the same to baseristrict infos for non-partial > > * indexes > > */ > > index->indrinfos = rel->baserestrictinfo; > > > > if (index->indpred == NIL) > > continue; /* ignore non-partial indexes */ > > > > if (index->predOK) > > continue; /* don't repeat work if already proven OK */ > > > > have_partial = true; > > break; > > } > > Attached is a v6 of the patch, which is actually the version submitted > by Kyotaro-san on 2015/10/8 rebased to current master and with two > additional changes. This relies on the fact I believe that no indexrelinfos are shared among any two baserels. Are you OK with that? > Firstly, I've removed the "break" from the initial foreach loop in > check_partial_indexes(). As explained in the previous message, I > believe this was a bug in the patch. I agreed. The break is surely a bug. (the regression didn't find it though..) > Secondly, I've tried to improve the comments to explain a bit better > what the code is doing. In check_partial_indexes, the following commnet. > /* > * Update restrictinfo for this index by excluding clauses that > * are implied by the index predicates. We first quickly check if > * there are any implied clauses, and if we found at least one > * we do the actual work. This way we don't need the construct > * a copy of the list unless actually needed. > */ Is "need the construct" a mistake of "need to construct" ? match_restriction_clauses_to_index is called only from create_index_paths, and now the former doesn't use its first parameter rel. We can safely remove the useless parameter. - match_restriction_clauses_to_index(RelOptInfo *rel, IndexOptInfo *index, - IndexClauseSet *clauseset) + match_restriction_clauses_to_index(IndexOptInfo *index, + IndexClauseSet *clauseset) I find no other problem and have no objection to this. May I mark this "Ready for committer" after fixing them? regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers