This is an automated email from the ASF dual-hosted git repository.
jackie 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 65907b78d6 Make ARRAY_TO_MV a not deterministic function (#13618)
65907b78d6 is described below
commit 65907b78d633faa29de6e05f955afd7f3d94343f
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Tue Jul 16 21:01:00 2024 +0200
Make ARRAY_TO_MV a not deterministic function (#13618)
---
.../common/function/TransformFunctionType.java | 21 ++++++++++++++++++++-
.../tests/MultiStageEngineIntegrationTest.java | 20 ++++++++++++++++++++
.../calcite/sql/PinotSqlTransformFunction.java | 9 ++++++++-
.../pinot/calcite/sql/fun/PinotOperatorTable.java | 2 +-
4 files changed, 49 insertions(+), 3 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
index d2c5c8127c..cb0683fd8b 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
@@ -106,7 +106,22 @@ public enum TransformFunctionType {
// object type
ARRAY_TO_MV("arrayToMV",
ReturnTypes.cascade(opBinding ->
positionalComponentReturnType(opBinding, 0), SqlTypeTransforms.FORCE_NULLABLE),
- OperandTypes.family(SqlTypeFamily.ARRAY), "array_to_mv"),
+ OperandTypes.family(SqlTypeFamily.ARRAY), "array_to_mv") {
+
+ @Override
+ public boolean isDeterministic() {
+ // ARRAY_TO_MV is not deterministic. In fact, it has no implementation
+ // We need to explicitly set it as not deterministic in order to do not
let Calcite to optimize expressions like
+ // `ARRAY_TO_MV(RandomAirports) = 'MFR' and ARRAY_TO_MV(RandomAirports)
= 'GTR'` as `false`.
+ // If the function were deterministic, its value would never be MFR and
GTR at the same time, so Calcite is
+ // smart enough to know there is no value that satisfies the condition.
+ // In fact what ARRAY_TO_MV does is just to trick Calcite typesystem,
but then what the leaf stage executor
+ // receives is `RandomAirports = 'MFR' and RandomAirports = 'GTR'`,
which in the V1 semantics means:
+ // true if and only if RandomAirports contains a value equal to 'MFR'
and RandomAirports contains a value equal
+ // to 'GTR'
+ return false;
+ }
+ },
// string functions
JSON_EXTRACT_SCALAR("jsonExtractScalar",
@@ -358,6 +373,10 @@ public enum TransformFunctionType {
return _sqlFunctionCategory;
}
+ public boolean isDeterministic() {
+ return true;
+ }
+
/** Returns the optional explicit returning type specification. */
private static RelDataType
positionalReturnTypeInferenceFromStringLiteral(SqlOperatorBinding opBinding,
int pos) {
return positionalReturnTypeInferenceFromStringLiteral(opBinding, pos,
SqlTypeName.ANY);
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
index 811e957fe1..602979fc57 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
@@ -704,6 +704,26 @@ public class MultiStageEngineIntegrationTest extends
BaseClusterIntegrationTestS
assertEquals(jsonNode.get("numRowsResultSet").asInt(), 3);
}
+ @Test
+ public void skipArrayToMvOptimization()
+ throws Exception {
+ String sqlQuery = "SELECT 1 "
+ + "FROM mytable "
+ + "WHERE ARRAY_TO_MV(RandomAirports) = 'MFR' and
ARRAY_TO_MV(RandomAirports) = 'GTR'";
+
+ JsonNode jsonNode = postQuery("Explain plan for " + sqlQuery);
+ JsonNode plan = jsonNode.get("resultTable").get("rows").get(0).get(1);
+
+ Pattern pattern = Pattern.compile("LogicalValues\\(tuples=\\[\\[]]\\)");
+ String planAsText = plan.asText();
+ boolean matches = pattern.matcher(planAsText).find();
+ Assert.assertFalse(matches, "Plan should not contain contain LogicalValues
node but plan is \n"
+ + planAsText);
+
+ jsonNode = postQuery(sqlQuery);
+ Assert.assertNotEquals(jsonNode.get("resultTable").get("rows").size(), 0);
+ }
+
@Test
public void testMultiValueColumnGroupByOrderBy()
throws Exception {
diff --git
a/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/PinotSqlTransformFunction.java
b/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/PinotSqlTransformFunction.java
index 7c97fbf7ae..ffd5b49667 100644
---
a/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/PinotSqlTransformFunction.java
+++
b/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/PinotSqlTransformFunction.java
@@ -31,10 +31,17 @@ import org.checkerframework.checker.nullness.qual.Nullable;
* Pinot SqlAggFunction class to register the Pinot aggregation functions with
the Calcite operator table.
*/
public class PinotSqlTransformFunction extends SqlFunction {
+ private final boolean _isDeterministic;
public PinotSqlTransformFunction(String name, SqlKind kind, @Nullable
SqlReturnTypeInference returnTypeInference,
@Nullable SqlOperandTypeInference operandTypeInference, @Nullable
SqlOperandTypeChecker operandTypeChecker,
- SqlFunctionCategory category) {
+ SqlFunctionCategory category, boolean isDeterministic) {
super(name, kind, returnTypeInference, operandTypeInference,
operandTypeChecker, category);
+ _isDeterministic = isDeterministic;
+ }
+
+ @Override
+ public boolean isDeterministic() {
+ return _isDeterministic;
}
}
diff --git
a/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/fun/PinotOperatorTable.java
b/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/fun/PinotOperatorTable.java
index 1eb1890304..68dbce88f3 100644
---
a/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/fun/PinotOperatorTable.java
+++
b/pinot-query-planner/src/main/java/org/apache/pinot/calcite/sql/fun/PinotOperatorTable.java
@@ -149,7 +149,7 @@ public class PinotOperatorTable extends SqlStdOperatorTable
{
PinotSqlTransformFunction sqlTransformFunction =
new PinotSqlTransformFunction(functionName.toUpperCase(Locale.ROOT),
functionType.getSqlKind(),
functionType.getReturnTypeInference(), null,
functionType.getOperandTypeChecker(),
- functionType.getSqlFunctionCategory());
+ functionType.getSqlFunctionCategory(),
functionType.isDeterministic());
if (notRegistered(sqlTransformFunction)) {
register(sqlTransformFunction);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
