Re: PATCH: index-only scans with partial indexes

On 03/31/2016 01:36 AM, Tom Lane wrote:
> Kevin Grittner <kgrittn(at)gmail(dot)com> writes:
>> I'm taking my name off as committer and marking it "Ready for
>> Committer". If someone else wants to comment on the issues where
>> Tom and Kyotaro-san still seem unsatisfied to the point where I
>> can get my head around it, I could maybe take it back on as
>> committer -- if anyone feels that could be a net win.
>
> I'll pick it up. In a quick scan I see some things I don't like, but
> nothing insoluble:
>
> * Having plancat.c initialize the per-IndexOptInfo restriction lists seems
> fairly bizarre. I realize that Tomas or Kyotaro probably did that in
> response to my complaint about having check_partial_indexes have
> side-effects on non-partial indexes, but this isn't an improvement. For
> one thing, it means we will produce an incorrect plan if more restriction
> clauses are added to the rel after plancat.c runs, as the comment for
> check_partial_indexes contemplates. (I *think* that comment may be
> obsolete, but I'm not sure.) I think a better answer to that complaint is
> to rename check_partial_indexes to something else, and more importantly
> adjust its header comment to reflect its expanded responsibilities --- as
> the patch I was commenting on at the time failed to do.
>
> * It took me some time to convince myself that the patch doesn't break
> generation of plans for EvalPlanQual. I eventually found where it
> skips removal of restrictions for target relations, but that's drastically
> undercommented.
>
> * Likewise, there's inadequate documentation of why it doesn't break
> bitmap scans, which also have to be able to recheck correctly.
>
> * I'm not sure that we have any regression test cases covering the
> above two points, but we should.
>
> * The comments leave a lot to be desired in general, eg there are
> repeated failures to update comments only a few lines away from a change.

Kyotaro,

I may look into fixing those issues early next week, but that's fairly
close to the freeze. Also, I'll have to look into parts that I'm not
very familiar with (like the EvalPlanQual), so feel free to address
those issues ;-)

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services