[GitHub] [pinot] siddharthteotia commented on a diff in pull request #9605: Fix NonAggregationGroupByToDistinctQueryRewriter

[GitBox <gi...@apache.org>]

siddharthteotia commented on code in PR #9605:
URL: https://github.com/apache/pinot/pull/9605#discussion_r997602000


##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriterTest.java:
##########
@@ -18,155 +18,45 @@
  */
 package org.apache.pinot.sql.parsers.rewriter;
 
-import org.apache.pinot.common.request.PinotQuery;
 import org.apache.pinot.sql.parsers.CalciteSqlParser;
-import org.testng.Assert;
+import org.apache.pinot.sql.parsers.SqlCompilationException;
 import org.testng.annotations.Test;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertThrows;
 
-public class NonAggregationGroupByToDistinctQueryRewriterTest {
 
+public class NonAggregationGroupByToDistinctQueryRewriterTest {
   private static final QueryRewriter QUERY_REWRITER = new NonAggregationGroupByToDistinctQueryRewriter();
 
   @Test
-  public void testQuery1() {
-    final PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery("SELECT A FROM myTable GROUP BY A");
-    QUERY_REWRITER.rewrite(pinotQuery);
-    Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator(), "distinct");
-    Assert.assertEquals(
-        pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(), "A");
-  }
-
-  @Test
-  // SELECT col1+col2*5 FROM foo GROUP BY col1, col2 => SELECT distinct col1+col2*5 FROM foo
-  public void testQuery2() {
-    final PinotQuery pinotQuery =
-        CalciteSqlParser.compileToPinotQuery("SELECT col1+col2*5 FROM foo GROUP BY col1, col2");
-    QUERY_REWRITER.rewrite(pinotQuery);
-    Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator(), "distinct");
-    Assert.assertEquals(
-        pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(0)
-            .getIdentifier().getName(), "col1");
-    Assert.assertEquals(
-        pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(1)
-            .getFunctionCall().getOperands().get(0).getIdentifier().getName(), "col2");
-    Assert.assertEquals(
-        pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(1)
-            .getFunctionCall().getOperands().get(1).getLiteral().getLongValue(), 5L);
+  public void testQueryRewrite() {
+    testQueryRewrite("SELECT A FROM myTable GROUP BY A", "SELECT DISTINCT A FROM myTable");
+    testQueryRewrite("SELECT col1, col2 FROM foo GROUP BY col1, col2", "SELECT DISTINCT col1, col2 FROM foo");

Review Comment:
   How do we handle when the user (intentionally or unintentionally) specifies different order in GROUP BY clause ?
   
   `SELECT col1, col2 FROM FOO GROUP BY col2, col1`



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org