From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Daniel Migowski <dmigowski(at)ikoffice(dot)de> |
Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Patch to clean Query after rewrite-and-analyze - reduces memusage up to 50% - increases TPS by up to 50% |
Date: | 2019-08-03 16:38:52 |
Message-ID: | 15446.1564850332@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Daniel Migowski <dmigowski(at)ikoffice(dot)de> writes:
> While examining the reasons for excessive memory usage in prepared
> statements I noticed that RTE_JOIN-kind RTEs contain a bunch of
> columnNames and joinaliasvars, that are irrelevant after the Query after
> has been rewritten.
Uh, they're not irrelevant to planning, nor to EXPLAIN. I don't know how
thoroughly you tested this patch, but it seems certain to break things.
As far as the final plan goes, setrefs.c's add_rte_to_flat_rtable already
drops RTE infrastructure that isn't needed by either the executor or
EXPLAIN. But we need it up to that point.
(After thinking a bit, I'm guessing that it seemed not to break because
your tests never actually exercised the generic-plan path, or perhaps
there was always a plancache invalidation before we tried to use the
query_list submitted by PrepareQuery. I wonder if this is telling us
something about the value of having PrepareQuery do that at all,
rather than just caching the raw parse tree and calling it a day.)
A few tips on submitting patches:
* Providing concrete test cases to back up improvement claims is a
good idea.
* Please try to make your code follow established PG style. Ignoring
project conventions about whitespace and brace layout just makes your
code harder to read. (A lot of us would just summarily run the code
through pgindent before trying to review it.)
* Please don't include random cosmetic changes (eg renaming of unrelated
variables) in a patch that's meant to illustrate some specific functional
change. It just confuses matters.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Banck | 2019-08-03 16:47:48 | Re: More issues with pg_verify_checksums and checksum verification in base backups |
Previous Message | David Fetter | 2019-08-03 16:22:29 | Re: [Patch] Adding CORRESPONDING/CORRESPONDING BY to set operation |