Thanks for the review. On Wed, Dec 7, 2022 at 4:00 AM Alvaro Herrera <alvhe...@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