Kevin Grittner <kgri...@gmail.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. regards, tom lane -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers