|From:||David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>|
|To:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: path toward faster partition pruning|
|Views:||Raw Message | Whole Thread | Download mbox|
On 7 November 2017 at 01:52, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> Thanks. I'll look over it all again starting my Tuesday morning. (UTC+13)
I have a little more review to share:
1. Missing "in" in comment. Should be "mentioned in"
* Return the list of partitions of rel that pass the clauses mentioned
2. Variable should be declared in inner scope with the following fragment:
which makes the code the same as where you moved it from.
3. Normally lfirst() is assigned to a variable at the start of a
foreach() loop. You have code which does not follow this.
if (IsA(lfirst(lc), RestrictInfo))
RestrictInfo *rinfo = lfirst(lc);
You could assign this to a Node * since the type is unknown to you at
the start of the loop.
* Useless if what we're thinking of as a constant is actually
* a Var coming from this relation.
if (bms_is_member(rel->relid, constrelids))
should this be moved to just above the op_strict() test? This one seems cheaper.
5. Typo "paritions": /* No clauses to prune paritions, so scan all
But thinking about it more the comment should something more along the
lines of /* No useful clauses for partition pruning. Scan all
The key difference is that there might be clauses, just without Consts.
Actually, the more I look at get_append_rel_partitions() I think it
would be better if you re-shaped that if/else if test so that it only
performs the loop over the partindexes if it's been set.
I ended up with the attached version of the function after moving
things around a little bit.
I'm still reviewing but thought I'd share this part so far.
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
|Next Message||Michael Paquier||2017-11-07 02:18:26||Re: [PATCH] Assert that the correct locks are held when calling PageGetLSN()|
|Previous Message||Malcolm Locke||2017-11-07 01:51:43||proposal - pg_dump: flag to suppress output of SQL comments|