You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/08/17 06:59:54 UTC
[GitHub] [spark] HyukjinKwon opened a new pull request, #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
HyukjinKwon opened a new pull request, #37548:
URL: https://github.com/apache/spark/pull/37548
### What changes were proposed in this pull request?
This PR is a followup of https://github.com/apache/spark/pull/37503 that adds a check if the `ElementAt` expression is resolved or not before getting its dataType.
### Why are the changes needed?
To make the tests pass with ANSI enabled. Currently it fails (https://github.com/apache/spark/runs/7870131749?check_suite_focus=true) as below:
```
[info] - map_filter *** FAILED *** (243 milliseconds)
[info] org.apache.spark.sql.catalyst.analysis.UnresolvedException: Invalid call to dataType on unresolved object
[info] at org.apache.spark.sql.catalyst.expressions.UnresolvedNamedLambdaVariable.dataType(higherOrderFunctions.scala:46)
[info] at org.apache.spark.sql.catalyst.expressions.ElementAt.initQueryContext(collectionOperations.scala:2275)
[info] at org.apache.spark.sql.catalyst.expressions.SupportQueryContext.$init$(Expression.scala:603)
[info] at org.apache.spark.sql.catalyst.expressions.ElementAt.<init>(collectionOperations.scala:2105)
[info] at org.apache.spark.sql.functions$.element_at(functions.scala:3958)
[info] at org.apache.spark.sql.DataFrameFunctionsSuite.$anonfun$new$452(DataFrameFunctionsSuite.scala:2476)
[info] at org.apache.spark.sql.functions$.createLambda(functions.scala:4029)
[info] at org.apache.spark.sql.functions$.map_filter(functions.scala:4256)
[info] at org.apache.spark.sql.DataFrameFunctionsSuite.$anonfun$new$451(DataFrameFunctionsSuite.scala:2476)
[info] at org.apache.spark.sql.QueryTest.checkAnswer(QueryTest.scala:133)
[info] at org.apache.spark.sql.DataFrameFunctionsSuite.$anonfun$new$445(DataFrameFunctionsSuite.scala:2478)
[info] at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info] at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info] at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info] at org.scalatest.Transformer.apply(Transformer.scala:22)
[info] at org.scalatest.Transformer.apply(Transformer.scala:20)
[info] at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:190)
[info] at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:204)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:188)
```
### Does this PR introduce _any_ user-facing change?
No, test-only.
### How was this patch tested?
Manually tested with ANSI mode enabled.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37548:
URL: https://github.com/apache/spark/pull/37548#issuecomment-1217684403
Tests passed.
Merged to master.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37548:
URL: https://github.com/apache/spark/pull/37548#issuecomment-1217540149
cc @gengliangwang
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37548:
URL: https://github.com/apache/spark/pull/37548#discussion_r947691426
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2272,7 +2272,7 @@ case class ElementAt(
newLeft: Expression, newRight: Expression): ElementAt = copy(left = newLeft, right = newRight)
override def initQueryContext(): Option[SQLQueryContext] = {
- if (failOnError && left.dataType.isInstanceOf[ArrayType]) {
+ if (failOnError && left.resolved && left.dataType.isInstanceOf[ArrayType]) {
Review Comment:
why would we call `initQueryContext` on an unresolved expression?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
URL: https://github.com/apache/spark/pull/37548
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on a diff in pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on code in PR #37548:
URL: https://github.com/apache/spark/pull/37548#discussion_r947528023
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2272,7 +2272,7 @@ case class ElementAt(
newLeft: Expression, newRight: Expression): ElementAt = copy(left = newLeft, right = newRight)
override def initQueryContext(): Option[SQLQueryContext] = {
- if (failOnError && left.dataType.isInstanceOf[ArrayType]) {
+ if (failOnError && left.resolved && left.dataType.isInstanceOf[ArrayType]) {
Review Comment:
This should work. Probably there is a better way.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #37548: [SPARK-40066][SQL][FOLLOW-UP] Check if ElementAt is resolved before getting its dataType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37548:
URL: https://github.com/apache/spark/pull/37548#issuecomment-1217544177
cc @cloud-fan 2
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org