From 75e2051195075c14171d5b8ab7f97abef79e80a0 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Mon, 9 Mar 2020 22:50:38 -0700 Subject: [PATCH] Convert array_contains() and array_overlaps() into native filters if possible (#9487) * Convert array_contains() and array_overlaps() into native filters if possible * make spotbugs happy and fix null results when null compatible --- .../benchmark/ExpressionFilterBenchmark.java | 195 ++++++++++++++++++ .../org/apache/druid/math/expr/Function.java | 6 +- docs/querying/filters.md | 2 + .../apache/druid/segment/filter/InFilter.java | 5 + .../druid/segment/filter/InFilterTest.java | 4 + .../expression/AliasedOperatorConversion.java | 41 ++++ .../ArrayContainsOperatorConversion.java | 71 +++++++ .../ArrayOverlapOperatorConversion.java | 78 +++++++ ...ExpressionDimFilterOperatorConversion.java | 43 ++-- .../sql/calcite/planner/PlannerFactory.java | 2 - .../druid/sql/calcite/CalciteQueryTest.java | 72 +++++-- 11 files changed, 481 insertions(+), 38 deletions(-) create mode 100644 benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java new file mode 100644 index 00000000000..6b96d613735 --- /dev/null +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionFilterBenchmark.java @@ -0,0 +1,195 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.benchmark; + +import com.google.common.collect.ImmutableList; +import org.apache.druid.benchmark.datagen.BenchmarkColumnSchema; +import org.apache.druid.benchmark.datagen.BenchmarkSchemaInfo; +import org.apache.druid.benchmark.datagen.SegmentGenerator; +import org.apache.druid.common.config.NullHandling; +import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.java.util.common.guava.Sequence; +import org.apache.druid.java.util.common.io.Closer; +import org.apache.druid.query.expression.TestExprMacroTable; +import org.apache.druid.query.filter.AndDimFilter; +import org.apache.druid.query.filter.DimFilter; +import org.apache.druid.query.filter.ExpressionDimFilter; +import org.apache.druid.query.filter.SelectorDimFilter; +import org.apache.druid.segment.ColumnValueSelector; +import org.apache.druid.segment.Cursor; +import org.apache.druid.segment.QueryableIndex; +import org.apache.druid.segment.QueryableIndexStorageAdapter; +import org.apache.druid.segment.VirtualColumns; +import org.apache.druid.segment.column.ValueType; +import org.apache.druid.timeline.DataSegment; +import org.apache.druid.timeline.partition.LinearShardSpec; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.TimeUnit; + +@State(Scope.Benchmark) +@Fork(value = 1) +@Warmup(iterations = 15) +@Measurement(iterations = 30) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +public class ExpressionFilterBenchmark +{ + static { + NullHandling.initializeForTests(); + } + + @Param({"1000000"}) + private int rowsPerSegment; + + private QueryableIndex index; + private Closer closer; + + private ExpressionDimFilter expressionFilter; + private DimFilter nativeFilter; + + @Setup(Level.Trial) + public void setup() + { + this.closer = Closer.create(); + + final BenchmarkSchemaInfo schemaInfo = new BenchmarkSchemaInfo( + ImmutableList.of( + BenchmarkColumnSchema.makeEnumerated( + "x", + ValueType.STRING, + false, + 3, + null, + Arrays.asList("Apple", "Orange", "Xylophone", "Corundum", null), + Arrays.asList(0.2, 0.25, 0.15, 0.10, 0.3) + ), + BenchmarkColumnSchema.makeEnumerated( + "y", + ValueType.STRING, + false, + 4, + null, + Arrays.asList("Hello", "World", "Foo", "Bar", "Baz"), + Arrays.asList(0.2, 0.25, 0.15, 0.10, 0.3) + ) + ), + ImmutableList.of(), + Intervals.of("2000/P1D"), + false + ); + + final DataSegment dataSegment = DataSegment.builder() + .dataSource("foo") + .interval(schemaInfo.getDataInterval()) + .version("1") + .shardSpec(new LinearShardSpec(0)) + .size(0) + .build(); + + final SegmentGenerator segmentGenerator = closer.register(new SegmentGenerator()); + this.index = closer.register( + segmentGenerator.generate(dataSegment, schemaInfo, Granularities.NONE, rowsPerSegment) + ); + + expressionFilter = new ExpressionDimFilter( + "array_contains(x, ['Orange', 'Xylophone'])", + TestExprMacroTable.INSTANCE + ); + nativeFilter = new AndDimFilter( + new SelectorDimFilter("x", "Orange", null), + new SelectorDimFilter("x", "Xylophone", null) + ); + } + + @TearDown(Level.Trial) + public void tearDown() throws Exception + { + closer.close(); + } + + @Benchmark + public void expressionFilter(Blackhole blackhole) + { + final Sequence cursors = new QueryableIndexStorageAdapter(index).makeCursors( + expressionFilter.toFilter(), + index.getDataInterval(), + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ); + final List results = cursors + .map(cursor -> { + final ColumnValueSelector selector = cursor.getColumnSelectorFactory().makeColumnValueSelector("x"); + consumeString(cursor, selector, blackhole); + return null; + }) + .toList(); + + blackhole.consume(results); + } + + @Benchmark + public void nativeFilter(Blackhole blackhole) + { + final Sequence cursors = new QueryableIndexStorageAdapter(index).makeCursors( + nativeFilter.toFilter(), + index.getDataInterval(), + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ); + final List results = cursors + .map(cursor -> { + final ColumnValueSelector selector = cursor.getColumnSelectorFactory().makeColumnValueSelector("x"); + consumeString(cursor, selector, blackhole); + return null; + }) + .toList(); + + blackhole.consume(results); + } + + private void consumeString(final Cursor cursor, final ColumnValueSelector selector, final Blackhole blackhole) + { + while (!cursor.isDone()) { + blackhole.consume(selector.getLong()); + cursor.advance(); + } + } +} diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java index ae71b7e2478..bf6a8d2e050 100644 --- a/core/src/main/java/org/apache/druid/math/expr/Function.java +++ b/core/src/main/java/org/apache/druid/math/expr/Function.java @@ -1780,8 +1780,7 @@ interface Function ExprType elementType = null; for (int i = 0; i < length; i++) { - - ExprEval evaluated = args.get(i).eval(bindings); + ExprEval evaluated = args.get(i).eval(bindings); if (elementType == null) { elementType = evaluated.type(); switch (elementType) { @@ -1802,6 +1801,9 @@ interface Function setArrayOutputElement(stringsOut, longsOut, doublesOut, elementType, i, evaluated); } + // There should be always at least one argument and thus elementType is never null. + // See validateArguments(). + //noinspection ConstantConditions switch (elementType) { case STRING: return ExprEval.ofStringArray(stringsOut); diff --git a/docs/querying/filters.md b/docs/querying/filters.md index 0d9ad6e30ac..034e332387e 100644 --- a/docs/querying/filters.md +++ b/docs/querying/filters.md @@ -231,6 +231,8 @@ The grammar for a IN filter is as follows: The IN filter supports the use of extraction functions, see [Filtering with Extraction Functions](#filtering-with-extraction-functions) for details. If an empty `values` array is passed to the IN filter, it will simply return an empty result. +If the `dimension` is a multi-valued dimension, the IN filter will return true if one of the dimension values is +in the `values` array. ### Like filter diff --git a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java index 54ad64b5066..6e1da91f11f 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java @@ -49,6 +49,11 @@ import java.util.Objects; import java.util.Set; /** + * The IN filter. + * For single-valued dimension, this filter returns true if the dimension value matches to one of the + * given {@link #values}. + * For multi-valued dimension, this filter returns true if one of the dimension values matches to one of the + * given {@link #values}. */ public class InFilter implements Filter { diff --git a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java index e4a8fde5750..4ca919279be 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java @@ -155,6 +155,10 @@ public class InFilterTest extends BaseFilterTest public void testMultiValueStringColumn() { if (NullHandling.replaceWithDefault()) { + assertFilterMatches( + toInFilter("dim2", "b", "d"), + ImmutableList.of("a") + ); assertFilterMatches( toInFilter("dim2", null), ImmutableList.of("b", "c", "f") diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/AliasedOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/AliasedOperatorConversion.java index 4e0717fda8a..01b9530e66d 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/AliasedOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/AliasedOperatorConversion.java @@ -24,9 +24,14 @@ import org.apache.calcite.sql.SqlFunction; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlOperator; import org.apache.druid.java.util.common.IAE; +import org.apache.druid.query.aggregation.PostAggregator; +import org.apache.druid.query.filter.DimFilter; import org.apache.druid.sql.calcite.planner.PlannerContext; +import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry; import org.apache.druid.sql.calcite.table.RowSignature; +import javax.annotation.Nullable; + public class AliasedOperatorConversion implements SqlOperatorConversion { private final SqlOperatorConversion baseConversion; @@ -68,4 +73,40 @@ public class AliasedOperatorConversion implements SqlOperatorConversion { return baseConversion.toDruidExpression(plannerContext, rowSignature, rexNode); } + + @Nullable + @Override + public DruidExpression toDruidExpressionWithPostAggOperands( + PlannerContext plannerContext, + RowSignature rowSignature, + RexNode rexNode, + PostAggregatorVisitor postAggregatorVisitor + ) + { + return baseConversion.toDruidExpression(plannerContext, rowSignature, rexNode); + } + + @Nullable + @Override + public DimFilter toDruidFilter( + PlannerContext plannerContext, + RowSignature rowSignature, + @Nullable VirtualColumnRegistry virtualColumnRegistry, + RexNode rexNode + ) + { + return baseConversion.toDruidFilter(plannerContext, rowSignature, virtualColumnRegistry, rexNode); + } + + @Nullable + @Override + public PostAggregator toPostAggregator( + PlannerContext plannerContext, + RowSignature querySignature, + RexNode rexNode, + PostAggregatorVisitor postAggregatorVisitor + ) + { + return baseConversion.toPostAggregator(plannerContext, querySignature, rexNode, postAggregatorVisitor); + } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayContainsOperatorConversion.java index dad30ddebbd..2bfe51ea055 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayContainsOperatorConversion.java @@ -19,11 +19,28 @@ package org.apache.druid.sql.calcite.expression.builtin; +import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlFunction; import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlTypeFamily; +import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprEval; +import org.apache.druid.math.expr.Parser; +import org.apache.druid.query.filter.AndDimFilter; +import org.apache.druid.query.filter.DimFilter; +import org.apache.druid.sql.calcite.expression.DruidExpression; +import org.apache.druid.sql.calcite.expression.Expressions; import org.apache.druid.sql.calcite.expression.OperatorConversions; +import org.apache.druid.sql.calcite.planner.PlannerContext; +import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry; +import org.apache.druid.sql.calcite.table.RowSignature; + +import javax.annotation.Nullable; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; public class ArrayContainsOperatorConversion extends BaseExpressionDimFilterOperatorConversion { @@ -51,4 +68,58 @@ public class ArrayContainsOperatorConversion extends BaseExpressionDimFilterOper { super(SQL_FUNCTION, EXPR_FUNCTION); } + + @Nullable + @Override + public DimFilter toDruidFilter( + final PlannerContext plannerContext, + RowSignature rowSignature, + @Nullable VirtualColumnRegistry virtualColumnRegistry, + final RexNode rexNode + ) + { + final List operands = ((RexCall) rexNode).getOperands(); + final List druidExpressions = Expressions.toDruidExpressions( + plannerContext, + rowSignature, + operands + ); + if (druidExpressions == null) { + return null; + } + + // Converts array_contains() function into an AND of Selector filters if possible. + final DruidExpression leftExpr = druidExpressions.get(0); + final DruidExpression rightExpr = druidExpressions.get(1); + + if (leftExpr.isSimpleExtraction()) { + Expr expr = Parser.parse(rightExpr.getExpression(), plannerContext.getExprMacroTable()); + // To convert this expression filter into an And of Selector filters, we need to extract all array elements. + // For now, we can optimize only when rightExpr is a literal because there is no way to extract the array elements + // by traversing the Expr. Note that all implementations of Expr are defined as package-private classes in a + // different package. + if (expr.isLiteral()) { + // Evaluate the expression to get out the array elements. + // We can safely pass a noop ObjectBinding if the expression is literal. + ExprEval exprEval = expr.eval(name -> null); + String[] arrayElements = exprEval.asStringArray(); + if (arrayElements == null || arrayElements.length == 0) { + // If arrayElements is empty which means rightExpr is an empty array, + // it is technically more correct to return a TrueDimFiler here. + // However, since both Calcite's SqlMultisetValueConstructor and Druid's ArrayConstructorFunction don't allow + // to create an empty array with no argument, we just return null. + return null; + } else if (arrayElements.length == 1) { + return newSelectorDimFilter(leftExpr.getSimpleExtraction(), arrayElements[0]); + } else { + final List selectFilters = Arrays + .stream(arrayElements) + .map(val -> newSelectorDimFilter(leftExpr.getSimpleExtraction(), val)) + .collect(Collectors.toList()); + return new AndDimFilter(selectFilters); + } + } + } + return toExpressionFilter(plannerContext, getDruidFunctionName(), druidExpressions); + } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java index 7fa3eba013f..57ef4b51bbe 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java @@ -19,11 +19,27 @@ package org.apache.druid.sql.calcite.expression.builtin; +import com.google.common.collect.Sets; +import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlFunction; import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlTypeFamily; +import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprEval; +import org.apache.druid.math.expr.Parser; +import org.apache.druid.query.filter.DimFilter; +import org.apache.druid.query.filter.InDimFilter; +import org.apache.druid.sql.calcite.expression.DruidExpression; +import org.apache.druid.sql.calcite.expression.Expressions; import org.apache.druid.sql.calcite.expression.OperatorConversions; +import org.apache.druid.sql.calcite.planner.PlannerContext; +import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry; +import org.apache.druid.sql.calcite.table.RowSignature; + +import javax.annotation.Nullable; +import java.util.List; public class ArrayOverlapOperatorConversion extends BaseExpressionDimFilterOperatorConversion { @@ -51,4 +67,66 @@ public class ArrayOverlapOperatorConversion extends BaseExpressionDimFilterOpera { super(SQL_FUNCTION, EXPR_FUNCTION); } + + @Nullable + @Override + public DimFilter toDruidFilter( + final PlannerContext plannerContext, + RowSignature rowSignature, + @Nullable VirtualColumnRegistry virtualColumnRegistry, + final RexNode rexNode + ) + { + final List operands = ((RexCall) rexNode).getOperands(); + final List druidExpressions = Expressions.toDruidExpressions( + plannerContext, + rowSignature, + operands + ); + if (druidExpressions == null) { + return null; + } + + // Converts array_overlaps() function into an OR of Selector filters if possible. + final boolean leftSimpleExtractionExpr = druidExpressions.get(0).isSimpleExtraction(); + final boolean rightSimpleExtractionExpr = druidExpressions.get(1).isSimpleExtraction(); + final DruidExpression simpleExtractionExpr; + final DruidExpression complexExpr; + + if (leftSimpleExtractionExpr ^ rightSimpleExtractionExpr) { + if (leftSimpleExtractionExpr) { + simpleExtractionExpr = druidExpressions.get(0); + complexExpr = druidExpressions.get(1); + } else { + simpleExtractionExpr = druidExpressions.get(1); + complexExpr = druidExpressions.get(0); + } + } else { + return toExpressionFilter(plannerContext, getDruidFunctionName(), druidExpressions); + } + + Expr expr = Parser.parse(complexExpr.getExpression(), plannerContext.getExprMacroTable()); + if (expr.isLiteral()) { + // Evaluate the expression to take out the array elements. + // We can safely pass null if the expression is literal. + ExprEval exprEval = expr.eval(name -> null); + String[] arrayElements = exprEval.asStringArray(); + if (arrayElements == null || arrayElements.length == 0) { + // If arrayElements is empty which means complexExpr is an empty array, + // it is technically more correct to return a TrueDimFiler here. + // However, since both Calcite's SqlMultisetValueConstructor and Druid's ArrayConstructorFunction don't allow + // to create an empty array with no argument, we just return null. + return null; + } else if (arrayElements.length == 1) { + return newSelectorDimFilter(simpleExtractionExpr.getSimpleExtraction(), arrayElements[0]); + } else { + return new InDimFilter( + simpleExtractionExpr.getSimpleExtraction().getColumn(), + Sets.newHashSet(arrayElements), + simpleExtractionExpr.getSimpleExtraction().getExtractionFn() + ); + } + } + return toExpressionFilter(plannerContext, getDruidFunctionName(), druidExpressions); + } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/BaseExpressionDimFilterOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/BaseExpressionDimFilterOperatorConversion.java index 1b212d90836..a1f6dc36bbb 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/BaseExpressionDimFilterOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/BaseExpressionDimFilterOperatorConversion.java @@ -19,19 +19,15 @@ package org.apache.druid.sql.calcite.expression.builtin; -import org.apache.calcite.rex.RexCall; -import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlOperator; import org.apache.druid.query.filter.DimFilter; import org.apache.druid.query.filter.ExpressionDimFilter; +import org.apache.druid.query.filter.SelectorDimFilter; import org.apache.druid.sql.calcite.expression.DirectOperatorConversion; import org.apache.druid.sql.calcite.expression.DruidExpression; -import org.apache.druid.sql.calcite.expression.Expressions; +import org.apache.druid.sql.calcite.expression.SimpleExtraction; import org.apache.druid.sql.calcite.planner.PlannerContext; -import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry; -import org.apache.druid.sql.calcite.table.RowSignature; -import javax.annotation.Nullable; import java.util.List; public abstract class BaseExpressionDimFilterOperatorConversion extends DirectOperatorConversion @@ -44,27 +40,30 @@ public abstract class BaseExpressionDimFilterOperatorConversion extends DirectOp super(operator, druidFunctionName); } - @Nullable - @Override - public DimFilter toDruidFilter( - final PlannerContext plannerContext, - RowSignature rowSignature, - @Nullable VirtualColumnRegistry virtualColumnRegistry, - final RexNode rexNode + protected static DimFilter toExpressionFilter( + PlannerContext plannerContext, + String druidFunctionName, + List druidExpressions ) { - final List operands = ((RexCall) rexNode).getOperands(); - final List druidExpressions = Expressions.toDruidExpressions( - plannerContext, - rowSignature, - operands - ); - final String filterExpr = DruidExpression.functionCall(getDruidFunctionName(), druidExpressions); + final String filterExpr = DruidExpression.functionCall(druidFunctionName, druidExpressions); - ExpressionDimFilter expressionDimFilter = new ExpressionDimFilter( + return new ExpressionDimFilter( filterExpr, plannerContext.getExprMacroTable() ); - return expressionDimFilter; + } + + protected static SelectorDimFilter newSelectorDimFilter( + SimpleExtraction simpleExtraction, + String value + ) + { + return new SelectorDimFilter( + simpleExtraction.getColumn(), + value, + simpleExtraction.getExtractionFn(), + null + ); } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java index ffa325273ae..4389040e5cc 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerFactory.java @@ -27,7 +27,6 @@ import org.apache.calcite.avatica.util.Quoting; import org.apache.calcite.config.CalciteConnectionConfig; import org.apache.calcite.config.CalciteConnectionConfigImpl; import org.apache.calcite.plan.Context; -import org.apache.calcite.plan.Contexts; import org.apache.calcite.plan.ConventionTraitDef; import org.apache.calcite.rel.RelCollationTraitDef; import org.apache.calcite.schema.SchemaPlus; @@ -120,7 +119,6 @@ public class PlannerFactory .operatorTable(operatorTable) .programs(Rules.programs(plannerContext, queryMaker)) .executor(new DruidRexExecutor(plannerContext)) - .context(Contexts.EMPTY_CONTEXT) .typeSystem(DruidTypeSystem.INSTANCE) .defaultSchema(rootSchema.getSubSchema(druidSchemaName)) .sqlToRelConverterConfig(sqlToRelConverterConfig) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 76cd55b72e7..8aabb454500 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -73,6 +73,7 @@ import org.apache.druid.query.dimension.DefaultDimensionSpec; import org.apache.druid.query.dimension.ExtractionDimensionSpec; import org.apache.druid.query.extraction.RegexDimExtractionFn; import org.apache.druid.query.extraction.SubstringDimExtractionFn; +import org.apache.druid.query.filter.AndDimFilter; import org.apache.druid.query.filter.BoundDimFilter; import org.apache.druid.query.filter.DimFilter; import org.apache.druid.query.filter.InDimFilter; @@ -2335,7 +2336,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest } @Test - @Ignore // Disabled since GROUP BY alias can confuse the validator; see DruidConformance::isGroupByAlias + @Ignore("Disabled since GROUP BY alias can confuse the validator; see DruidConformance::isGroupByAlias") public void testGroupByAndOrderByAlias() throws Exception { testQuery( @@ -10673,12 +10674,9 @@ public class CalciteQueryTest extends BaseCalciteQueryTest ); } - /** - * In Calcite 1.17, this test worked, but after upgrading to Calcite 1.21, this query fails with: - * org.apache.calcite.sql.validate.SqlValidatorException: Column 'dim1' is ambiguous - */ @Test - @Ignore + @Ignore("In Calcite 1.17, this test worked, but after upgrading to Calcite 1.21, this query fails with:" + + " org.apache.calcite.sql.validate.SqlValidatorException: Column 'dim1' is ambiguous") public void testProjectAfterSort3() throws Exception { testQuery( @@ -11717,7 +11715,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest newScanQueryBuilder() .dataSource(CalciteTests.DATASOURCE3) .intervals(querySegmentSpec(Filtration.eternity())) - .filters(expressionFilter("array_overlap(\"dim3\",array('a','b'))")) + .filters(new InDimFilter("dim3", ImmutableList.of("a", "b"), null)) .columns("dim3") .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) .limit(5) @@ -11732,15 +11730,15 @@ public class CalciteQueryTest extends BaseCalciteQueryTest } @Test - public void testMultiValueStringOverlapFilterNonConstant() throws Exception + public void testMultiValueStringOverlapFilterNonLiteral() throws Exception { testQuery( - "SELECT dim3 FROM druid.numfoo WHERE MV_OVERLAP(dim3, ARRAY['a','b']) LIMIT 5", + "SELECT dim3 FROM druid.numfoo WHERE MV_OVERLAP(dim3, ARRAY[dim2]) LIMIT 5", ImmutableList.of( newScanQueryBuilder() .dataSource(CalciteTests.DATASOURCE3) .intervals(querySegmentSpec(Filtration.eternity())) - .filters(expressionFilter("array_overlap(\"dim3\",array('a','b'))")) + .filters(expressionFilter("array_overlap(\"dim3\",array(\"dim2\"))")) .columns("dim3") .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) .limit(5) @@ -11749,7 +11747,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest ), ImmutableList.of( new Object[]{"[\"a\",\"b\"]"}, - new Object[]{"[\"b\",\"c\"]"} + new Object[]{useDefault ? "" : null} ) ); } @@ -11763,7 +11761,12 @@ public class CalciteQueryTest extends BaseCalciteQueryTest newScanQueryBuilder() .dataSource(CalciteTests.DATASOURCE3) .intervals(querySegmentSpec(Filtration.eternity())) - .filters(expressionFilter("array_contains(\"dim3\",array('a','b'))")) + .filters( + new AndDimFilter( + new SelectorDimFilter("dim3", "a", null), + new SelectorDimFilter("dim3", "b", null) + ) + ) .columns("dim3") .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) .limit(5) @@ -11776,6 +11779,51 @@ public class CalciteQueryTest extends BaseCalciteQueryTest ); } + @Test + public void testMultiValueStringContainsArrayOfOneElement() throws Exception + { + testQuery( + "SELECT dim3 FROM druid.numfoo WHERE MV_CONTAINS(dim3, ARRAY['a']) LIMIT 5", + ImmutableList.of( + newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters(new SelectorDimFilter("dim3", "a", null)) + .columns("dim3") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .limit(5) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"} + ) + ); + } + + @Test + public void testMultiValueStringContainsArrayOfNonLiteral() throws Exception + { + testQuery( + "SELECT dim3 FROM druid.numfoo WHERE MV_CONTAINS(dim3, ARRAY[dim2]) LIMIT 5", + ImmutableList.of( + newScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters(expressionFilter("array_contains(\"dim3\",array(\"dim2\"))")) + .columns("dim3") + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .limit(5) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{"[\"a\",\"b\"]"}, + new Object[]{useDefault ? "" : null} + ) + ); + } + @Test public void testMultiValueStringSlice() throws Exception {