This is an automated email from the ASF dual-hosted git repository.
jiaguo 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 6303658500 update RewriterConstants so that expr min max would not
collide with columns start with "parent" (#13357)
6303658500 is described below
commit 6303658500c343bc02d5582791106398b9eda94e
Author: Jia Guo <jia...@linkedin.com>
AuthorDate: Wed Jun 12 15:22:14 2024 -0700
update RewriterConstants so that expr min max would not collide with
columns start with "parent" (#13357)
* update enums and fix tests
update enums and fix tests
update RewriterConstants
* fix tests
* fix tests
* add logging to debug
* add logging to debug
* fix test
---
.../parsers/rewriter/ExprMinMaxRewriterTest.java | 40 +++++++++++-----------
.../function/AggregationFunctionFactory.java | 8 ++---
.../org/apache/pinot/queries/ExprMinMaxTest.java | 22 +++++++-----
.../pinot/segment/spi/AggregationFunctionType.java | 8 ++---
.../apache/pinot/spi/utils/CommonConstants.java | 4 +--
5 files changed, 43 insertions(+), 39 deletions(-)
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/ExprMinMaxRewriterTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/ExprMinMaxRewriterTest.java
index d44b9d7abf..c0486d0392 100644
---
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/ExprMinMaxRewriterTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/rewriter/ExprMinMaxRewriterTest.java
@@ -30,21 +30,21 @@ public class ExprMinMaxRewriterTest {
@Test
public void testQueryRewrite() {
testQueryRewrite("SELECT EXPR_MIN(col2,col1), EXPR_MIN(col3,col1) FROM
myTable",
- "SELECT CHILD_EXPR_MIN(0,col2,col2,col1), "
- + "CHILD_EXPR_MIN(0,col3,col3,col1),"
- + "PARENT_EXPR_MIN(0,1,col1,col2,col3) FROM myTable");
+ "SELECT PINOTCHILDAGG_EXPR_MIN(0,col2,col2,col1), "
+ + "PINOTCHILDAGG_EXPR_MIN(0,col3,col3,col1),"
+ + "PINOTPARENTAGG_EXPR_MIN(0,1,col1,col2,col3) FROM myTable");
testQueryRewrite("SELECT EXPR_MIN(col2,col1), EXPR_MIN(col2,col1) FROM
myTable",
- "SELECT CHILD_EXPR_MIN(0,col2,col2,col1),"
- + "PARENT_EXPR_MIN(0,1,col1,col2) FROM myTable");
+ "SELECT PINOTCHILDAGG_EXPR_MIN(0,col2,col2,col1),"
+ + "PINOTPARENTAGG_EXPR_MIN(0,1,col1,col2) FROM myTable");
testQueryRewrite("SELECT EXPR_MIN(col5,col1,col2),
EXPR_MIN(col6,col1,col2), EXPR_MAX(col6,col1,col2) "
+ "FROM myTable",
- "SELECT CHILD_EXPR_MIN(0,col5,col5,col1,col2), "
- + "CHILD_EXPR_MIN(0,col6,col6,col1,col2), "
- + "CHILD_EXPR_MAX(0,col6,col6,col1,col2),"
- + "PARENT_EXPR_MIN(0,2,col1,col2,col5,col6),"
- + "PARENT_EXPR_MAX(0,2,col1,col2,col6) FROM myTable");
+ "SELECT PINOTCHILDAGG_EXPR_MIN(0,col5,col5,col1,col2), "
+ + "PINOTCHILDAGG_EXPR_MIN(0,col6,col6,col1,col2), "
+ + "PINOTCHILDAGG_EXPR_MAX(0,col6,col6,col1,col2),"
+ + "PINOTPARENTAGG_EXPR_MIN(0,2,col1,col2,col5,col6),"
+ + "PINOTPARENTAGG_EXPR_MAX(0,2,col1,col2,col6) FROM myTable");
}
@Test
@@ -52,20 +52,20 @@ public class ExprMinMaxRewriterTest {
testQueryRewrite("SELECT EXPR_MIN(col5,col1,col2),
EXPR_MIN(col6,col1,col3),"
+ "EXPR_MIN(col6,col3,col1) FROM myTable GROUP BY col3 "
+ "ORDER BY col3 DESC",
- "SELECT CHILD_EXPR_MIN(0,col5,col5,col1,col2), "
- + "CHILD_EXPR_MIN(1,col6,col6,col1,col3),"
- + "CHILD_EXPR_MIN(2,col6,col6,col3,col1),"
- + "PARENT_EXPR_MIN(1,2,col1,col3,col6),"
- + "PARENT_EXPR_MIN(0,2,col1,col2,col5),"
- + "PARENT_EXPR_MIN(2,2,col3,col1,col6)"
+ "SELECT PINOTCHILDAGG_EXPR_MIN(0,col5,col5,col1,col2), "
+ + "PINOTCHILDAGG_EXPR_MIN(1,col6,col6,col1,col3),"
+ + "PINOTCHILDAGG_EXPR_MIN(2,col6,col6,col3,col1),"
+ + "PINOTPARENTAGG_EXPR_MIN(1,2,col1,col3,col6),"
+ + "PINOTPARENTAGG_EXPR_MIN(0,2,col1,col2,col5),"
+ + "PINOTPARENTAGG_EXPR_MIN(2,2,col3,col1,col6)"
+ "FROM myTable GROUP BY col3 ORDER BY col3 DESC");
testQueryRewrite("SELECT EXPR_MIN(col5,col1,col2),
EXPR_MAX(col5,col1,col2) FROM myTable GROUP BY col3 "
+ "ORDER BY ADD(co1, co3) DESC",
- "SELECT CHILD_EXPR_MIN(0,col5,col5,col1,col2),"
- + "CHILD_EXPR_MAX(0,col5,col5,col1,col2),"
- + "PARENT_EXPR_MIN(0,2,col1,col2,col5), "
- + "PARENT_EXPR_MAX(0,2,col1,col2,col5) "
+ "SELECT PINOTCHILDAGG_EXPR_MIN(0,col5,col5,col1,col2),"
+ + "PINOTCHILDAGG_EXPR_MAX(0,col5,col5,col1,col2),"
+ + "PINOTPARENTAGG_EXPR_MIN(0,2,col1,col2,col5), "
+ + "PINOTPARENTAGG_EXPR_MAX(0,2,col1,col2,col5) "
+ "FROM myTable GROUP BY col3 ORDER BY ADD(co1, co3) DESC");
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java
index 7e6dbc63e4..96491acbd4 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java
@@ -447,13 +447,13 @@ public class AggregationFunctionFactory {
return new
SumValuesIntegerTupleSketchAggregationFunction(arguments,
IntegerSummary.Mode.Sum);
case AVGVALUEINTEGERSUMTUPLESKETCH:
return new
AvgValueIntegerTupleSketchAggregationFunction(arguments,
IntegerSummary.Mode.Sum);
- case PARENTEXPRMAX:
+ case PINOTPARENTAGGEXPRMAX:
return new ParentExprMinMaxAggregationFunction(arguments, true);
- case PARENTEXPRMIN:
+ case PINOTPARENTAGGEXPRMIN:
return new ParentExprMinMaxAggregationFunction(arguments, false);
- case CHILDEXPRMAX:
+ case PINOTCHILDAGGEXPRMAX:
return new ChildExprMinMaxAggregationFunction(arguments, true);
- case CHILDEXPRMIN:
+ case PINOTCHILDAGGEXPRMIN:
return new ChildExprMinMaxAggregationFunction(arguments, false);
case EXPRMAX:
case EXPRMIN:
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
index 98a63579f1..121ed8c563 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
@@ -45,11 +45,14 @@ import
org.apache.pinot.spi.exception.BadQueryRequestException;
import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.sql.parsers.rewriter.QueryRewriterFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import static org.apache.pinot.spi.utils.CommonConstants.RewriterConstants.*;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
@@ -60,6 +63,7 @@ import static org.testng.Assert.fail;
* Queries test for exprmin/exprmax functions.
*/
public class ExprMinMaxTest extends BaseQueriesTest {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ExprMinMaxTest.class);
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
"ExprMinMaxTest");
private static final String RAW_TABLE_NAME = "testTable";
private static final String SEGMENT_NAME = "testSegment";
@@ -555,15 +559,15 @@ public class ExprMinMaxTest extends BaseQueriesTest {
+ "expr_min(mvStringColumn, intColumn, doubleColumn) FROM testTable
GROUP BY groupByMVIntColumn";
BrokerResponseNative brokerResponse = getBrokerResponse(query);
Object groupByExplainPlan =
brokerResponse.getResultTable().getRows().get(3)[0];
- Assert.assertTrue(groupByExplainPlan
- .toString().contains("child_exprMin('0', mvIntColumn, mvIntColumn,
intColumn)"));
- Assert.assertTrue(groupByExplainPlan
- .toString()
- .contains("child_exprMin('1', mvStringColumn, mvStringColumn,
intColumn, doubleColumn)"));
- Assert.assertTrue(groupByExplainPlan
- .toString().contains("parent_exprMin('0', '1', intColumn,
mvIntColumn)"));
- Assert.assertTrue(groupByExplainPlan
- .toString().contains("parent_exprMin('1', '2', intColumn,
doubleColumn, mvStringColumn)"));
+ String explainPlan = groupByExplainPlan.toString();
+ Assert.assertTrue(
+ explainPlan.contains(CHILD_AGGREGATION_NAME_PREFIX + "_exprMin('0',
mvIntColumn, mvIntColumn, intColumn)"));
+ Assert.assertTrue(explainPlan.contains(
+ CHILD_AGGREGATION_NAME_PREFIX + "_exprMin('1', mvStringColumn,
mvStringColumn, intColumn, doubleColumn)"));
+ Assert.assertTrue(
+ explainPlan.contains(PARENT_AGGREGATION_NAME_PREFIX + "_exprMin('0',
'1', intColumn, mvIntColumn)"));
+ Assert.assertTrue(explainPlan.contains(
+ PARENT_AGGREGATION_NAME_PREFIX + "_exprMin('1', '2', intColumn,
doubleColumn, mvStringColumn)"));
}
@Test
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
index 2a511cf814..9c91a1d806 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
@@ -303,20 +303,20 @@ public enum AggregationFunctionType {
OperandTypes.family(ImmutableList.of(SqlTypeFamily.ANY), ordinal ->
ordinal > 1), ReturnTypes.ARG0,
ReturnTypes.explicit(SqlTypeName.OTHER)),
-
PARENTEXPRMIN(CommonConstants.RewriterConstants.PARENT_AGGREGATION_NAME_PREFIX
+ EXPRMIN.getName(), null,
+
PINOTPARENTAGGEXPRMIN(CommonConstants.RewriterConstants.PARENT_AGGREGATION_NAME_PREFIX
+ EXPRMIN.getName(), null,
SqlKind.OTHER_FUNCTION, SqlFunctionCategory.USER_DEFINED_FUNCTION,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.INTEGER,
SqlTypeFamily.ANY), ordinal -> ordinal > 2),
ReturnTypes.explicit(SqlTypeName.OTHER),
ReturnTypes.explicit(SqlTypeName.OTHER)),
-
PARENTEXPRMAX(CommonConstants.RewriterConstants.PARENT_AGGREGATION_NAME_PREFIX
+ EXPRMAX.getName(), null,
+
PINOTPARENTAGGEXPRMAX(CommonConstants.RewriterConstants.PARENT_AGGREGATION_NAME_PREFIX
+ EXPRMAX.getName(), null,
SqlKind.OTHER_FUNCTION, SqlFunctionCategory.USER_DEFINED_FUNCTION,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.INTEGER,
SqlTypeFamily.ANY), ordinal -> ordinal > 2),
ReturnTypes.explicit(SqlTypeName.OTHER),
ReturnTypes.explicit(SqlTypeName.OTHER)),
- CHILDEXPRMIN(CommonConstants.RewriterConstants.CHILD_AGGREGATION_NAME_PREFIX
+ EXPRMIN.getName(), null,
+
PINOTCHILDAGGEXPRMIN(CommonConstants.RewriterConstants.CHILD_AGGREGATION_NAME_PREFIX
+ EXPRMIN.getName(), null,
SqlKind.OTHER_FUNCTION, SqlFunctionCategory.USER_DEFINED_FUNCTION,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.INTEGER,
SqlTypeFamily.ANY), ordinal -> ordinal > 3),
ReturnTypes.ARG1, ReturnTypes.explicit(SqlTypeName.OTHER)),
- CHILDEXPRMAX(CommonConstants.RewriterConstants.CHILD_AGGREGATION_NAME_PREFIX
+ EXPRMAX.getName(), null,
+
PINOTCHILDAGGEXPRMAX(CommonConstants.RewriterConstants.CHILD_AGGREGATION_NAME_PREFIX
+ EXPRMAX.getName(), null,
SqlKind.OTHER_FUNCTION, SqlFunctionCategory.USER_DEFINED_FUNCTION,
OperandTypes.family(ImmutableList.of(SqlTypeFamily.INTEGER,
SqlTypeFamily.ANY), ordinal -> ordinal > 3),
ReturnTypes.ARG1, ReturnTypes.explicit(SqlTypeName.OTHER)),
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index 265a870656..8b85f861e5 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -1097,8 +1097,8 @@ public class CommonConstants {
}
public static class RewriterConstants {
- public static final String PARENT_AGGREGATION_NAME_PREFIX = "parent";
- public static final String CHILD_AGGREGATION_NAME_PREFIX = "child";
+ public static final String PARENT_AGGREGATION_NAME_PREFIX =
"pinotparentagg";
+ public static final String CHILD_AGGREGATION_NAME_PREFIX = "pinotchildagg";
public static final String CHILD_AGGREGATION_SEPERATOR = "@";
public static final String CHILD_KEY_SEPERATOR = "_";
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
