You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xi...@apache.org on 2021/11/10 01:37:22 UTC
[pinot] branch master updated: Fixing DISTINCT with AS function
(#7678)
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 4f22c86 Fixing DISTINCT with AS function (#7678)
4f22c86 is described below
commit 4f22c8672ce1aafc31e4e806b3bf3b3c596c8b33
Author: Xiang Fu <xi...@gmail.com>
AuthorDate: Tue Nov 9 17:37:10 2021 -0800
Fixing DISTINCT with AS function (#7678)
* fixing distinct with as function
* remove as transform function
* simplify some logic
Co-authored-by: Xiaotian (Jackie) Jiang <ja...@gmail.com>
---
.../apache/pinot/sql/parsers/CalciteSqlParser.java | 16 +-
...nAggregationGroupByToDistinctQueryRewriter.java | 8 +-
.../sql/parsers/rewriter/QueryRewriterFactory.java | 2 +-
.../pinot/sql/parsers/CalciteSqlCompilerTest.java | 32 ++--
...regationGroupByToDistinctQueryRewriterTest.java | 172 +++++++++++++++++++++
.../parsers/rewriter/QueryRewriterFactoryTest.java | 8 +-
.../core/query/reduce/BrokerReduceService.java | 19 ++-
.../BrokerRequestToQueryContextConverter.java | 40 +++--
.../apache/pinot/queries/DistinctQueriesTest.java | 6 +-
.../tests/OfflineClusterIntegrationTest.java | 11 ++
10 files changed, 272 insertions(+), 42 deletions(-)
diff --git a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index 1ffae46..1882a6c 100644
--- a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++ b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -161,7 +161,7 @@ public class CalciteSqlParser {
}
List<Expression> orderByList = pinotQuery.getOrderByList();
if (orderByList != null) {
- List<Expression> distinctExpressions = function.getOperands();
+ List<Expression> distinctExpressions = getAliasLeftExpressionsFromDistinctExpression(function);
for (Expression orderByExpression : orderByList) {
// NOTE: Order-by is always a Function with the ordering of the Expression
if (!distinctExpressions.contains(orderByExpression.getFunctionCall().getOperands().get(0))) {
@@ -173,6 +173,19 @@ public class CalciteSqlParser {
}
}
+ private static List<Expression> getAliasLeftExpressionsFromDistinctExpression(Function function) {
+ List<Expression> operands = function.getOperands();
+ List<Expression> expressions = new ArrayList<>(operands.size());
+ for (Expression operand : operands) {
+ if (isAsFunction(operand)) {
+ expressions.add(operand.getFunctionCall().getOperands().get(0));
+ } else {
+ expressions.add(operand);
+ }
+ }
+ return expressions;
+ }
+
/**
* Check recursively if an expression contains any reference not appearing in the GROUP BY clause.
*/
@@ -636,7 +649,6 @@ public class CalciteSqlParser {
}
}
-
public static String canonicalize(String functionName) {
return StringUtils.remove(functionName, '_').toLowerCase();
}
diff --git a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
index 577c78f..11338bf 100644
--- a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
+++ b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriter.java
@@ -22,7 +22,6 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.Set;
import org.apache.pinot.common.request.Expression;
-import org.apache.pinot.common.request.Function;
import org.apache.pinot.common.request.PinotQuery;
import org.apache.pinot.common.utils.request.RequestUtils;
import org.apache.pinot.sql.parsers.CalciteSqlParser;
@@ -60,12 +59,7 @@ public class NonAggregationGroupByToDistinctQueryRewriter implements QueryRewrit
if (groupByIdentifiers.containsAll(selectIdentifiers)) {
Expression distinctExpression = RequestUtils.getFunctionExpression("DISTINCT");
for (Expression select : pinotQuery.getSelectList()) {
- if (CalciteSqlParser.isAsFunction(select)) {
- Function asFunc = select.getFunctionCall();
- distinctExpression.getFunctionCall().addToOperands(asFunc.getOperands().get(0));
- } else {
- distinctExpression.getFunctionCall().addToOperands(select);
- }
+ distinctExpression.getFunctionCall().addToOperands(select);
}
pinotQuery.setSelectList(Arrays.asList(distinctExpression));
pinotQuery.setGroupByList(Collections.emptyList());
diff --git a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactory.java b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactory.java
index 7b867d7..6591f13 100644
--- a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactory.java
+++ b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactory.java
@@ -36,7 +36,7 @@ public class QueryRewriterFactory {
static final List<String> DEFAULT_QUERY_REWRITERS_CLASS_NAMES =
ImmutableList.of(CompileTimeFunctionsInvoker.class.getName(), SelectionsRewriter.class.getName(),
PredicateComparisonRewriter.class.getName(), OrdinalsUpdater.class.getName(),
- NonAggregationGroupByToDistinctQueryRewriter.class.getName(), AliasApplier.class.getName());
+ AliasApplier.class.getName(), NonAggregationGroupByToDistinctQueryRewriter.class.getName());
public static void init(String queryRewritersClassNamesStr) {
List<String> queryRewritersClassNames =
diff --git a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
index cb25bc8..2cbf021 100644
--- a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
+++ b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
@@ -1677,8 +1677,8 @@ public class CalciteSqlCompilerTest {
+ "1 ASC";
PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
Assert.assertEquals(
- pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
- "baseballStats.playerName");
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(0)
+ .getIdentifier().getName(), "baseballStats.playerName");
Assert.assertTrue(pinotQuery.getGroupByList().isEmpty());
Assert.assertEquals(
pinotQuery.getOrderByList().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
@@ -1988,25 +1988,33 @@ public class CalciteSqlCompilerTest {
Assert.assertEquals(pinotQuery.getSelectListSize(), 1);
Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator().toUpperCase(), "DISTINCT");
Assert.assertEquals(
- pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperator(),
- "PLUS");
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperator()
+ .toUpperCase(), "AS");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(1)
+ .getIdentifier().getName(), "col3");
Assert.assertEquals(
pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(0)
- .getIdentifier().getName(), "col1");
+ .getFunctionCall().getOperator(), "PLUS");
Assert.assertEquals(
- pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(1)
- .getFunctionCall().getOperator(), "TIMES");
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().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");
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(1).getFunctionCall().getOperator(), "TIMES");
Assert.assertEquals(
- pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(1)
- .getFunctionCall().getOperands().get(1).getLiteral().getLongValue(), 5L);
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().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(0)
+ .getFunctionCall().getOperands().get(1).getFunctionCall().getOperands().get(1).getLiteral().getLongValue(),
+ 5L);
Assert.assertEquals(brokerRequest.getAggregationsInfo().size(), 1);
Assert.assertEquals(brokerRequest.getAggregationsInfo().get(0).getAggregationType().toUpperCase(), "DISTINCT");
Assert.assertEquals(brokerRequest.getAggregationsInfo().get(0).getAggregationParams().get("column"),
- "plus(col1,times(col2,'5'))");
+ "as(plus(col1,times(col2,'5')),col3)");
}
@Test(expectedExceptions = SqlCompilationException.class)
diff --git a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriterTest.java b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriterTest.java
new file mode 100644
index 0000000..565461d
--- /dev/null
+++ b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/NonAggregationGroupByToDistinctQueryRewriterTest.java
@@ -0,0 +1,172 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+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.testng.annotations.Test;
+
+
+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);
+ }
+
+ @Test
+ // SELECT col1, col2 FROM foo GROUP BY col1, col2 => SELECT distinct col1, col2 FROM foo
+ public void testQuery3() {
+ final PinotQuery pinotQuery =
+ CalciteSqlParser.compileToPinotQuery("SELECT col1, col2 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).getIdentifier().getName(), "col1");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getIdentifier().getName(), "col2");
+ }
+
+ @Test
+ // SELECT col1 as col2 FROM foo GROUP BY col1 => SELECT distinct col1 as col2 FROM foo
+ public void testQuery4() {
+ final PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery("SELECT col1 as col2 FROM foo GROUP BY col1");
+ QUERY_REWRITER.rewrite(pinotQuery);
+ Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator(), "DISTINCT");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperator(), "AS");
+ 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)
+ .getIdentifier().getName(), "col2");
+ }
+
+ @Test
+ // SELECT col1 as a, col2 as b, concat(col3, col4, '') as c FROM foo GROUP BY a,b,c => SELECT DISTINCT col1 as a,
+ // col2 as b, concat(col3, col4, '') as c FROM foo
+ public void testQuery5() {
+ final PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT col1 as a, col2 as b, concat(col3, col4, '') as c FROM foo GROUP BY a,b,c");
+ QUERY_REWRITER.rewrite(pinotQuery);
+ Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator(), "DISTINCT");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperator(), "AS");
+ 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)
+ .getIdentifier().getName(), "a");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getFunctionCall().getOperator(), "AS");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getFunctionCall().getOperands().get(0)
+ .getIdentifier().getName(), "col2");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getFunctionCall().getOperands().get(1)
+ .getIdentifier().getName(), "b");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperator(), "AS");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperator(), "CONCAT");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(0).getIdentifier().getName(), "col3");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(1).getIdentifier().getName(), "col4");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(2).getLiteral().getStringValue(), "");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(1)
+ .getIdentifier().getName(), "c");
+ }
+
+ @Test
+ // SELECT col1 as a, col2 as b, concat(col3, col4, '') as c FROM foo GROUP BY col1, col2, col3, col4 => SELECT
+ // DISTINCT col1 as a, col2 as b, concat(col3, col4, '') as c FROM foo
+ public void testQuery6() {
+ final PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(
+ "SELECT col1 as a, col2 as b, concat(col3, col4, '') as c FROM foo GROUP BY col1, col2, col3, col4");
+ QUERY_REWRITER.rewrite(pinotQuery);
+ Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator(), "DISTINCT");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(0).getFunctionCall().getOperator(), "AS");
+ 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)
+ .getIdentifier().getName(), "a");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getFunctionCall().getOperator(), "AS");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getFunctionCall().getOperands().get(0)
+ .getIdentifier().getName(), "col2");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(1).getFunctionCall().getOperands().get(1)
+ .getIdentifier().getName(), "b");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperator(), "AS");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperator(), "CONCAT");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(0).getIdentifier().getName(), "col3");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(1).getIdentifier().getName(), "col4");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(0)
+ .getFunctionCall().getOperands().get(2).getLiteral().getStringValue(), "");
+ Assert.assertEquals(
+ pinotQuery.getSelectList().get(0).getFunctionCall().getOperands().get(2).getFunctionCall().getOperands().get(1)
+ .getIdentifier().getName(), "c");
+ }
+}
diff --git a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactoryTest.java b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactoryTest.java
index 32055be..e1e349b 100644
--- a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactoryTest.java
+++ b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/QueryRewriterFactoryTest.java
@@ -36,8 +36,8 @@ public class QueryRewriterFactoryTest {
Assert.assertTrue(QUERY_REWRITERS.get(1) instanceof SelectionsRewriter);
Assert.assertTrue(QUERY_REWRITERS.get(2) instanceof PredicateComparisonRewriter);
Assert.assertTrue(QUERY_REWRITERS.get(3) instanceof OrdinalsUpdater);
- Assert.assertTrue(QUERY_REWRITERS.get(4) instanceof NonAggregationGroupByToDistinctQueryRewriter);
- Assert.assertTrue(QUERY_REWRITERS.get(5) instanceof AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(4) instanceof AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(5) instanceof NonAggregationGroupByToDistinctQueryRewriter);
// Check init with other configs
QueryRewriterFactory.init("org.apache.pinot.sql.parsers.rewriter.PredicateComparisonRewriter,"
@@ -55,7 +55,7 @@ public class QueryRewriterFactoryTest {
Assert.assertTrue(QUERY_REWRITERS.get(1) instanceof SelectionsRewriter);
Assert.assertTrue(QUERY_REWRITERS.get(2) instanceof PredicateComparisonRewriter);
Assert.assertTrue(QUERY_REWRITERS.get(3) instanceof OrdinalsUpdater);
- Assert.assertTrue(QUERY_REWRITERS.get(4) instanceof NonAggregationGroupByToDistinctQueryRewriter);
- Assert.assertTrue(QUERY_REWRITERS.get(5) instanceof AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(4) instanceof AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(5) instanceof NonAggregationGroupByToDistinctQueryRewriter);
}
}
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
index 2ec8b44..77aae3d 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/reduce/BrokerReduceService.java
@@ -97,7 +97,7 @@ public class BrokerReduceService {
}
String[] columnNames = resultTable.getDataSchema().getColumnNames();
- List<ExpressionContext> selectExpressions = queryContext.getSelectExpressions();
+ List<ExpressionContext> selectExpressions = getSelectExpressions(queryContext.getSelectExpressions());
int numSelectExpressions = selectExpressions.size();
// For query like `SELECT *`, we skip alias update.
if (columnNames.length != numSelectExpressions) {
@@ -111,6 +111,15 @@ public class BrokerReduceService {
}
}
+ private static List<ExpressionContext> getSelectExpressions(List<ExpressionContext> selectExpressions) {
+ // NOTE: For DISTINCT queries, need to extract the arguments as the SELECT expressions
+ if (selectExpressions.size() == 1 && selectExpressions.get(0).getType() == ExpressionContext.Type.FUNCTION
+ && selectExpressions.get(0).getFunction().getFunctionName().equals("distinct")) {
+ return selectExpressions.get(0).getFunction().getArguments();
+ }
+ return selectExpressions;
+ }
+
public BrokerResponseNative reduceOnDataTable(BrokerRequest brokerRequest,
Map<ServerRoutingInstance, DataTable> dataTableMap, long reduceTimeOutMs, @Nullable BrokerMetrics brokerMetrics) {
if (dataTableMap.isEmpty()) {
@@ -252,10 +261,10 @@ public class BrokerReduceService {
String rawTableName = TableNameBuilder.extractRawTableName(tableName);
if (brokerMetrics != null) {
brokerMetrics.addMeteredTableValue(rawTableName, BrokerMeter.DOCUMENTS_SCANNED, numDocsScanned);
- brokerMetrics
- .addMeteredTableValue(rawTableName, BrokerMeter.ENTRIES_SCANNED_IN_FILTER, numEntriesScannedInFilter);
- brokerMetrics
- .addMeteredTableValue(rawTableName, BrokerMeter.ENTRIES_SCANNED_POST_FILTER, numEntriesScannedPostFilter);
+ brokerMetrics.addMeteredTableValue(rawTableName, BrokerMeter.ENTRIES_SCANNED_IN_FILTER,
+ numEntriesScannedInFilter);
+ brokerMetrics.addMeteredTableValue(rawTableName, BrokerMeter.ENTRIES_SCANNED_POST_FILTER,
+ numEntriesScannedPostFilter);
brokerMetrics.addTimedTableValue(rawTableName, BrokerTimer.OFFLINE_THREAD_CPU_TIME_NS, offlineThreadCpuTimeNs,
TimeUnit.NANOSECONDS);
brokerMetrics.addTimedTableValue(rawTableName, BrokerTimer.REALTIME_THREAD_CPU_TIME_NS, realtimeThreadCpuTimeNs,
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverter.java b/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverter.java
index e030f4c..35ac4e2 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverter.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverter.java
@@ -65,19 +65,39 @@ public class BrokerRequestToQueryContextConverter {
List<String> aliasList = new ArrayList<>(selectList.size());
selectExpressions = new ArrayList<>(selectList.size());
for (Expression thriftExpression : selectList) {
- ExpressionContext expression;
- if (thriftExpression.getType() == ExpressionType.FUNCTION && thriftExpression.getFunctionCall().getOperator()
- .equalsIgnoreCase("AS")) {
- // Handle alias
- List<Expression> operands = thriftExpression.getFunctionCall().getOperands();
- expression = RequestContextUtils.getExpression(operands.get(0));
- aliasList.add(operands.get(1).getIdentifier().getName());
+ // Handle alias
+ Expression expressionWithoutAlias = thriftExpression;
+ if (thriftExpression.getType() == ExpressionType.FUNCTION) {
+ Function function = thriftExpression.getFunctionCall();
+ List<Expression> operands = function.getOperands();
+ switch (function.getOperator().toUpperCase()) {
+ case "AS":
+ expressionWithoutAlias = operands.get(0);
+ aliasList.add(operands.get(1).getIdentifier().getName());
+ break;
+ case "DISTINCT":
+ int numOperands = operands.size();
+ for (int i = 0; i < numOperands; i++) {
+ Expression operand = operands.get(i);
+ Function operandFunction = operand.getFunctionCall();
+ if (operandFunction != null && operandFunction.getOperator().equalsIgnoreCase("AS")) {
+ operands.set(i, operandFunction.getOperands().get(0));
+ aliasList.add(operandFunction.getOperands().get(1).getIdentifier().getName());
+ } else {
+ aliasList.add(null);
+ }
+ }
+ break;
+ default:
+ // Add null as a placeholder for alias.
+ aliasList.add(null);
+ break;
+ }
} else {
- expression = RequestContextUtils.getExpression(thriftExpression);
- // Add null as a place holder for alias.
+ // Add null as a placeholder for alias.
aliasList.add(null);
}
- selectExpressions.add(expression);
+ selectExpressions.add(RequestContextUtils.getExpression(expressionWithoutAlias));
}
// WHERE
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
index 8708a5f..8a9f813 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
@@ -1005,6 +1005,7 @@ public class DistinctQueriesTest extends BaseQueriesTest {
+ "ORDER BY stringColumn DESC, ADD(intColumn, floatColumn) ASC LIMIT 10",
"SELECT DISTINCT(floatColumn, longColumn) FROM testTable WHERE stringColumn = 'a' ORDER BY longColumn LIMIT 10",
"SELECT DISTINCT(intColumn) FROM testTable WHERE floatColumn > 200 ORDER BY intColumn ASC LIMIT 5",
+ "SELECT DISTINCT(longColumn) FROM testTable WHERE doubleColumn < 200 ORDER BY longColumn DESC LIMIT 5",
"SELECT DISTINCT(longColumn) FROM testTable WHERE doubleColumn < 200 ORDER BY longColumn DESC LIMIT 5"
};
String[] sqlQueries = new String[]{
@@ -1020,7 +1021,10 @@ public class DistinctQueriesTest extends BaseQueriesTest {
"SELECT floatColumn, longColumn FROM testTable WHERE stringColumn = 'a' "
+ "GROUP BY floatColumn, longColumn ORDER BY longColumn LIMIT 10",
"SELECT intColumn FROM testTable WHERE floatColumn > 200 GROUP BY intColumn ORDER BY intColumn ASC LIMIT 5",
- "SELECT longColumn FROM testTable WHERE doubleColumn < 200 GROUP BY longColumn ORDER BY longColumn DESC LIMIT 5"
+ "SELECT longColumn FROM testTable WHERE doubleColumn < 200 GROUP BY longColumn"
+ + " ORDER BY longColumn DESC LIMIT 5",
+ "SELECT longColumn as lc FROM testTable WHERE doubleColumn < 200 GROUP BY longColumn"
+ + " ORDER BY longColumn DESC LIMIT 5"
};
//@formatter:on
testDistinctInterSegmentHelper(pqlQueries, sqlQueries);
diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
index 641e3b9..609b26d 100644
--- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
+++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
@@ -1476,6 +1476,17 @@ public class OfflineClusterIntegrationTest extends BaseClusterIntegrationTestSet
+ "CarrierName ORDER BY cnt2";
testSqlQuery(query, Collections.singletonList(query));
}
+ {
+ //test alias with distinct
+ String query =
+ "SELECT distinct ArrTime, Carrier, Carrier AS CarrierName1, Carrier AS CarrierName2, DaysSinceEpoch FROM "
+ + "mytable ORDER BY DaysSinceEpoch DESC";
+ testSqlQuery(query, Collections.singletonList(query));
+
+ query = "SELECT ArrTime, Carrier, Carrier AS CarrierName1, Carrier AS CarrierName2, DaysSinceEpoch FROM mytable "
+ + "GROUP BY ArrTime, Carrier, DaysSinceEpoch ORDER BY DaysSinceEpoch DESC";
+ testSqlQuery(query, Collections.singletonList(query));
+ }
}
@AfterClass
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org