On Tue, Jul 22, 2025 at 03:28:24PM -0500, Sami Imseih wrote: > I know Michael opposed the idea of carrying these structures, > at least CachedPlan, to Executor hooks ( or maybe just not QueryDesc?? ). > It will be good to see what he think, or if others an opinion about this, > about > adding a pointer to CachedPlanSource in PlannedStmt vs setting a flag in > PlannedStmt to track plan cache type for the current execution? The former > does provide more capability for extensions, as Andrei has pointed out > earlier.
My line is pretty simple here: we should never include in executor-specific or plan-specific areas structures that are handled by entirely different memory contexts or portions of the code, all parts (executor, query description and or [cached] plan) relying on entirely different assumptions and contexts, because I suspect that it would bite us back hard in the shape of corrupted stacks depending on the timing where these resources are freed. One of the upthread proposals where a reference of the cached plan pointer was added to an executor state structure crossed this line. Simple fields are no-brainers, they are just carried in the same context as the caller, independently of other states. When it comes to stats and monitoring, we don't have to be exact, we can sometimes even be a bit lossy in the reports. A simple "plan type" field in a PlannedStmt should be more than OK, as it would be carried across the contexts where we want to know about it and handle it, aka PGSS entry storing. -- Michael
signature.asc
Description: PGP signature
