[SPARK-37202][SQL] Use AnalysisContext to track referred temp functions #34546

linhongliu-db · 2021-11-10T12:03:00Z

What changes were proposed in this pull request?

This PR uses AnalysisContext to track the referred temp functions in order to fix a temp
function resolution issue when it's registered with a FunctionBuilder and referred by a temp view.

During temporary view creation, we need to collect all the temp functions and save them
to the metadata. So that next time when resolving the view SQL text, the functions can be
resolved correctly. But if the temp function is registered with a FunctionBuilder, it's not a
UserDefinedExpression so it cannot be collected as a temp function. As a result, the next time
when the analyzer resolves a temp view, the registered function couldn't be found.

Example:

val func = CatalogFunction(FunctionIdentifier("tempFunc", None), ...)
val builder = (e: Seq[Expression]) => e.head
spark.sessionState.catalog.registerFunction(func, Some(builder))
sql("create temp view tv as select tempFunc(a, b) from values (1, 2) t(a, b)")
sql("select * from tv").collect()

Why are the changes needed?

bug-fix

Does this PR introduce any user-facing change?

no

How was this patch tested?

newly added test cases.

SparkQA · 2021-11-10T12:58:18Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49532/

SparkQA · 2021-11-10T13:40:51Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49532/

SparkQA · 2021-11-10T15:57:41Z

Test build #145062 has finished for PR 34546 at commit 6f2e9a6.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

linhongliu-db · 2021-11-11T05:14:58Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala

+    // Collect the referred temporary functions from AnalysisContext
+    copy(referredTempFunctions = analysisContext.referredTempFunctionNames.toSeq)
+  }
+
  def markAsAnalyzed(): LogicalPlan = copy(isAnalyzed = true)


i'm thinking about combining them to

def markAsAnalyzed(analysisContext: AnalysisContext): LogicalPlan = { copy( isAnalyzed = true, referredTempFunctions = analysisContext.referredTempFunctionNames.toSeq) }

cannot decide which is better

I think combining is better.

linhongliu-db · 2021-11-11T05:16:46Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

      executeSameContext(plan)
-    } finally {
-      AnalysisContext.reset()


The rule ResolveEncodersInUDF will call analyzer. execute() again, the AnalysisContext will be messed up after that.

This looks like a bug, can we open a separate PR for it? then we can backport it.

SparkQA · 2021-11-11T05:51:55Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49549/

SparkQA · 2021-11-11T06:51:03Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49549/

SparkQA · 2021-11-11T09:11:13Z

Test build #145080 has finished for PR 34546 at commit aa9ef80.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

linhongliu-db · 2021-11-11T09:41:46Z

cc @cloud-fan

SparkQA · 2021-11-11T09:52:47Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49566/

SparkQA · 2021-11-11T10:50:54Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49566/

SparkQA · 2021-11-11T12:21:26Z

Test build #145096 has finished for PR 34546 at commit 76153a6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-11-12T06:19:04Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49616/

SparkQA · 2021-11-12T06:20:21Z

Test build #145145 has finished for PR 34546 at commit 44ce71c.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-11-12T08:39:41Z

Test build #145154 has finished for PR 34546 at commit f59a709.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-11-12T09:24:33Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49626/

SparkQA · 2021-11-12T12:08:15Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49629/

SparkQA · 2021-11-12T12:54:40Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49629/

cloud-fan · 2021-11-12T14:20:00Z

thanks, merging to master/3.2!

This PR uses `AnalysisContext` to track the referred temp functions in order to fix a temp function resolution issue when it's registered with a `FunctionBuilder` and referred by a temp view. During temporary view creation, we need to collect all the temp functions and save them to the metadata. So that next time when resolving the view SQL text, the functions can be resolved correctly. But if the temp function is registered with a `FunctionBuilder`, it's not a `UserDefinedExpression` so it cannot be collected as a temp function. As a result, the next time when the analyzer resolves a temp view, the registered function couldn't be found. Example: ```scala val func = CatalogFunction(FunctionIdentifier("tempFunc", None), ...) val builder = (e: Seq[Expression]) => e.head spark.sessionState.catalog.registerFunction(func, Some(builder)) sql("create temp view tv as select tempFunc(a, b) from values (1, 2) t(a, b)") sql("select * from tv").collect() ``` bug-fix no newly added test cases. Closes #34546 from linhongliu-db/SPARK-37702-ver3. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 68a0ab5) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

SparkQA · 2021-11-12T16:01:41Z

Test build #145158 has finished for PR 34546 at commit 86ab71b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

…ableAsSelect ### What changes were proposed in this pull request? This is a follow-up of PR #34546 which uses the `AnalysisContext` to store referred temporary functions when creating temp views. But the `CacheTableAsSelect` also needs to update the temporary functions because it's a temp view under the hood. ### Why are the changes needed? Followup of #34546 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? newly added ut Closes #34603 from linhongliu-db/SPARK-37702-followup. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…ableAsSelect ### What changes were proposed in this pull request? This is a follow-up of PR #34546 which uses the `AnalysisContext` to store referred temporary functions when creating temp views. But the `CacheTableAsSelect` also needs to update the temporary functions because it's a temp view under the hood. ### Why are the changes needed? Followup of #34546 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? newly added ut Closes #34603 from linhongliu-db/SPARK-37702-followup. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b6c472e) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

This PR uses `AnalysisContext` to track the referred temp functions in order to fix a temp function resolution issue when it's registered with a `FunctionBuilder` and referred by a temp view. During temporary view creation, we need to collect all the temp functions and save them to the metadata. So that next time when resolving the view SQL text, the functions can be resolved correctly. But if the temp function is registered with a `FunctionBuilder`, it's not a `UserDefinedExpression` so it cannot be collected as a temp function. As a result, the next time when the analyzer resolves a temp view, the registered function couldn't be found. Example: ```scala val func = CatalogFunction(FunctionIdentifier("tempFunc", None), ...) val builder = (e: Seq[Expression]) => e.head spark.sessionState.catalog.registerFunction(func, Some(builder)) sql("create temp view tv as select tempFunc(a, b) from values (1, 2) t(a, b)") sql("select * from tv").collect() ``` bug-fix no newly added test cases. Closes apache#34546 from linhongliu-db/SPARK-37702-ver3. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 68a0ab5) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…ableAsSelect ### What changes were proposed in this pull request? This is a follow-up of PR apache#34546 which uses the `AnalysisContext` to store referred temporary functions when creating temp views. But the `CacheTableAsSelect` also needs to update the temporary functions because it's a temp view under the hood. ### Why are the changes needed? Followup of apache#34546 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? newly added ut Closes apache#34603 from linhongliu-db/SPARK-37702-followup. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b6c472e) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

This PR uses `AnalysisContext` to track the referred temp functions in order to fix a temp function resolution issue when it's registered with a `FunctionBuilder` and referred by a temp view. During temporary view creation, we need to collect all the temp functions and save them to the metadata. So that next time when resolving the view SQL text, the functions can be resolved correctly. But if the temp function is registered with a `FunctionBuilder`, it's not a `UserDefinedExpression` so it cannot be collected as a temp function. As a result, the next time when the analyzer resolves a temp view, the registered function couldn't be found. Example: ```scala val func = CatalogFunction(FunctionIdentifier("tempFunc", None), ...) val builder = (e: Seq[Expression]) => e.head spark.sessionState.catalog.registerFunction(func, Some(builder)) sql("create temp view tv as select tempFunc(a, b) from values (1, 2) t(a, b)") sql("select * from tv").collect() ``` bug-fix no newly added test cases. Closes apache#34546 from linhongliu-db/SPARK-37702-ver3. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 68a0ab5) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…ableAsSelect ### What changes were proposed in this pull request? This is a follow-up of PR apache#34546 which uses the `AnalysisContext` to store referred temporary functions when creating temp views. But the `CacheTableAsSelect` also needs to update the temporary functions because it's a temp view under the hood. ### Why are the changes needed? Followup of apache#34546 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? newly added ut Closes apache#34603 from linhongliu-db/SPARK-37702-followup. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b6c472e) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

This PR uses `AnalysisContext` to track the referred temp functions in order to fix a temp function resolution issue when it's registered with a `FunctionBuilder` and referred by a temp view. During temporary view creation, we need to collect all the temp functions and save them to the metadata. So that next time when resolving the view SQL text, the functions can be resolved correctly. But if the temp function is registered with a `FunctionBuilder`, it's not a `UserDefinedExpression` so it cannot be collected as a temp function. As a result, the next time when the analyzer resolves a temp view, the registered function couldn't be found. Example: ```scala val func = CatalogFunction(FunctionIdentifier("tempFunc", None), ...) val builder = (e: Seq[Expression]) => e.head spark.sessionState.catalog.registerFunction(func, Some(builder)) sql("create temp view tv as select tempFunc(a, b) from values (1, 2) t(a, b)") sql("select * from tv").collect() ``` bug-fix no newly added test cases. Closes apache#34546 from linhongliu-db/SPARK-37702-ver3. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 68a0ab5) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…ableAsSelect ### What changes were proposed in this pull request? This is a follow-up of PR apache#34546 which uses the `AnalysisContext` to store referred temporary functions when creating temp views. But the `CacheTableAsSelect` also needs to update the temporary functions because it's a temp view under the hood. ### Why are the changes needed? Followup of apache#34546 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? newly added ut Closes apache#34603 from linhongliu-db/SPARK-37702-followup. Authored-by: Linhong Liu <linhong.liu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b6c472e) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

[SPARK-37702] Use AnalysisContext to track referred temp functions

6f2e9a6

linhongliu-db changed the title ~~[SPARK-37702] Use AnalysisContext to track referred temp functions~~ [SPARK-37702][SQL] Use AnalysisContext to track referred temp functions Nov 10, 2021

github-actions bot added the SQL label Nov 10, 2021

linhongliu-db mentioned this pull request Nov 10, 2021

[SPARK-37202][SQL] Treat injectedFunctions as custom built-in functions #34528

Closed

linhongliu-db added 2 commits November 11, 2021 12:30

address comments

c63c0a8

fix typo

aa9ef80

linhongliu-db commented Nov 11, 2021

View reviewed changes

fix test

76153a6

linhongliu-db added 2 commits November 12, 2021 13:48

address comment

2b4c771

code style

44ce71c

fix

f59a709

fix

86ab71b

cloud-fan approved these changes Nov 12, 2021

View reviewed changes

cloud-fan closed this in 68a0ab5 Nov 12, 2021

cloud-fan changed the title ~~[SPARK-37702][SQL] Use AnalysisContext to track referred temp functions~~ [SPARK-37202][SQL] Use AnalysisContext to track referred temp functions Nov 12, 2021

linhongliu-db mentioned this pull request Nov 15, 2021

[SPARK-37702][SQL][FOLLOWUP] Store referred temp functions for CacheTableAsSelect #34603

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-37202][SQL] Use AnalysisContext to track referred temp functions #34546

