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/02/03 09:37:21 UTC

[GitHub] [spark] SaurabhChawla100 opened a new pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

SaurabhChawla100 opened a new pull request #35389:
URL: https://github.com/apache/spark/pull/35389


   ### What changes were proposed in this pull request?
   Migrate the following errors in QueryCompilationErrors onto use error classes:
   groupingMustWithGroupingSetsOrCubeOrRollupError => INVALID_GROUPING_EXPRESSION
   
   ### Why are the changes needed?
   Porting grouping /grouping Id  errors to new error framework.
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Added the unit test in QueryCompilationErrorsSuite and tested the unit test


-- 
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 #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

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


   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] MaxGekk closed pull request #35389: [SPARK-37943][SQL] Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #35389:
URL: https://github.com/apache/spark/pull/35389


   


-- 
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] amaliujia commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r800139803



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       NVM. `Sort with grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup` is not really more helpful.




-- 
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] SaurabhChawla100 commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
SaurabhChawla100 commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r799240599



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       As per the code `val groupingExprs = findGroupingExprs(child)`  . findGroupingExprs is called from two places
   
   https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L733)
   
    and 
   
   https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L740.
   
   In that case if want to parameterised the error by updating the input param to the method 
   `def findGroupingExprs(plan: LogicalPlan) ` to  `def findGroupingExprs(plan: LogicalPlan, parentInfo: String)`
   
   So that when throwing the exception we can add the where it failed either in filter or sort.
   
   If this looks fine, I can make that change
   




-- 
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] SaurabhChawla100 commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
SaurabhChawla100 commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r799232253



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       If I am understanding correctly So the message should be like this 
   "Sort with grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup" instead of 
   "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup"
   
   Similar case for filter
   "Filter with grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup" instead of 
   "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup"
   




-- 
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] SaurabhChawla100 commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
SaurabhChawla100 commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r799232253



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       If I am understanding correctly So the message should be like this 
   
   ```
   "Sort with grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup" 
      
   instead of 
   
   "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup"
   ```
   
   
   Similar case for filter
   ```
   "Filter with grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup" 
   
   instead of 
   
   "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup"
   ```
   




-- 
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] amaliujia commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r798913158



##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -153,6 +153,9 @@
     "message" : [ "The feature is not supported: %s" ],
     "sqlState" : "0A000"
   },
+  "UNSUPPORTED_GROUPING_EXPRESSION" : {
+    "message" : [ "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup" ]

Review comment:
       I am thinking `GroupingSets/Cube/Rollup` is not easy to understand. Right now I cannot immediately recall what is grouping sets or cube or rollup. 
   
   Is it possible to use other way to explain grouping()/grouping_id()? Like they can only be used in syntax a, b, c 




-- 
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] MaxGekk commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r800224372



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       ```suggestion
       Seq("grouping", "grouping_id").foreach { grouping =>
         val errMsg = intercept[AnalysisException] {
           df.filter(s"$grouping(CustomerId)=17850")
             .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
         }
         assert(errMsg.message ===
           "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
         assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
       }
   ```

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+
+    // Sort with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.sort(grouping_id("CustomerId")).
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       ```suggestion
       Seq(grouping("CustomerId"), grouping_id("CustomerId")).foreach { grouping =>
         val errMsg = intercept[AnalysisException] {
           df.sort(grouping)
             .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
         }
         assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
         assert(errMsg.message ===
           "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
       }
     }
   ```

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+
+    // Sort with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.sort(grouping_id("CustomerId")).
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+

Review comment:
       nit: remove the empty line.




-- 
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] SaurabhChawla100 commented on a change in pull request #35389: [SPARK-37943][SQL] Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
SaurabhChawla100 commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r801278596



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+
+    // Sort with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.sort(grouping_id("CustomerId")).
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       done

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       done

##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+
+    // Sort with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.sort(grouping_id("CustomerId")).
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+

Review comment:
       done




-- 
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] amaliujia commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r800139942



##########
File path: core/src/main/resources/error/error-classes.json
##########
@@ -153,6 +153,9 @@
     "message" : [ "The feature is not supported: %s" ],
     "sqlState" : "0A000"
   },
+  "UNSUPPORTED_GROUPING_EXPRESSION" : {
+    "message" : [ "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup" ]

Review comment:
       `GroupingSets/Cube/Rollup` are actually first class SQL syntax. So saying them in error message should be fine as people can quickly search and then know what `GroupingSets/Cube/Rollup` is.




-- 
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] MaxGekk commented on pull request #35389: [SPARK-37943][SQL] Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #35389:
URL: https://github.com/apache/spark/pull/35389#issuecomment-1032364302


   +1, LGTM. Merging to master.
   Thank you, @SaurabhChawla100 and @amaliujia for 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] amaliujia commented on a change in pull request #35389: [SPARK-37943][SQL]Use error classes in the compilation errors of grouping

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #35389:
URL: https://github.com/apache/spark/pull/35389#discussion_r799103151



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
##########
@@ -77,4 +78,56 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
       " but got: org.apache.spark.sql.types.NumericType"))
   }
 
+  test("UNSUPPORTED_GROUPING_EXPRESSION: filter with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+    // filter with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.filter("grouping(CustomerId)=17850")
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+
+    // filter with grouping_id
+    errMsg = intercept[AnalysisException] {
+      df.filter("grouping_id(CustomerId)=17850").
+        groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")
+  }
+
+  test("UNSUPPORTED_GROUPING_EXPRESSION: Sort with grouping/grouping_Id expression") {
+    val df = Seq(
+      (536361, "85123A", 2, 17850),
+      (536362, "85123B", 4, 17850),
+      (536363, "86123A", 6, 17851)
+    ).toDF("InvoiceNo", "StockCode", "Quantity", "CustomerID")
+
+   // Sort with grouping
+    var errMsg = intercept[AnalysisException] {
+      df.sort(grouping("CustomerId"))
+        .groupBy("CustomerId").agg(Map("Quantity" -> "max"))
+    }
+    assert(errMsg.errorClass === Some("UNSUPPORTED_GROUPING_EXPRESSION"))
+    assert(errMsg.message ===
+      "grouping()/grouping_id() can only be used with GroupingSets/Cube/Rollup")

Review comment:
       Seems like we can already pass a parametrized reason here as we know this is sort with grouping?




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