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

From Tom Lane
Subject Re: Patch to clean Query after rewrite-and-analyze - reduces memusage up to 50% - increases TPS by up to 50%
Date
Msg-id 15446.1564850332@sss.pgh.pa.us
Whole thread Raw
In response to Patch to clean Query after rewrite-and-analyze - reduces memusage upto 50% - increases TPS by up to 50%  (Daniel Migowski <dmigowski@ikoffice.de>)
Responses Re: Patch to clean Query after rewrite-and-analyze - reduces memusageup to 50% - increases TPS by up to 50%  (Daniel Migowski <dmigowski@ikoffice.de>)
List pgsql-hackers
Daniel Migowski <dmigowski@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.

(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



pgsql-hackers by date:

Previous
From: David Fetter
Date:
Subject: Re: [Patch] Adding CORRESPONDING/CORRESPONDING BY to set operation
Next
From: Michael Banck
Date:
Subject: Re: More issues with pg_verify_checksums and checksum verificationin base backups