From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | tomas(dot)vondra(at)2ndquadrant(dot)com |
Cc: | kgrittn(at)ymail(dot)com, simon(at)2ndQuadrant(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: PATCH: index-only scans with partial indexes |
Date: | 2016-02-26 06:01:41 |
Message-ID: | 20160226.150141.67405263.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
At Thu, 25 Feb 2016 12:22:45 +0100, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote in <56CEE405(dot)30108(at)2ndquadrant(dot)com>
> >> 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?
>
> I'm not sure what this refers to? You mean the fact that an index will
> have one IndexOptInfo instance for each baserel? Yes, that seems fine
> to me.
Yes that what I meant.
> >> 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?
>
> OK. Do you want me to do the fixes, or will you do that?
Ah. I will do. Please wait a minute.
regares,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2016-02-26 06:05:08 | Re: Declarative partitioning |
Previous Message | Michael Paquier | 2016-02-26 05:43:23 | Re: [REVIEW] In-core regression tests for replication, cascading, archiving, PITR, etc. |