[SPARK-37202][SQL] Use AnalysisContext to track referred temp functions #34546

linhongliu-db commented Nov 10, 2021 •

edited

Loading

SparkQA commented Nov 10, 2021

SparkQA commented Nov 10, 2021

SparkQA commented Nov 10, 2021

linhongliu-db Nov 11, 2021

cloud-fan Nov 12, 2021

linhongliu-db Nov 11, 2021

cloud-fan Nov 12, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

linhongliu-db commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

cloud-fan commented Nov 12, 2021

SparkQA commented Nov 12, 2021

[SPARK-37202][SQL] Use AnalysisContext to track referred temp functions #34546

[SPARK-37202][SQL] Use AnalysisContext to track referred temp functions #34546

Conversation

linhongliu-db commented Nov 10, 2021 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Nov 10, 2021

SparkQA commented Nov 10, 2021

SparkQA commented Nov 10, 2021

linhongliu-db Nov 11, 2021

Choose a reason for hiding this comment

cloud-fan Nov 12, 2021

Choose a reason for hiding this comment

linhongliu-db Nov 11, 2021

Choose a reason for hiding this comment

cloud-fan Nov 12, 2021

Choose a reason for hiding this comment

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

linhongliu-db commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 11, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

SparkQA commented Nov 12, 2021

cloud-fan commented Nov 12, 2021

SparkQA commented Nov 12, 2021

linhongliu-db commented Nov 10, 2021 •

edited

Loading