This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch handle_non_standard_sql
in repository https://gitbox.apache.org/repos/asf/pinot.git
commit 741712158c2eb3ab4296aba4fb613b8cde743858
Author: Xiaotian (Jackie) Jiang <jackie....@gmail.com>
AuthorDate: Wed Nov 8 18:20:27 2023 -0800
Handle legacy query and log error
---
.../pinot/sql/parsers/rewriter/AliasApplier.java | 28 ++++++++++++++++++
.../pinot/sql/parsers/CalciteSqlCompilerTest.java | 30 --------------------
.../transform/function/CaseTransformFunction.java | 19 ++++++++++---
.../function/CaseTransformFunctionTest.java | 33 ----------------------
.../tests/OfflineClusterIntegrationTest.java | 17 -----------
5 files changed, 43 insertions(+), 84 deletions(-)
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 de8d06f338..729b9df374 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
@@ -26,9 +26,12 @@ import org.apache.pinot.common.request.Function;
import org.apache.pinot.common.request.Identifier;
import org.apache.pinot.common.request.PinotQuery;
import org.apache.pinot.sql.parsers.SqlCompilationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class AliasApplier implements QueryRewriter {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(AliasApplier.class);
@Override
public PinotQuery rewrite(PinotQuery pinotQuery) {
@@ -53,6 +56,10 @@ public class AliasApplier implements QueryRewriter {
}
private static void applyAlias(Map<String, Expression> aliasMap, PinotQuery
pinotQuery) {
+ Expression filterExpression = pinotQuery.getFilterExpression();
+ if (filterExpression != null) {
+ applyAliasForFilter(aliasMap, filterExpression);
+ }
List<Expression> groupByList = pinotQuery.getGroupByList();
if (groupByList != null) {
for (Expression expression : groupByList) {
@@ -90,4 +97,25 @@ public class AliasApplier implements QueryRewriter {
}
}
}
+
+ private static void applyAliasForFilter(Map<String, Expression> aliasMap,
Expression expression) {
+ Identifier identifier = expression.getIdentifier();
+ if (identifier != null) {
+ Expression aliasExpression = aliasMap.get(identifier.getName());
+ if (aliasExpression != null) {
+ LOGGER.error("LEGACY QUERY: Cannot apply alias for filter: {}",
expression);
+ expression.setType(aliasExpression.getType());
+ expression.setIdentifier(aliasExpression.getIdentifier());
+ expression.setFunctionCall(aliasExpression.getFunctionCall());
+ expression.setLiteral(aliasExpression.getLiteral());
+ }
+ return;
+ }
+ Function function = expression.getFunctionCall();
+ if (function != null) {
+ for (Expression operand : function.getOperands()) {
+ applyAliasForFilter(aliasMap, operand);
+ }
+ }
+ }
}
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 3576c19660..ef49ade2a8 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
@@ -1467,27 +1467,6 @@ public class CalciteSqlCompilerTest {
pinotQuery.getOrderByList().get(1).getFunctionCall().getOperands().get(0).getFunctionCall().getOperands().get(0)
.getIdentifier().getName(), "rsvp_count");
- sql = "select secondsSinceEpoch/86400 AS daysSinceEpoch, sum(rsvp_count)
as sum_rsvp_count, count(*) as cnt"
- + " from meetupRsvp where daysSinceEpoch = 18523 group by
daysSinceEpoch order by cnt, sum_rsvp_count DESC"
- + " limit 50";
- pinotQuery = 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).getIdentifier().getName(),
- "daysSinceEpoch");
- Assert.assertEquals(
-
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(1).getLiteral().getIntValue(),
18523);
- Assert.assertEquals(pinotQuery.getGroupByListSize(), 1);
-
Assert.assertEquals(pinotQuery.getGroupByList().get(0).getFunctionCall().getOperator(),
"divide");
- Assert.assertEquals(
-
pinotQuery.getGroupByList().get(0).getFunctionCall().getOperands().get(0).getIdentifier().getName(),
- "secondsSinceEpoch");
- Assert.assertEquals(
-
pinotQuery.getGroupByList().get(0).getFunctionCall().getOperands().get(1).getLiteral().getIntValue(),
86400);
- Assert.assertEquals(pinotQuery.getOrderByListSize(), 2);
-
// 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";
@@ -1535,15 +1514,6 @@ 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 = 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";
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java
index 0d008c212e..318a1a394a 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java
@@ -19,6 +19,7 @@
package org.apache.pinot.core.operator.transform.function;
import com.google.common.base.Preconditions;
+import com.google.common.util.concurrent.RateLimiter;
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Arrays;
@@ -37,6 +38,8 @@ import org.apache.pinot.spi.utils.BytesUtils;
import org.apache.pinot.spi.utils.CommonConstants.NullValuePlaceHolder;
import org.apache.pinot.spi.utils.TimestampUtils;
import org.roaringbitmap.RoaringBitmap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
@@ -59,6 +62,9 @@ import org.roaringbitmap.RoaringBitmap;
* PostgreSQL documentation: <a
href="https://www.postgresql.org/docs/current/typeconv-union-case.html";>CASE</a>
*/
public class CaseTransformFunction extends
ComputeDifferentlyWhenNullHandlingEnabledTransformFunction {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(CaseTransformFunction.class);
+ private static final RateLimiter LOG_RATE_LIMITER = RateLimiter.create(1.0);
+
public static final String FUNCTION_NAME = "case";
private List<TransformFunction> _whenStatements = new ArrayList<>();
@@ -147,10 +153,15 @@ public class CaseTransformFunction extends
ComputeDifferentlyWhenNullHandlingEna
checkLiteral(currentType, ((LiteralTransformFunction)
newFunction).getStringLiteral());
} else {
// Only allow upcast from numeric to numeric: INT -> LONG -> FLOAT ->
DOUBLE -> BIG_DECIMAL
- Preconditions.checkArgument(currentType.isNumeric() &&
newType.isNumeric(), "Cannot upcast from %s to %s",
- currentType, newType);
- if (newType.ordinal() > currentType.ordinal()) {
- currentTypeAndUnresolvedLiterals.setLeft(newType);
+ if (currentType.isNumeric() && newType.isNumeric()) {
+ if (newType.ordinal() > currentType.ordinal()) {
+ currentTypeAndUnresolvedLiterals.setLeft(newType);
+ }
+ } else {
+ if (LOG_RATE_LIMITER.tryAcquire()) {
+ LOGGER.error("LEGACY QUERY: Cannot upcast from {} to {}",
currentType, newType);
+ }
+ currentTypeAndUnresolvedLiterals.setLeft(DataType.STRING);
}
}
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunctionTest.java
index 315b53e9e0..90dfd455ee 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunctionTest.java
@@ -145,39 +145,6 @@ public class CaseTransformFunctionTest extends
BaseTransformFunctionTest {
}
}
- @DataProvider
- public static String[] illegalExpressions() {
- //@formatter:off
- return new String[] {
- // '10.0' cannot be parsed as INT/LONG/TIMESTAMP/BYTES
- String.format("CASE WHEN true THEN %s ELSE '10.0' END", INT_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE '10.0' END",
LONG_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE '10.0' END",
TIMESTAMP_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE '10.0' END",
BYTES_SV_COLUMN),
- // 'abc' cannot be parsed as any type other than STRING
- String.format("CASE WHEN true THEN %s ELSE 'abc' END", INT_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE 'abc' END", LONG_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE 'abc' END",
FLOAT_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE 'abc' END",
DOUBLE_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE 'abc' END",
BIG_DECIMAL_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE 'abc' END",
TIMESTAMP_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE 'abc' END",
BYTES_SV_COLUMN),
- // Cannot mix 2 types that are not both numeric
- String.format("CASE WHEN true THEN %s ELSE %s END", INT_SV_COLUMN,
TIMESTAMP_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE %s END", INT_SV_COLUMN,
STRING_SV_COLUMN),
- String.format("CASE WHEN true THEN %s ELSE %s END", INT_SV_COLUMN,
BYTES_SV_COLUMN),
- String.format("CASE WHEN true THEN 100 ELSE %s END", TIMESTAMP_COLUMN),
- String.format("CASE WHEN true THEN 100 ELSE %s END", STRING_SV_COLUMN),
- String.format("CASE WHEN true THEN 100 ELSE %s END", BYTES_SV_COLUMN)
- };
- //@formatter:on
- }
-
- @Test(dataProvider = "illegalExpressions", expectedExceptions =
Exception.class)
- public void testInvalidCaseTransformFunction(String expression) {
-
TransformFunctionFactory.get(RequestContextUtils.getExpression(expression),
_dataSourceMap);
- }
-
@Test
public void testCaseTransformationWithNullColumn() {
ExpressionContext expression = RequestContextUtils.getExpression(
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 c6018cacc1..2307264ca4 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
@@ -2418,13 +2418,6 @@ 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 "
@@ -2449,16 +2442,6 @@ 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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
