Feedback-ID: ia2694551:Fastmail
Date: Tue, 6 Dec 2022 20:00:33 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Jacob Champion <jchampion@timescale.com>,
	David Rowley <dgrowleyml@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: generic plans and "initial" pruning
Message-ID: <20221206190033.mr5fiucsj4izhc6k@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+HiwqHvF7g7DFeNtMLgJXU2zgcbF8qU5b+BNLCu1AdHA7Q=+w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20221206190033.mr5fiucsj4izhc6k%40alvherre.pgsql>
Precedence: bulk

I find the API of GetCachedPlans a little weird after this patch.  I
think it may be better to have it return a pointer of a new struct --
one that contains both the CachedPlan pointer and the list of pruning
results.  (As I understand, the sole caller that isn't interested in the
pruning results, SPI_plan_get_cached_plan, can be explained by the fact
that it knows there won't be any.  So I don't think we need to worry
about this case?)

And I think you should make that struct also be the last argument of
PortalDefineQuery, so you don't need the separate
PortalStorePartitionPruneResults function -- because as far as I can
tell, the callers that pass a non-NULL pointer there are the exactly
same that later call PortalStorePartitionPruneResults.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La primera ley de las demostraciones en vivo es: no trate de usar el sistema.
Escriba un guión que no toque nada para no causar daños." (Jakob Nielsen)