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 e1236429a7 Fix the alias handling in single-stage engine (#11610)
e1236429a7 is described below
commit e1236429a709b5af3012bc92f51cc4fbcb12741d
Author: Xiaotian (Jackie) Jiang <17555551+jackie-ji...@users.noreply.github.com>
AuthorDate: Mon Sep 18 22:22:01 2023 -0700
Fix the alias handling in single-stage engine (#11610)
* Fix the alias handling in single-stage engine
* Throw exception when alias is used in filter clause
---------
Co-authored-by: Xiang Fu <xiangfu.1...@gmail.com>
---
.../requesthandler/BaseBrokerRequestHandler.java | 38 +++++----------
.../MultiStageBrokerRequestHandler.java | 5 ++
.../BaseBrokerRequestHandlerTest.java | 18 +++----
.../pinot/sql/parsers/rewriter/AliasApplier.java | 52 ++++++--------------
.../sql/parsers/rewriter/QueryRewriterFactory.java | 8 ++-
.../pinot/sql/parsers/CalciteSqlCompilerTest.java | 57 ++++++++++++----------
.../parsers/rewriter/QueryRewriterFactoryTest.java | 11 ++---
.../realtime/PinotLLCRealtimeSegmentManager.java | 13 ++---
.../BrokerRequestToQueryContextConverterTest.java | 3 +-
.../pinot/queries/MultiValueRawQueriesTest.java | 11 +++--
.../tests/OfflineClusterIntegrationTest.java | 18 ++++++-
.../integration/tests/custom/JsonPathTest.java | 2 +-
12 files changed, 111 insertions(+), 125 deletions(-)
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
index 185e3d5f62..f0b30544f4 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
@@ -25,7 +25,6 @@ import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -1399,11 +1398,10 @@ public abstract class BaseBrokerRequestHandler
implements BrokerRequestHandler {
@VisibleForTesting
static void updateColumnNames(String rawTableName, PinotQuery pinotQuery,
boolean isCaseInsensitive,
Map<String, String> columnNameMap) {
- Map<String, String> aliasMap = new HashMap<>();
if (pinotQuery != null) {
boolean hasStar = false;
for (Expression expression : pinotQuery.getSelectList()) {
- fixColumnName(rawTableName, expression, columnNameMap, aliasMap,
isCaseInsensitive);
+ fixColumnName(rawTableName, expression, columnNameMap,
isCaseInsensitive);
//check if the select expression is '*'
if (!hasStar && expression.equals(STAR)) {
hasStar = true;
@@ -1415,25 +1413,26 @@ public abstract class BaseBrokerRequestHandler
implements BrokerRequestHandler {
}
Expression filterExpression = pinotQuery.getFilterExpression();
if (filterExpression != null) {
- fixColumnName(rawTableName, filterExpression, columnNameMap, aliasMap,
isCaseInsensitive);
+ // We don't support alias in filter expression, so we don't need to
pass aliasMap
+ fixColumnName(rawTableName, filterExpression, columnNameMap,
isCaseInsensitive);
}
List<Expression> groupByList = pinotQuery.getGroupByList();
if (groupByList != null) {
for (Expression expression : groupByList) {
- fixColumnName(rawTableName, expression, columnNameMap, aliasMap,
isCaseInsensitive);
+ fixColumnName(rawTableName, expression, columnNameMap,
isCaseInsensitive);
}
}
List<Expression> orderByList = pinotQuery.getOrderByList();
if (orderByList != null) {
for (Expression expression : orderByList) {
// NOTE: Order-by is always a Function with the ordering of the
Expression
- fixColumnName(rawTableName,
expression.getFunctionCall().getOperands().get(0), columnNameMap, aliasMap,
+ fixColumnName(rawTableName,
expression.getFunctionCall().getOperands().get(0), columnNameMap,
isCaseInsensitive);
}
}
Expression havingExpression = pinotQuery.getHavingExpression();
if (havingExpression != null) {
- fixColumnName(rawTableName, havingExpression, columnNameMap, aliasMap,
isCaseInsensitive);
+ fixColumnName(rawTableName, havingExpression, columnNameMap,
isCaseInsensitive);
}
}
}
@@ -1466,32 +1465,23 @@ public abstract class BaseBrokerRequestHandler
implements BrokerRequestHandler {
* Fixes the column names to the actual column names in the given expression.
*/
private static void fixColumnName(String rawTableName, Expression
expression, Map<String, String> columnNameMap,
- Map<String, String> aliasMap, boolean ignoreCase) {
+ boolean ignoreCase) {
ExpressionType expressionType = expression.getType();
if (expressionType == ExpressionType.IDENTIFIER) {
Identifier identifier = expression.getIdentifier();
- identifier.setName(getActualColumnName(rawTableName,
identifier.getName(), columnNameMap, aliasMap, ignoreCase));
+ identifier.setName(getActualColumnName(rawTableName,
identifier.getName(), columnNameMap, ignoreCase));
} else if (expressionType == ExpressionType.FUNCTION) {
final Function functionCall = expression.getFunctionCall();
switch (functionCall.getOperator()) {
case "as":
- fixColumnName(rawTableName, functionCall.getOperands().get(0),
columnNameMap, aliasMap, ignoreCase);
- final Expression rightAsExpr = functionCall.getOperands().get(1);
- if (rightAsExpr.isSetIdentifier()) {
- String rightColumn = rightAsExpr.getIdentifier().getName();
- if (ignoreCase) {
- aliasMap.put(rightColumn.toLowerCase(), rightColumn);
- } else {
- aliasMap.put(rightColumn, rightColumn);
- }
- }
+ fixColumnName(rawTableName, functionCall.getOperands().get(0),
columnNameMap, ignoreCase);
break;
case "lookup":
// LOOKUP function looks up another table's schema, skip the check
for now.
break;
default:
for (Expression operand : functionCall.getOperands()) {
- fixColumnName(rawTableName, operand, columnNameMap, aliasMap,
ignoreCase);
+ fixColumnName(rawTableName, operand, columnNameMap, ignoreCase);
}
break;
}
@@ -1505,7 +1495,7 @@ public abstract class BaseBrokerRequestHandler implements
BrokerRequestHandler {
*/
@VisibleForTesting
static String getActualColumnName(String rawTableName, String columnName,
@Nullable Map<String, String> columnNameMap,
- @Nullable Map<String, String> aliasMap, boolean ignoreCase) {
+ boolean ignoreCase) {
if ("*".equals(columnName)) {
return columnName;
}
@@ -1523,12 +1513,6 @@ public abstract class BaseBrokerRequestHandler
implements BrokerRequestHandler {
return actualColumnName;
}
}
- if (aliasMap != null) {
- String actualAlias = aliasMap.get(columnNameToCheck);
- if (actualAlias != null) {
- return actualAlias;
- }
- }
if (columnName.charAt(0) == '$') {
return columnName;
}
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
index 2befc73535..e352ce906e 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java
@@ -140,6 +140,11 @@ public class MultiStageBrokerRequestHandler extends
BaseBrokerRequestHandler {
String consolidatedMessage =
ExceptionUtils.consolidateExceptionMessages(e);
LOGGER.warn("Caught exception planning request {}: {}, {}", requestId,
query, consolidatedMessage);
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.REQUEST_COMPILATION_EXCEPTIONS,
1);
+ if (e.getMessage().matches(".* Column .* not found in any table'")) {
+ requestContext.setErrorCode(QueryException.UNKNOWN_COLUMN_ERROR_CODE);
+ return new BrokerResponseNative(
+ QueryException.getException(QueryException.UNKNOWN_COLUMN_ERROR,
consolidatedMessage));
+ }
requestContext.setErrorCode(QueryException.QUERY_PLANNING_ERROR_CODE);
return new BrokerResponseNative(
QueryException.getException(QueryException.QUERY_PLANNING_ERROR,
consolidatedMessage));
diff --git
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandlerTest.java
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandlerTest.java
index 03d7ae7525..f2ee4cd03e 100644
---
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandlerTest.java
+++
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandlerTest.java
@@ -88,11 +88,11 @@ public class BaseBrokerRequestHandlerTest {
Map<String, String> columnNameMap = new HashMap<>();
columnNameMap.put("student_name", "student_name");
String actualColumnName =
- BaseBrokerRequestHandler.getActualColumnName("mytable",
"mytable.student_name", columnNameMap, null, false);
+ BaseBrokerRequestHandler.getActualColumnName("mytable",
"mytable.student_name", columnNameMap, false);
Assert.assertEquals(actualColumnName, "student_name");
boolean exceptionThrown = false;
try {
- BaseBrokerRequestHandler.getActualColumnName("mytable",
"mytable2.student_name", columnNameMap, null, false);
+ BaseBrokerRequestHandler.getActualColumnName("mytable",
"mytable2.student_name", columnNameMap, false);
Assert.fail("should throw exception if column is not known");
} catch (BadQueryRequestException ex) {
exceptionThrown = true;
@@ -100,7 +100,7 @@ public class BaseBrokerRequestHandlerTest {
Assert.assertTrue(exceptionThrown, "should throw exception if column is
not known");
exceptionThrown = false;
try {
- BaseBrokerRequestHandler.getActualColumnName("mytable",
"MYTABLE.student_name", columnNameMap, null, false);
+ BaseBrokerRequestHandler.getActualColumnName("mytable",
"MYTABLE.student_name", columnNameMap, false);
Assert.fail("should throw exception if case sensitive and table name
different");
} catch (BadQueryRequestException ex) {
exceptionThrown = true;
@@ -108,12 +108,12 @@ public class BaseBrokerRequestHandlerTest {
Assert.assertTrue(exceptionThrown, "should throw exception if column is
not known");
columnNameMap.put("mytable_student_name", "mytable_student_name");
String wrongColumnName2 =
- BaseBrokerRequestHandler.getActualColumnName("mytable",
"mytable_student_name", columnNameMap, null, false);
+ BaseBrokerRequestHandler.getActualColumnName("mytable",
"mytable_student_name", columnNameMap, false);
Assert.assertEquals(wrongColumnName2, "mytable_student_name");
columnNameMap.put("mytable", "mytable");
String wrongColumnName3 =
- BaseBrokerRequestHandler.getActualColumnName("mytable", "mytable",
columnNameMap, null, false);
+ BaseBrokerRequestHandler.getActualColumnName("mytable", "mytable",
columnNameMap, false);
Assert.assertEquals(wrongColumnName3, "mytable");
}
@@ -122,11 +122,11 @@ public class BaseBrokerRequestHandlerTest {
Map<String, String> columnNameMap = new HashMap<>();
columnNameMap.put("student_name", "student_name");
String actualColumnName =
- BaseBrokerRequestHandler.getActualColumnName("mytable",
"MYTABLE.student_name", columnNameMap, null, true);
+ BaseBrokerRequestHandler.getActualColumnName("mytable",
"MYTABLE.student_name", columnNameMap, true);
Assert.assertEquals(actualColumnName, "student_name");
boolean exceptionThrown = false;
try {
- BaseBrokerRequestHandler.getActualColumnName("student",
"MYTABLE2.student_name", columnNameMap, null, true);
+ BaseBrokerRequestHandler.getActualColumnName("student",
"MYTABLE2.student_name", columnNameMap, true);
Assert.fail("should throw exception if column is not known");
} catch (BadQueryRequestException ex) {
exceptionThrown = true;
@@ -134,12 +134,12 @@ public class BaseBrokerRequestHandlerTest {
Assert.assertTrue(exceptionThrown, "should throw exception if column is
not known");
columnNameMap.put("mytable_student_name", "mytable_student_name");
String wrongColumnName2 =
- BaseBrokerRequestHandler.getActualColumnName("mytable",
"MYTABLE_student_name", columnNameMap, null, true);
+ BaseBrokerRequestHandler.getActualColumnName("mytable",
"MYTABLE_student_name", columnNameMap, true);
Assert.assertEquals(wrongColumnName2, "mytable_student_name");
columnNameMap.put("mytable", "mytable");
String wrongColumnName3 =
- BaseBrokerRequestHandler.getActualColumnName("MYTABLE", "mytable",
columnNameMap, null, true);
+ BaseBrokerRequestHandler.getActualColumnName("MYTABLE", "mytable",
columnNameMap, true);
Assert.assertEquals(wrongColumnName3, "mytable");
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/AliasApplier.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/AliasApplier.java
index 5a80914ff5..de8d06f338 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/AliasApplier.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/AliasApplier.java
@@ -19,10 +19,8 @@
package org.apache.pinot.sql.parsers.rewriter;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import org.apache.pinot.common.request.Expression;
import org.apache.pinot.common.request.Function;
import org.apache.pinot.common.request.Identifier;
@@ -31,38 +29,30 @@ import org.apache.pinot.sql.parsers.SqlCompilationException;
public class AliasApplier implements QueryRewriter {
+
@Override
public PinotQuery rewrite(PinotQuery pinotQuery) {
-
- // Update alias
- Map<Identifier, Expression> aliasMap =
extractAlias(pinotQuery.getSelectList());
+ Map<String, Expression> aliasMap =
extractAlias(pinotQuery.getSelectList());
applyAlias(aliasMap, pinotQuery);
-
- // Validate
- validateSelectionClause(aliasMap, pinotQuery);
return pinotQuery;
}
- private static Map<Identifier, Expression> extractAlias(List<Expression>
expressions) {
- Map<Identifier, Expression> aliasMap = new HashMap<>();
- for (Expression expression : expressions) {
- Function functionCall = expression.getFunctionCall();
- if (functionCall == null) {
+ private static Map<String, Expression> extractAlias(List<Expression>
selectExpressions) {
+ Map<String, Expression> aliasMap = new HashMap<>();
+ for (Expression expression : selectExpressions) {
+ Function function = expression.getFunctionCall();
+ if (function == null || !function.getOperator().equals("as")) {
continue;
}
- if (functionCall.getOperator().equals("as")) {
- Expression identifierExpr = functionCall.getOperands().get(1);
- aliasMap.put(identifierExpr.getIdentifier(),
functionCall.getOperands().get(0));
+ String alias = function.getOperands().get(1).getIdentifier().getName();
+ if (aliasMap.put(alias, function.getOperands().get(0)) != null) {
+ throw new SqlCompilationException("Find duplicate alias: " + alias);
}
}
return aliasMap;
}
- private static void applyAlias(Map<Identifier, Expression> aliasMap,
PinotQuery pinotQuery) {
- Expression filterExpression = pinotQuery.getFilterExpression();
- if (filterExpression != null) {
- applyAlias(aliasMap, filterExpression);
- }
+ private static void applyAlias(Map<String, Expression> aliasMap, PinotQuery
pinotQuery) {
List<Expression> groupByList = pinotQuery.getGroupByList();
if (groupByList != null) {
for (Expression expression : groupByList) {
@@ -81,10 +71,10 @@ public class AliasApplier implements QueryRewriter {
}
}
- private static void applyAlias(Map<Identifier, Expression> aliasMap,
Expression expression) {
- Identifier identifierKey = expression.getIdentifier();
- if (identifierKey != null) {
- Expression aliasExpression = aliasMap.get(identifierKey);
+ private static void applyAlias(Map<String, Expression> aliasMap, Expression
expression) {
+ Identifier identifier = expression.getIdentifier();
+ if (identifier != null) {
+ Expression aliasExpression = aliasMap.get(identifier.getName());
if (aliasExpression != null) {
expression.setType(aliasExpression.getType());
expression.setIdentifier(aliasExpression.getIdentifier());
@@ -100,16 +90,4 @@ public class AliasApplier implements QueryRewriter {
}
}
}
-
- private static void validateSelectionClause(Map<Identifier, Expression>
aliasMap, PinotQuery pinotQuery)
- throws SqlCompilationException {
- // Sanity check on selection expression shouldn't use alias reference.
- Set<String> aliasKeys = new HashSet<>();
- for (Identifier identifier : aliasMap.keySet()) {
- String aliasName = identifier.getName().toLowerCase();
- if (!aliasKeys.add(aliasName)) {
- throw new SqlCompilationException("Duplicated alias name found.");
- }
- }
- }
}
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 ef36ee1080..4bd2abf609 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
@@ -33,10 +33,14 @@ public class QueryRewriterFactory {
private static final Logger LOGGER =
LoggerFactory.getLogger(QueryRewriterFactory.class);
+ // NOTE:
+ // OrdinalsUpdater must be applied after AliasApplier because
OrdinalsUpdater can put the select expression
+ // (reference) into the group-by list, but the alias should not be applied
to the reference.
+ // E.g. SELECT a + 1 AS a FROM table GROUP BY 1
public static final List<String> DEFAULT_QUERY_REWRITERS_CLASS_NAMES =
ImmutableList.of(CompileTimeFunctionsInvoker.class.getName(),
SelectionsRewriter.class.getName(),
- PredicateComparisonRewriter.class.getName(),
OrdinalsUpdater.class.getName(),
- AliasApplier.class.getName(),
NonAggregationGroupByToDistinctQueryRewriter.class.getName());
+ PredicateComparisonRewriter.class.getName(),
AliasApplier.class.getName(), OrdinalsUpdater.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 46aac8c898..02f110603f 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
@@ -1418,16 +1418,11 @@ public class CalciteSqlCompilerTest {
+ " limit 50";
pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
Assert.assertEquals(pinotQuery.getSelectListSize(), 3);
+ // Alias should not be applied to filter
Assert.assertEquals(pinotQuery.getFilterExpression().getFunctionCall().getOperator(),
FilterKind.EQUALS.name());
Assert.assertEquals(
-
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(0).getFunctionCall().getOperator(),
- "divide");
- Assert.assertEquals(
-
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(0)
- .getIdentifier().getName(), "secondsSinceEpoch");
- Assert.assertEquals(
-
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(1)
- .getLiteral().getLongValue(), 86400);
+
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(0).getIdentifier().getName(),
+ "daysSinceEpoch");
Assert.assertEquals(
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(1).getLiteral().getLongValue(),
18523);
Assert.assertEquals(pinotQuery.getGroupByListSize(), 1);
@@ -1441,7 +1436,7 @@ public class CalciteSqlCompilerTest {
// Invalid groupBy clause shouldn't contain aggregate expression, like
sum(rsvp_count), count(*).
try {
- sql = "select sum(rsvp_count), count(*) as cnt from meetupRsvp group by
group_country, cnt limit 50";
+ sql = "select sum(rsvp_count), count(*) as cnt from meetupRsvp group by
group_country, cnt limit 50";
CalciteSqlParser.compileToPinotQuery(sql);
Assert.fail("Query should have failed compilation");
} catch (Exception e) {
@@ -1452,10 +1447,9 @@ public class CalciteSqlCompilerTest {
@Test
public void testAliasInSelection() {
- String sql;
- PinotQuery pinotQuery;
- sql = "SELECT C1 AS ALIAS_C1, C2 AS ALIAS_C2, ADD(C1, C2) FROM Foo";
- pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+ // Alias should not be applied
+ String sql = "SELECT C1 AS ALIAS_C1, C2 AS ALIAS_C2, ALIAS_C1 + ALIAS_C2
FROM Foo";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
Assert.assertEquals(pinotQuery.getSelectListSize(), 3);
Assert.assertEquals(pinotQuery.getSelectList().get(0).getFunctionCall().getOperator(),
"as");
Assert.assertEquals(
@@ -1469,19 +1463,11 @@ public class CalciteSqlCompilerTest {
Assert.assertEquals(
pinotQuery.getSelectList().get(1).getFunctionCall().getOperands().get(1).getIdentifier().getName(),
"ALIAS_C2");
-
Assert.assertEquals(pinotQuery.getSelectList().get(2).getFunctionCall().getOperator(),
"add");
+
Assert.assertEquals(pinotQuery.getSelectList().get(2).getFunctionCall().getOperator(),
"plus");
Assert.assertEquals(
-
pinotQuery.getSelectList().get(2).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"C1");
+
pinotQuery.getSelectList().get(2).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"ALIAS_C1");
Assert.assertEquals(
-
pinotQuery.getSelectList().get(2).getFunctionCall().getOperands().get(1).getIdentifier().getName(),
"C2");
-
- // Invalid groupBy clause shouldn't contain aggregate expression, like
sum(rsvp_count), count(*).
- try {
- sql = "SELECT C1 AS ALIAS_C1, C2 AS ALIAS_C2, ADD(alias_c1, alias_c2)
FROM Foo";
- CalciteSqlParser.compileToPinotQuery(sql);
- } catch (Exception e) {
- Assert.fail("Query compilation shouldn't fail");
- }
+
pinotQuery.getSelectList().get(2).getFunctionCall().getOperands().get(1).getIdentifier().getName(),
"ALIAS_C2");
}
@Test
@@ -1495,6 +1481,26 @@ public class CalciteSqlCompilerTest {
Assert.assertEquals(pinotQuery.getSelectList().get(1).getIdentifier().getName(),
"C2");
}
+ @Test
+ public void testAliasInFilter() {
+ // Alias should not be applied
+ String sql = "SELECT C1 AS ALIAS_CI FROM Foo WHERE ALIAS_CI > 10";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+ Assert.assertEquals(
+
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"ALIAS_CI");
+ }
+
+ @Test
+ public void testColumnOverride() {
+ String sql = "SELECT C1 + 1 AS C1, COUNT(*) AS cnt FROM Foo GROUP BY 1";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+
Assert.assertEquals(pinotQuery.getGroupByList().get(0).getFunctionCall().getOperator(),
"plus");
+ Assert.assertEquals(
+
pinotQuery.getGroupByList().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
"C1");
+ Assert.assertEquals(
+
pinotQuery.getGroupByList().get(0).getFunctionCall().getOperands().get(1).getLiteral().getLongValue(),
1);
+ }
+
@Test
public void testArithmeticOperator() {
PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery("select
a,b+2,c*5,(d+5)*2 from myTable");
@@ -3124,8 +3130,7 @@ public class CalciteSqlCompilerTest {
right = join.getRight();
Assert.assertEquals(right.getTableName(), "self");
rightSubquery = right.getSubquery();
- Assert.assertEquals(rightSubquery,
- CalciteSqlParser.compileToPinotQuery("SELECT key FROM T1"));
+ Assert.assertEquals(rightSubquery,
CalciteSqlParser.compileToPinotQuery("SELECT key FROM T1"));
Assert.assertEquals(join.getCondition(),
CalciteSqlParser.compileToExpression("T1.key = self.key"));
}
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 e1e349b421..7288c1f843 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
@@ -27,16 +27,15 @@ import static
org.apache.pinot.sql.parsers.CalciteSqlParser.QUERY_REWRITERS;
public class QueryRewriterFactoryTest {
@Test
- public void testQueryRewriters()
- throws ReflectiveOperationException {
+ public void testQueryRewriters() {
// Default behavior
QueryRewriterFactory.init(null);
Assert.assertEquals(QUERY_REWRITERS.size(), 6);
Assert.assertTrue(QUERY_REWRITERS.get(0) instanceof
CompileTimeFunctionsInvoker);
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 AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(3) instanceof AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(4) instanceof OrdinalsUpdater);
Assert.assertTrue(QUERY_REWRITERS.get(5) instanceof
NonAggregationGroupByToDistinctQueryRewriter);
// Check init with other configs
@@ -54,8 +53,8 @@ public class QueryRewriterFactoryTest {
Assert.assertTrue(QUERY_REWRITERS.get(0) instanceof
CompileTimeFunctionsInvoker);
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 AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(3) instanceof AliasApplier);
+ Assert.assertTrue(QUERY_REWRITERS.get(4) instanceof OrdinalsUpdater);
Assert.assertTrue(QUERY_REWRITERS.get(5) instanceof
NonAggregationGroupByToDistinctQueryRewriter);
}
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
index 971dd4333d..a29910ea09 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
@@ -1468,9 +1468,7 @@ public class PinotLLCRealtimeSegmentManager {
return 0L;
}
- if (!isLowLevelConsumer(tableNameWithType, tableConfig)
- || !getIsSplitCommitEnabled()
- || !isTmpSegmentAsyncDeletionEnabled()) {
+ if (!getIsSplitCommitEnabled() || !isTmpSegmentAsyncDeletionEnabled()) {
return 0L;
}
@@ -1503,7 +1501,8 @@ public class PinotLLCRealtimeSegmentManager {
return deletedTmpSegments;
}
- private boolean isTmpAndCanDelete(URI uri, Set<String> deepURIs, PinotFS
pinotFS) throws Exception {
+ private boolean isTmpAndCanDelete(URI uri, Set<String> deepURIs, PinotFS
pinotFS)
+ throws Exception {
long lastModified = pinotFS.lastModified(uri);
if (lastModified <= 0) {
LOGGER.warn("file {} modification time {} is not positive, ineligible
for delete", uri.toString(), lastModified);
@@ -1514,12 +1513,6 @@ public class PinotLLCRealtimeSegmentManager {
&& getCurrentTimeMs() - lastModified >
_controllerConf.getTmpSegmentRetentionInSeconds() * 1000L;
}
- private boolean isLowLevelConsumer(String tableNameWithType, TableConfig
tableConfig) {
- PartitionLevelStreamConfig streamConfig = new
PartitionLevelStreamConfig(tableNameWithType,
- IngestionConfigUtils.getStreamConfigMap(tableConfig));
- return streamConfig.hasLowLevelConsumerType();
- }
-
/**
* Force commit the current segments in consuming state and restart
consumption
*/
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java
index 0fdab1c04e..866282478e 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java
@@ -321,7 +321,8 @@ public class BrokerRequestToQueryContextConverterTest {
// Alias
// NOTE: All the references to the alias should already be converted to
the original expressions.
{
- String query = "SELECT SUM(foo) AS a, bar AS b FROM testTable WHERE b IN
(5, 10, 15) GROUP BY b ORDER BY a DESC";
+ String query =
+ "SELECT SUM(foo) AS a, bar AS b FROM testTable WHERE bar IN (5, 10,
15) GROUP BY b ORDER BY a DESC";
QueryContext queryContext =
QueryContextConverterUtils.getQueryContext(query);
assertEquals(queryContext.getTableName(), "testTable");
List<ExpressionContext> selectExpressions =
queryContext.getSelectExpressions();
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/MultiValueRawQueriesTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/MultiValueRawQueriesTest.java
index 45b70cea74..f8b212a788 100644
---
a/pinot-core/src/test/java/org/apache/pinot/queries/MultiValueRawQueriesTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/queries/MultiValueRawQueriesTest.java
@@ -2154,15 +2154,16 @@ public class MultiValueRawQueriesTest extends
BaseQueriesTest {
private String getConcatUseInWhereQueryString(String concatFunction, String
col1, String col2, String concatCol,
String table, String compareOperator, int mvPerArray, int limit) {
- return String.format("SELECT %s(%s, %s) AS %s FROM %s WHERE
arraylength(%s) %s %d LIMIT %d", concatFunction, col1,
- col2, concatCol, table, concatCol, compareOperator, mvPerArray, limit);
+ String function = String.format("%s(%s, %s)", concatFunction, col1, col2);
+ return String.format("SELECT %s AS %s FROM %s WHERE arraylength(%s) %s %d
LIMIT %d", function, concatCol, table,
+ function, compareOperator, mvPerArray, limit);
}
private String getConcatGroupByQueryString(String concatFunction, String
col1, String col2, String concatCol,
String table, String compareOperator, int mvPerArray, int limit) {
- return String.format(
- "SELECT %s(%s, %s) AS %s, sum(svIntCol) FROM %s WHERE arraylength(%s)
%s %d GROUP BY %s LIMIT %d",
- concatFunction, col1, col2, concatCol, table, concatCol,
compareOperator, mvPerArray, concatCol, limit);
+ String function = String.format("%s(%s, %s)", concatFunction, col1, col2);
+ return String.format("SELECT %s AS %s, sum(svIntCol) FROM %s WHERE
arraylength(%s) %s %d GROUP BY %s LIMIT %d",
+ function, concatCol, table, function, compareOperator, mvPerArray,
concatCol, limit);
}
@Test
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 89d2d24495..df045e1d5b 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
@@ -2230,6 +2230,13 @@ public class OfflineClusterIntegrationTest extends
BaseClusterIntegrationTestSet
public void testQueryWithAlias(boolean useMultiStageQueryEngine)
throws Exception {
setUseMultiStageQueryEngine(useMultiStageQueryEngine);
+ {
+ String pinotQuery = "SELECT count(*), DaysSinceEpoch as d FROM mytable
WHERE d = 16138 GROUP BY d";
+ JsonNode jsonNode = postQuery(pinotQuery);
+ JsonNode exceptions = jsonNode.get("exceptions");
+ assertFalse(exceptions.isEmpty());
+ assertEquals(exceptions.get(0).get("errorCode").asInt(), 710);
+ }
{
//test same alias name with column name
String query = "SELECT ArrTime AS ArrTime, Carrier AS Carrier,
DaysSinceEpoch AS DaysSinceEpoch FROM mytable "
@@ -2254,6 +2261,16 @@ public class OfflineClusterIntegrationTest extends
BaseClusterIntegrationTestSet
query = "SELECT count(*) AS cnt, Carrier AS CarrierName FROM mytable
GROUP BY CarrierName ORDER BY cnt";
testQuery(query);
+
+ // Test: 1. Alias should not be applied to filter; 2. Ordinal can be
properly applied
+ query =
+ "SELECT DaysSinceEpoch + 100 AS DaysSinceEpoch, COUNT(*) AS cnt FROM
mytable WHERE DaysSinceEpoch <= 16312 "
+ + "GROUP BY 1 ORDER BY 1 DESC";
+ // NOTE: H2 does not support ordinal in GROUP BY
+ String h2Query =
+ "SELECT DaysSinceEpoch + 100 AS DaysSinceEpoch, COUNT(*) AS cnt FROM
mytable WHERE DaysSinceEpoch <= 16312 "
+ + "GROUP BY DaysSinceEpoch ORDER BY 1 DESC";
+ testQuery(query, h2Query);
}
{
//test multiple alias
@@ -2589,7 +2606,6 @@ public class OfflineClusterIntegrationTest extends
BaseClusterIntegrationTestSet
testQuery(pinotQuery, h2Query);
}
-
@Test
public void testQuerySourceWithDatabaseNameV2()
throws Exception {
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/JsonPathTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/JsonPathTest.java
index c310417b7d..7dd460d1f5 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/JsonPathTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/JsonPathTest.java
@@ -339,7 +339,7 @@ public class JsonPathTest extends
CustomDataQueryClusterIntegrationTest {
JsonNode pinotResponse = postQuery(query);
int expectedStatusCode;
if (useMultiStageQueryEngine) {
- expectedStatusCode = QueryException.QUERY_PLANNING_ERROR_CODE;
+ expectedStatusCode = QueryException.UNKNOWN_COLUMN_ERROR_CODE;
} else {
expectedStatusCode = QueryException.SQL_PARSING_ERROR_CODE;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
