|From:||Daniel Migowski <dmigowski(at)ikoffice(dot)de>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Subject:||Re: Patch to clean Query after rewrite-and-analyze - reduces memusage up to 50% - increases TPS by up to 50%|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Am 03.08.2019 um 18:38 schrieb Tom Lane:
> 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.
OK, I will investigate.
> (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.)
Having PreparyQuery do _what_ exactly? Sorry, I am still learning how
everything works here.
It seems like the patch crashes the postmaster when I use JOINSs
directly in the PreparedStatement, not when I just place all the Joins
in views. I will also look into this further.
> A few tips on submitting patches:
> * Providing concrete test cases to back up improvement claims is a
> good idea.
OK, I will provide.
> * 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.)
OK. I just tried to have git diff stop marking my indentations red, but
I am also new to git, will use pgindent now.
> * 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.
Was useful here because I had to declare a ListCell anyway, and
ListCell's in other places where named 'lc' not 'l', and 'l' was usually
used for lists, so I thought reusal was nice, but OK.
|Next Message||Alvaro Herrera||2019-08-04 06:29:25||Re: Problem with default partition pruning|
|Previous Message||Alvaro Herrera||2019-08-04 03:57:01||Re: concerns around pg_lsn|