MaxGekk commented on code in PR #56869:
URL: https://github.com/apache/spark/pull/56869#discussion_r3496537995
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala:
##########
@@ -111,9 +112,72 @@ class FunctionResolution(
* aligned with relation order.
*/
private[analysis] def sqlResolutionPathEntriesForAnalysis: Seq[Seq[String]] =
- catalogManager.resolutionPathEntriesForAnalysis(
- AnalysisContext.get.resolutionPathEntries,
- AnalysisContext.get.catalogAndNamespace)
+ currentResolutionPathCache.pathEntries
+
+ /**
+ * Per-analysis-pass cache of the resolution search path and the derived
+ * "built-in precedes session" flag.
+ *
+ * Computing the path (reading the [[AnalysisContext]] thread-local, the live
+ * [[CatalogManager]], and several confs, then allocating `Seq`s) used to
run once per
+ * [[UnresolvedFunction]] -- and, under Spark Connect, once per node on
every re-analysis of
+ * the growing plan. That per-node recomputation is the SPARK-57758
regression.
+ *
+ * The path is stable within a single analysis pass: `SET PATH` / `USE` /
conf changes happen
+ * between passes, and each pass (and each view / SQL-function body) runs
under a fresh
+ * [[AnalysisContext]] object (see [[AnalysisContext.reset]],
`withAnalysisContext`,
+ * `withNewAnalysisContext`). We therefore key the cache on the identity of
the current
+ * [[AnalysisContext]] and recompute when it changes. This is stale-free
only because the cached
+ * values derive solely from the context's immutable fields
(`resolutionPathEntries`,
+ * `catalogAndNamespace`); a scope change always allocates a new context.
Other context fields
+ * (e.g. `relationCache`, `referredTempFunctionNames`) DO mutate under a
stable identity, so
+ * nothing derived from them may be cached under this key.
+ *
+ * The reference to the [[AnalysisContext]] is held weakly: during a pass
the context is strongly
+ * reachable via its own thread-local, but [[AnalysisContext.reset]] clears
that thread-local
+ * between passes. Keying weakly lets the finished pass's context (and its
`relationCache` plan
+ * graph) be collected rather than pinned on this pooled thread until the
next query overwrites
+ * the entry; a cleared reference simply reads as a miss and recomputes.
+ *
+ * A [[ThreadLocal]] is used because a single [[FunctionResolution]]
instance is shared across
+ * concurrent query threads, each with its own [[AnalysisContext]]
thread-local.
+ */
+ private case class ResolutionPathCache(
+ contextRef: WeakReference[AnalysisContext],
+ pathEntries: Seq[Seq[String]],
+ builtinFastPathSafe: Boolean)
+
+ private val resolutionPathCache = new ThreadLocal[ResolutionPathCache]()
Review Comment:
Thanks, great suggestion -- done in `9cb1be0`. The memo now lives on
`AnalysisContext` as a lazily-filled field (`memoizedResolutionPath`), and as
you predicted all three pieces dropped out:
- **No `ThreadLocal`** -- `AnalysisContext` is already thread-confined via
its own thread-local.
- **No `WeakReference`** -- the memo is collected with the context, so a
finished pass's `relationCache` plan graph isn't pinned on the pooled thread.
- **No `eq`-identity key** -- each caller reads the current context; the
nested view / SQL-function-body case just works because the `copy`/construction
for the body context starts a fresh memo.
I also added a regression test for the nested case you called out (`SECTION
17f`): a SQL-function body pinned to a builtin-first path while the caller
resolves the same unqualified name to a catalog-first shadowing function, so a
single statement yields both resolutions -- confirming neither context reuses
the other's memo. Agreed the shared per-pass context is the natural home; left
unifying relation/variable resolution onto the same memo as a possible
follow-up.
--
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]
