Skip to content

Commit

Permalink
[SPARK-33992][SQL] override transformUpWithNewOutput to add allowInvo…
Browse files Browse the repository at this point in the history
…kingTransformsInAnalyzer

### What changes were proposed in this pull request?

In #29643, we move  the plan rewriting methods to QueryPlan. we need to override transformUpWithNewOutput to add allowInvokingTransformsInAnalyzer
 because it and resolveOperatorsUpWithNewOutput are called in the analyzer.
For example,

PaddingAndLengthCheckForCharVarchar could fail query when resolveOperatorsUpWithNewOutput
with
```logtalk
[info] - char/varchar resolution in sub query  *** FAILED *** (367 milliseconds)
[info]   java.lang.RuntimeException: This method should not be called in the analyzer
[info]   at org.apache.spark.sql.catalyst.plans.logical.AnalysisHelper.assertNotAnalysisRule(AnalysisHelper.scala:150)
[info]   at org.apache.spark.sql.catalyst.plans.logical.AnalysisHelper.assertNotAnalysisRule$(AnalysisHelper.scala:146)
[info]   at org.apache.spark.sql.catalyst.plans.logical.LogicalPlan.assertNotAnalysisRule(LogicalPlan.scala:29)
[info]   at org.apache.spark.sql.catalyst.plans.logical.AnalysisHelper.transformDown(AnalysisHelper.scala:161)
[info]   at org.apache.spark.sql.catalyst.plans.logical.AnalysisHelper.transformDown$(AnalysisHelper.scala:160)
[info]   at org.apache.spark.sql.catalyst.plans.logical.LogicalPlan.transformDown(LogicalPlan.scala:29)
[info]   at org.apache.spark.sql.catalyst.plans.logical.LogicalPlan.transformDown(LogicalPlan.scala:29)
[info]   at org.apache.spark.sql.catalyst.plans.QueryPlan.org$apache$spark$sql$catalyst$plans$QueryPlan$$updateOuterReferencesInSubquery(QueryPlan.scala:267)
```
### Why are the changes needed?

trivial bugfix
### Does this PR introduce _any_ user-facing change?

no
### How was this patch tested?

new tests

Closes #31013 from yaooqinn/SPARK-33992.

Authored-by: Kent Yao <yao@apache.org>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
  • Loading branch information
yaooqinn authored and cloud-fan committed Jan 5, 2021
1 parent 15a863f commit f0ffe0c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,15 @@ trait AnalysisHelper extends QueryPlan[LogicalPlan] { self: LogicalPlan =>
}
}

override def transformUpWithNewOutput(
rule: PartialFunction[LogicalPlan, (LogicalPlan, Seq[(Attribute, Attribute)])],
skipCond: LogicalPlan => Boolean,
canGetOutput: LogicalPlan => Boolean): LogicalPlan = {
AnalysisHelper.allowInvokingTransformsInAnalyzer {
super.transformUpWithNewOutput(rule, skipCond, canGetOutput)
}
}

/**
* Recursively transforms the expressions of a tree, skipping nodes that have already
* been analyzed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,21 @@ trait CharVarcharTestSuite extends QueryTest with SQLTestUtils {
Seq(Row("char(5)"), Row("varchar(3)")))
}
}

test("SPARK-33992: char/varchar resolution in correlated sub query") {
withTable("t1", "t2") {
sql(s"CREATE TABLE t1(v VARCHAR(3), c CHAR(5)) USING $format")
sql(s"CREATE TABLE t2(v VARCHAR(3), c CHAR(5)) USING $format")
sql("INSERT INTO t1 VALUES ('c', 'b')")
sql("INSERT INTO t2 VALUES ('a', 'b')")

checkAnswer(sql(
"""
|SELECT v FROM t1
|WHERE 'a' IN (SELECT v FROM t2 WHERE t1.c = t2.c )""".stripMargin),
Row("c"))
}
}
}

// Some basic char/varchar tests which doesn't rely on table implementation.
Expand Down

0 comments on commit f0ffe0c

Please sign in to comment.