zhuqi-lucas opened a new issue, #25141: URL: https://github.com/apache/datafusion/issues/25141
Follow-up to #25098 (cc @asolimando). ## Background `StatisticsContext`'s memoization cache is keyed by the raw `ExecutionPlan` node pointer address. This is safe for the common usage — a fresh context computing statistics over a single, stable plan tree. It becomes subtle when one context is **shared across a physical-optimizer pass that rewrites the plan** (e.g. `EnsureRequirements` in #25098 reuses one context across its `ensure_distribution` `transform_up`). During such a pass a node can be freed when it is replaced, and a later allocation could reuse that address, so a stale cache entry could be returned for a different node (an ABA hazard). ## Current mitigation (in #25098) The consumer resets the cache after any node whose plan pointer actually changed (`Arc::ptr_eq` before/after). This is safe, but it puts the burden on every consumer that shares a context across a rewrite. ## Proposal Make the cache robust so consumers do not have to think about this, per @asolimando's suggestions: - **(a)** Give each constructed `ExecutionPlan` a unique id and key the cache on it instead of the pointer. Cleanest long-term, but touches every plan node. - **(b)** Have the cache keep the nodes alive for its lifetime (hold an `Arc` clone of each cached node) so an address cannot be reused. Smaller in spirit, but `compute` currently takes `&dyn ExecutionPlan` and is called from ~10 sites, so threading an `Arc` through would be a wider API change. Either removes the consumer-side reset. Filing so the robustness work is tracked separately from #25098 (which keeps the safe reset). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
