Re: PATCH: index-only scans with partial indexes

At Thu, 31 Mar 2016 14:51:02 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <19589(dot)1459450262(at)sss(dot)pgh(dot)pa(dot)us>
> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
> > Thank you for the comment. The new version is attached.
>
> Committed with rather heavy editorialization and a batch of regression
> test cases.
>
> regards, tom lane

Many thanks for the editorialization (or refactoring), and many
descriptive comments and testing, then committing.

There seems no problem for me of that. The followings are my
reviw and thought on that, FWIW.

======
check_index_predicates:
+ * At one time it was possible for this to get re-run after adding more
+ * restrictions to the rel, thus possibly letting us prove more indexes OK.
+ * That doesn't happen any more (at least not in the core code's usage),
!+ * but this code still supports it in case extensions want to mess with the
!+ * baserestrictinfo list. We assume that adding more restrictions can't make
+ * an index not predOK. We must recompute indrestrictinfo each time, though,
+ * to make sure any newly-added restrictions get into it if needed.

I didn't imagine that the assumption is for the sake of extensions..

+check_index_predicates(PlannerInfo *root, RelOptInfo *rel)
...
+ index->indrestrictinfo = rel->baserestrictinfo;

Ah. This has been retuened here.

+ is_target_rel = (rel->relid == root->parse->resultRelation ||
+ get_plan_rowmark(root->rowMarks, rel->relid) != NULL);

This is very descriptive even for me.

- * We can also discard quals that are implied by a partial index's
- * predicate, but only in a plain SELECT; when scanning a target relation
- * of UPDATE/DELETE/SELECT FOR UPDATE, we must leave such quals in the
- * plan so that they'll be properly rechecked by EvalPlanQual testing.
- *

Uggg. I'm sorry that I couldn't find this commnet just above what
I removed.

+ * way because predicate conditions need to be rechecked if the scan
+ * becomes lossy, so they have to be included in bitmapqualorig.

Of couse 'lossy' means 'may contain unqualified data', my brain
must have been in powersaving mode.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center