[SPARK-35673][SQL] Fix user-defined hint and unrecognized hint in subquery. #32841

cfmcgrady · 2021-06-09T10:18:31Z

What changes were proposed in this pull request?

Use UnresolvedHint.resolved = child.resolved instead UnresolvedHint.resolved = false, then the plan contains UnresolvedHint child can be optimized by rule in batch Resolution.

For instance, before this pr, the following plan can't be optimized by ResolveReferences.

!'Project [*]                                      
 +- SubqueryAlias __auto_generated_subquery_name  
    +- UnresolvedHint use_hash                     
       +- Project [42 AS 42#10]                    
          +- OneRowRelation

Why are the changes needed?

fix hint in subquery bug

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New test.

…query.

cfmcgrady · 2021-06-09T11:42:07Z

cc @cloud-fan @maropu

cloud-fan · 2021-06-09T14:03:47Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/hints.scala

  extends UnaryNode {

-  override lazy val resolved: Boolean = false
+  override lazy val resolved: Boolean = child.resolved


Let's add some comments to explain the reason: we need it to be resolved so that the analyzer can continue to analyze the rest of the query plan.

To make up for this change, let's add a check in CheckAnalysis and fail if the plan still contains UnresolvedHint after analysis.

It's ok for me to add this check.
And note that in the build-in analyzer, all of UnresolvedHint will be removed by batch Remove Unresolved Hints.

Yea, it's just for sanity check, to make sure UnresolvedHint shouldn't exist after analysis.

cloud-fan · 2021-06-10T04:52:56Z

ok to test

SparkQA · 2021-06-10T06:21:52Z

Kubernetes integration test unable to build dist.

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

cloud-fan · 2021-06-10T07:32:08Z

thanks, merging to master/3.1/3.0!

…query Use `UnresolvedHint.resolved = child.resolved` instead `UnresolvedHint.resolved = false`, then the plan contains `UnresolvedHint` child can be optimized by rule in batch `Resolution`. For instance, before this pr, the following plan can't be optimized by `ResolveReferences`. ``` !'Project [*] +- SubqueryAlias __auto_generated_subquery_name +- UnresolvedHint use_hash +- Project [42 AS 42#10] +- OneRowRelation ``` fix hint in subquery bug No. New test. Closes #32841 from cfmcgrady/SPARK-35673. Authored-by: Fu Chen <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 5280f02) Signed-off-by: Wenchen Fan <[email protected]>

SparkQA · 2021-06-10T10:27:48Z

Test build #139615 has finished for PR 32841 at commit 94d22b2.

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

maropu · 2021-06-11T01:25:37Z

late lgtm. Thank you for this fix, @cfmcgrady .

…query Use `UnresolvedHint.resolved = child.resolved` instead `UnresolvedHint.resolved = false`, then the plan contains `UnresolvedHint` child can be optimized by rule in batch `Resolution`. For instance, before this pr, the following plan can't be optimized by `ResolveReferences`. ``` !'Project [*] +- SubqueryAlias __auto_generated_subquery_name +- UnresolvedHint use_hash +- Project [42 AS 42#10] +- OneRowRelation ``` fix hint in subquery bug No. New test. Closes apache#32841 from cfmcgrady/SPARK-35673. Authored-by: Fu Chen <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

…query Use `UnresolvedHint.resolved = child.resolved` instead `UnresolvedHint.resolved = false`, then the plan contains `UnresolvedHint` child can be optimized by rule in batch `Resolution`. For instance, before this pr, the following plan can't be optimized by `ResolveReferences`. ``` !'Project [*] +- SubqueryAlias __auto_generated_subquery_name +- UnresolvedHint use_hash +- Project [42 AS 42#10] +- OneRowRelation ``` fix hint in subquery bug No. New test. Closes apache#32841 from cfmcgrady/SPARK-35673. Authored-by: Fu Chen <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 5280f02) Signed-off-by: Wenchen Fan <[email protected]>

AngersZhuuuu · 2021-11-18T02:53:20Z

This issue was introduced by #25746
Looks good.

…query Use `UnresolvedHint.resolved = child.resolved` instead `UnresolvedHint.resolved = false`, then the plan contains `UnresolvedHint` child can be optimized by rule in batch `Resolution`. For instance, before this pr, the following plan can't be optimized by `ResolveReferences`. ``` !'Project [*] +- SubqueryAlias __auto_generated_subquery_name +- UnresolvedHint use_hash +- Project [42 AS 42#10] +- OneRowRelation ``` fix hint in subquery bug No. New test. Closes apache#32841 from cfmcgrady/SPARK-35673. Authored-by: Fu Chen <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 5280f02) Signed-off-by: Wenchen Fan <[email protected]>

### What changes were proposed in this pull request? Skip `UnresolvedHint` in rule `AddMetadataColumns` to avoid call exprId on `UnresolvedAttribute`. ### Why are the changes needed? ``` CREATE TABLE t1(c1 bigint) USING PARQUET; CREATE TABLE t2(c2 bigint) USING PARQUET; SELECT /*+ hash(t2) */ * FROM t1 join t2 on c1 = c2; ``` failed with msg: ``` org.apache.spark.sql.catalyst.analysis.UnresolvedException: Invalid call to exprId on unresolved object at org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute.exprId(unresolved.scala:147) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$4(Analyzer.scala:1005) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$4$adapted(Analyzer.scala:1005) at scala.collection.Iterator.exists(Iterator.scala:969) at scala.collection.Iterator.exists$(Iterator.scala:967) at scala.collection.AbstractIterator.exists(Iterator.scala:1431) at scala.collection.IterableLike.exists(IterableLike.scala:79) at scala.collection.IterableLike.exists$(IterableLike.scala:78) at scala.collection.AbstractIterable.exists(Iterable.scala:56) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$3(Analyzer.scala:1005) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$3$adapted(Analyzer.scala:1005) ``` But before just a warning: `WARN HintErrorLogger: Unrecognized hint: hash(t2)` ### Does this PR introduce _any_ user-facing change? yes, fix regression from 3.3.1. Note, the root reason is we mark `UnresolvedHint` is resolved if child is resolved since #32841, then #37758 trigger this bug. ### How was this patch tested? add test Closes #38662 from ulysses-you/hint. Authored-by: ulysses-you <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

Skip `UnresolvedHint` in rule `AddMetadataColumns` to avoid call exprId on `UnresolvedAttribute`. ``` CREATE TABLE t1(c1 bigint) USING PARQUET; CREATE TABLE t2(c2 bigint) USING PARQUET; SELECT /*+ hash(t2) */ * FROM t1 join t2 on c1 = c2; ``` failed with msg: ``` org.apache.spark.sql.catalyst.analysis.UnresolvedException: Invalid call to exprId on unresolved object at org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute.exprId(unresolved.scala:147) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$4(Analyzer.scala:1005) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$4$adapted(Analyzer.scala:1005) at scala.collection.Iterator.exists(Iterator.scala:969) at scala.collection.Iterator.exists$(Iterator.scala:967) at scala.collection.AbstractIterator.exists(Iterator.scala:1431) at scala.collection.IterableLike.exists(IterableLike.scala:79) at scala.collection.IterableLike.exists$(IterableLike.scala:78) at scala.collection.AbstractIterable.exists(Iterable.scala:56) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$3(Analyzer.scala:1005) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$3$adapted(Analyzer.scala:1005) ``` But before just a warning: `WARN HintErrorLogger: Unrecognized hint: hash(t2)` yes, fix regression from 3.3.1. Note, the root reason is we mark `UnresolvedHint` is resolved if child is resolved since #32841, then #37758 trigger this bug. add test Closes #38662 from ulysses-you/hint. Authored-by: ulysses-you <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit a9bf5d2) Signed-off-by: Wenchen Fan <[email protected]>

### What changes were proposed in this pull request? Skip `UnresolvedHint` in rule `AddMetadataColumns` to avoid call exprId on `UnresolvedAttribute`. ### Why are the changes needed? ``` CREATE TABLE t1(c1 bigint) USING PARQUET; CREATE TABLE t2(c2 bigint) USING PARQUET; SELECT /*+ hash(t2) */ * FROM t1 join t2 on c1 = c2; ``` failed with msg: ``` org.apache.spark.sql.catalyst.analysis.UnresolvedException: Invalid call to exprId on unresolved object at org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute.exprId(unresolved.scala:147) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$4(Analyzer.scala:1005) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$4$adapted(Analyzer.scala:1005) at scala.collection.Iterator.exists(Iterator.scala:969) at scala.collection.Iterator.exists$(Iterator.scala:967) at scala.collection.AbstractIterator.exists(Iterator.scala:1431) at scala.collection.IterableLike.exists(IterableLike.scala:79) at scala.collection.IterableLike.exists$(IterableLike.scala:78) at scala.collection.AbstractIterable.exists(Iterable.scala:56) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$3(Analyzer.scala:1005) at org.apache.spark.sql.catalyst.analysis.Analyzer$AddMetadataColumns$.$anonfun$hasMetadataCol$3$adapted(Analyzer.scala:1005) ``` But before just a warning: `WARN HintErrorLogger: Unrecognized hint: hash(t2)` ### Does this PR introduce _any_ user-facing change? yes, fix regression from 3.3.1. Note, the root reason is we mark `UnresolvedHint` is resolved if child is resolved since apache#32841, then apache#37758 trigger this bug. ### How was this patch tested? add test Closes apache#38662 from ulysses-you/hint. Authored-by: ulysses-you <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

[SPARK-35673][SQL] Fix user-defined hint and unrecognized hint in sub…

fe72228

…query.

github-actions bot added the SQL label Jun 9, 2021

cloud-fan reviewed Jun 9, 2021

View reviewed changes

cfmcgrady added 2 commits June 10, 2021 10:51

checkAnalysis for UnresolvedHint

b542543

typo

94d22b2

cloud-fan approved these changes Jun 10, 2021

View reviewed changes

cloud-fan closed this in 5280f02 Jun 10, 2021

ulysses-you mentioned this pull request Nov 15, 2022

[SPARK-41144][SQL] Unresolved hint should not cause query failure #38662

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-35673][SQL] Fix user-defined hint and unrecognized hint in subquery. #32841

[SPARK-35673][SQL] Fix user-defined hint and unrecognized hint in subquery. #32841

Uh oh!

cfmcgrady commented Jun 9, 2021

Uh oh!

cfmcgrady commented Jun 9, 2021

Uh oh!

cloud-fan Jun 9, 2021 •

edited

Loading

Uh oh!

cloud-fan Jun 9, 2021

Uh oh!

cfmcgrady Jun 10, 2021

Uh oh!

cloud-fan Jun 10, 2021

Uh oh!

cloud-fan commented Jun 10, 2021

Uh oh!

SparkQA commented Jun 10, 2021

Uh oh!

cloud-fan commented Jun 10, 2021

Uh oh!

SparkQA commented Jun 10, 2021

Uh oh!

maropu commented Jun 11, 2021

Uh oh!

AngersZhuuuu commented Nov 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[SPARK-35673][SQL] Fix user-defined hint and unrecognized hint in subquery. #32841

[SPARK-35673][SQL] Fix user-defined hint and unrecognized hint in subquery. #32841

Uh oh!

Conversation

cfmcgrady commented Jun 9, 2021

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?

Uh oh!

cfmcgrady commented Jun 9, 2021

Uh oh!

cloud-fan Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 9, 2021

Choose a reason for hiding this comment

Uh oh!

cfmcgrady Jun 10, 2021

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 10, 2021

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jun 10, 2021

Uh oh!

SparkQA commented Jun 10, 2021

Uh oh!

cloud-fan commented Jun 10, 2021

Uh oh!

SparkQA commented Jun 10, 2021

Uh oh!

maropu commented Jun 11, 2021

Uh oh!

AngersZhuuuu commented Nov 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cloud-fan Jun 9, 2021 •

edited

Loading