From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, "Imai, Yoshikazu" <imai(dot)yoshikazu(at)jp(dot)fujitsu(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: speeding up planning with partitions |
Date: | 2019-02-18 17:27:35 |
Message-ID: | 30972.1550510855@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> [ v22 patch set ]
I started to look at this, and immediately choked on the 0001 patch:
if (childpruned ||
!apply_child_basequals(root, rel, childrel, childRTE, appinfo) ||
relation_excluded_by_constraints(root, childrel, childRTE))
{
Frankly, that code is just horrid. Having a function with side effects
in an if-test is questionable at the best of times, and having it be
the second of three conditions (which the third condition silently depends
on) is unreadable and unmaintainable.
I think the existing code here is considerably cleaner than what this
patch proposes.
I suppose you are doing this because you intend to jam some additional
cleanup code into the successfully-pruned-it code path, but if said
code is really too bulky to have multiple copies of, couldn't you
put it into a subroutine? You're not going to be able to get to only
one copy of such cleanup anyhow, because there is another early-exit
further down, for the case where set_rel_size detects dummy-ness.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-02-18 17:37:27 | Re: 2019-03 CF Summary / Review - Tranche #2 |
Previous Message | Stephen Frost | 2019-02-18 17:12:36 | Re: WAL insert delay settings |