Am 03.08.2019 um 18:38 schrieb Tom Lane:
Daniel Migowski <dmigow...@ikoffice.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.




Reply via email to