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/06/01 15:35:55 UTC

[GitHub] [spark] cxzl25 opened a new pull request, #36740: [SPARK-39355][SQL] UnresolvedAttribute should only use CatalystSqlParser if name contains dot

cxzl25 opened a new pull request, #36740:
URL: https://github.com/apache/spark/pull/36740

   ### What changes were proposed in this pull request?
   Only if the name of `UnresolvedAttribute` contains dot, try to use `CatalystSqlParser.parseMultipartIdentifier`
   
   ### Why are the changes needed?
   ```sql
   SELECT *
   FROM (
       SELECT '2022-06-01' AS c1
   ) a
   WHERE c1 IN (
       SELECT date_add('2022-06-01', 0)
   );
   ```
   ```
   Error in query:
   mismatched input '(' expecting {<EOF>, '.', '-'}(line 1, pos 8)
   == SQL ==
   date_add(2022-06-01, 0)
   --------^^^ 
   ```
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   add UT
   


-- 
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 #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36740:
URL: https://github.com/apache/spark/pull/36740#discussion_r896290278


##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2176,4 +2176,32 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-39355: Avoid UnresolvedAttribute.apply throwing ParseException") {
+    checkAnswer(
+      sql("""
+            |SELECT *

Review Comment:
   I think we should fix `Alias.toAttribute`. It should create an attribute to reference the output of this alias, which is a single column, and we need to use `UnresolvedAttribute.quoted`.



-- 
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 #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36740:
URL: https://github.com/apache/spark/pull/36740#discussion_r895797089


##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2176,4 +2176,32 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-39355: Avoid UnresolvedAttribute.apply throwing ParseException") {
+    checkAnswer(
+      sql("""
+            |SELECT *

Review Comment:
   I'm surprised this bug causes real query failures. Can you explain how this query triggers `UnresolvedAttribute.apply`?



-- 
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] cxzl25 commented on pull request #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on PR #36740:
URL: https://github.com/apache/spark/pull/36740#issuecomment-1153476515

   Gentle ping @sarutak @cloud-fan @dongjoon-hyun
   This should be a bug, hope to help review.


-- 
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 #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #36740:
URL: https://github.com/apache/spark/pull/36740#discussion_r896841545


##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2176,4 +2176,32 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-39355: Avoid UnresolvedAttribute.apply throwing ParseException") {
+    checkAnswer(
+      sql("""
+            |SELECT *

Review Comment:
   yea we should also use `.quoted` there. good catch!



-- 
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] cxzl25 commented on a diff in pull request #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #36740:
URL: https://github.com/apache/spark/pull/36740#discussion_r896444180


##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2176,4 +2176,32 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-39355: Avoid UnresolvedAttribute.apply throwing ParseException") {
+    checkAnswer(
+      sql("""
+            |SELECT *

Review Comment:
   It seems that there may be a problem here, although I didn't reproduce it using SQL, do I need to fix it?
   
   https://github.com/apache/spark/blob/2349175e1b81b0a61e1ed90c2d051c01cf78de9b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/interfaces.scala#L120
   
   



-- 
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] cxzl25 commented on pull request #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on PR #36740:
URL: https://github.com/apache/spark/pull/36740#issuecomment-1147014470

   @sarutak @cloud-fan @dongjoon-hyun
   
   After the introduction of SPARK-34636, some SQL will fail to parse.


-- 
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 pull request #36740: [SPARK-39355][SQL] Single column uses quoted to construct UnresolvedAttribute

Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #36740:
URL: https://github.com/apache/spark/pull/36740#issuecomment-1156130633

   thanks, merging to master/3.3/3.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


[GitHub] [spark] AmplabJenkins commented on pull request #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36740:
URL: https://github.com/apache/spark/pull/36740#issuecomment-1145590291

   Can one of the admins verify this patch?


-- 
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 closed pull request #36740: [SPARK-39355][SQL] Single column uses quoted to construct UnresolvedAttribute

Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #36740: [SPARK-39355][SQL] Single column uses quoted to construct UnresolvedAttribute
URL: https://github.com/apache/spark/pull/36740


-- 
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] cxzl25 commented on a diff in pull request #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #36740:
URL: https://github.com/apache/spark/pull/36740#discussion_r895854783


##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2176,4 +2176,32 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-39355: Avoid UnresolvedAttribute.apply throwing ParseException") {
+    checkAnswer(
+      sql("""
+            |SELECT *

Review Comment:
   A `Cast` may be generated in `InConversion`, at which time `newSub.output` will call `UnresolvedAttribute.apply`.
   https://github.com/apache/spark/blob/d3d22928d4fca1cd71f96a1308f0cc5d00120ad5/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala#L349-L360



-- 
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] cxzl25 commented on a diff in pull request #36740: [SPARK-39355][SQL] Avoid UnresolvedAttribute.apply throwing ParseException

Posted by GitBox <gi...@apache.org>.
cxzl25 commented on code in PR #36740:
URL: https://github.com/apache/spark/pull/36740#discussion_r895854783


##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -2176,4 +2176,32 @@ class SubquerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
       }
     }
   }
+
+  test("SPARK-39355: Avoid UnresolvedAttribute.apply throwing ParseException") {
+    checkAnswer(
+      sql("""
+            |SELECT *

Review Comment:
   A `Cast` may be generated `InConversion`, at which time `newSub.output` will call `UnresolvedAttribute.apply`.
   https://github.com/apache/spark/blob/d3d22928d4fca1cd71f96a1308f0cc5d00120ad5/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala#L349-L360



-- 
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