From: | "Imai, Yoshikazu" <imai(dot)yoshikazu(at)jp(dot)fujitsu(dot)com> |
---|---|
To: | 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | David Rowley <david(dot)rowley(at)2ndquadrant(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-19 00:49:28 |
Message-ID: | 0F97FA9ABBDBE54F91744A9B37151A5127E7D2@g01jpexmbkw24 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Feb 18, 2019 at 5:28 PM, Tom Lane wrote:
> 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.
When I reviewed this, I thought there are no problems in the codes, but I googled what Tom pointed out[1], read it and I was ashamed of my ignorance.
> 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?
ISTM the 0004 patch eventually removes these codes from multiple places (set_append_rel_size and set_inherited_target_rel_sizes) so we might be better to not be struggling here?
[1] https://www.teamten.com/lawrence/programming/keep-if-clauses-side-effect-free.html
--
Yoshikazu Imai
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2019-02-19 00:54:47 | Re: speeding up planning with partitions |
Previous Message | Andres Freund | 2019-02-19 00:41:07 | Re: Speed up transaction completion faster after many relations are accessed in a transaction |