vladimirg-db commented on code in PR #49445: URL: https://github.com/apache/spark/pull/49445#discussion_r1923923937
########## sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogManager.scala: ########## @@ -49,6 +49,18 @@ class CatalogManager( // TODO: create a real SYSTEM catalog to host `TempVariableManager` under the SESSION namespace. val tempVariableManager: TempVariableManager = new TempVariableManager + // This field will be populated and cleaned up by SqlScriptingExecution. + private val sqlScriptingLocalVariableManager: InheritableThreadLocal[Option[VariableManager]] = Review Comment: > How about we add a new thread-local Thread-local variables complicate things. Shall we move from thread-locals to some cross-query state on the driver? Let me give you some examples where thread-local `AnalysisContext` creates problems: 1. For recursive analyzer invocations, you have to carefully preserve previous contexts. 2. In single-pass Analyzer we try to reuse some code from fixed-point (like `RelationResolution`). But we don't use `AnalysisContext`, because we have proper object-oriented ways to store the resolver state. This creates surprises down the road, where the code that we reuse expects `AnalysisContext` to be populated. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
