MIME-Version: 1.0
References: 
 <CA+HiwqHvF7g7DFeNtMLgJXU2zgcbF8qU5b+BNLCu1AdHA7Q=+w@mail.gmail.com>
 <20221206190033.mr5fiucsj4izhc6k@alvherre.pgsql>
In-Reply-To: <20221206190033.mr5fiucsj4izhc6k@alvherre.pgsql>
From: Amit Langote <amitlangote09@gmail.com>
Date: Fri, 9 Dec 2022 17:26:59 +0900
Message-ID: 
 <CA+HiwqG04V9u9mfeGOW1QTDphQ2PvQv5jJz2EFtuspU4fFcLBw@mail.gmail.com>
Subject: Re: generic plans and "initial" pruning
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
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>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BHiwqG04V9u9mfeGOW1QTDphQ2PvQv5jJz2EFtuspU4fFcLBw%40mail.gmail.com>
Precedence: bulk

Thanks for the review.

On Wed, Dec 7, 2022 at 4:00 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> 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?)

David, in his Apr 7 reply on this thread, also sounded to suggest
something similar.

Hmm, I was / am not so sure if GetCachedPlan() should return something
that is not CachedPlan.  An idea I had today was to replace the
part_prune_results_list output List parameter with, say,
QueryInitPruningResult, or something like that and put the current
list into that struct.   Was looking at QueryEnvironment to come up
with *that* name.  Any thoughts?

> 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.

Yes, it would be better to not need PortalStorePartitionPruneResults.


--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com