Re: Patch to clean Query after rewrite-and-analyze - reduces memusage up to 50% - increases TPS by up to 50%

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.