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 <gor...@users.noreply.github.com> 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: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org