From 7911cb1932f07be1bd8f00f2b58b5ab02f482c92 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 29 Jun 2022 18:14:35 -0700 Subject: [PATCH 1/2] fix bug when rewriting sql virtual column registry --- .../calcite/rel/VirtualColumnRegistry.java | 8 ++- .../CalciteMultiValueStringQueryTest.java | 61 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java index d07586ec2fb7..1763c283498a 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java @@ -31,10 +31,12 @@ import javax.annotation.Nullable; import java.util.Collection; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Queue; import java.util.stream.Collectors; /** @@ -198,13 +200,15 @@ public List findVirtualColumnExpressions(List allColumn public void visitAllSubExpressions(DruidExpression.DruidExpressionShuttle shuttle) { - for (Map.Entry entry : virtualColumnsByName.entrySet()) { + final Queue> toVisit = new LinkedList<>(virtualColumnsByName.entrySet()); + while(!toVisit.isEmpty()) { + final Map.Entry entry = toVisit.poll(); final String key = entry.getKey(); final ExpressionAndTypeHint wrapped = entry.getValue(); - virtualColumnsByExpression.remove(wrapped); final List newArgs = shuttle.visitAll(wrapped.getExpression().getArguments()); final ExpressionAndTypeHint newWrapped = wrap(wrapped.getExpression().withArguments(newArgs), wrapped.getTypeHint()); virtualColumnsByName.put(key, newWrapped); + virtualColumnsByExpression.remove(wrapped); virtualColumnsByExpression.put(newWrapped, key); } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteMultiValueStringQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteMultiValueStringQueryTest.java index 28fd4d463f6a..a62402cba401 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteMultiValueStringQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteMultiValueStringQueryTest.java @@ -1428,6 +1428,67 @@ public void testMultiValueListFilterComposedDeny() throws Exception ); } + @Test + public void testMultiValueListFilterComposedMultipleExpressions() throws Exception + { + // Cannot vectorize due to usage of expressions. + cannotVectorize(); + + testQuery( + "SELECT MV_LENGTH(MV_FILTER_ONLY(dim3, ARRAY['b'])), MV_LENGTH(dim3), SUM(cnt) FROM druid.numfoo GROUP BY 1,2 ORDER BY 3 DESC", + ImmutableList.of( + GroupByQuery.builder() + .setDataSource(CalciteTests.DATASOURCE3) + .setInterval(querySegmentSpec(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setVirtualColumns( + expressionVirtualColumn( + "v0", + "array_length(\"v2\")", + ColumnType.LONG + ), + expressionVirtualColumn( + "v1", + "array_length(\"dim3\")", + ColumnType.LONG + ), + new ListFilteredVirtualColumn( + "v2", + DefaultDimensionSpec.of("dim3"), + ImmutableSet.of("b"), + true + ) + ) + .setDimensions( + dimensions( + new DefaultDimensionSpec("v0", "_d0", ColumnType.LONG), + new DefaultDimensionSpec("v1", "_d1", ColumnType.LONG) + ) + ) + .setAggregatorSpecs(aggregators(new LongSumAggregatorFactory("a0", "cnt"))) + .setLimitSpec(new DefaultLimitSpec( + ImmutableList.of(new OrderByColumnSpec( + "a0", + OrderByColumnSpec.Direction.DESCENDING, + StringComparators.NUMERIC + )), + Integer.MAX_VALUE + )) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + useDefault ? ImmutableList.of( + new Object[]{0, 0, 3L}, + new Object[]{1, 2, 2L}, + new Object[]{0, 1, 1L} + ) : ImmutableList.of( + new Object[]{null, null, 2L}, + new Object[]{null, 1, 2L}, + new Object[]{1, 2, 2L} + ) + ); + } + @Test public void testFilterOnMultiValueListFilterNoMatch() throws Exception { From c5e58eb39829dfb3e5eab782f73d34a2dba81680 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Thu, 30 Jun 2022 12:07:20 -0700 Subject: [PATCH 2/2] fix stuff --- .../apache/druid/sql/calcite/rel/VirtualColumnRegistry.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java index 1763c283498a..ec6f8c7ee306 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java @@ -29,9 +29,9 @@ import org.apache.druid.sql.calcite.planner.PlannerContext; import javax.annotation.Nullable; +import java.util.ArrayDeque; import java.util.Collection; import java.util.HashMap; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; @@ -200,8 +200,8 @@ public List findVirtualColumnExpressions(List allColumn public void visitAllSubExpressions(DruidExpression.DruidExpressionShuttle shuttle) { - final Queue> toVisit = new LinkedList<>(virtualColumnsByName.entrySet()); - while(!toVisit.isEmpty()) { + final Queue> toVisit = new ArrayDeque<>(virtualColumnsByName.entrySet()); + while (!toVisit.isEmpty()) { final Map.Entry entry = toVisit.poll(); final String key = entry.getKey(); final ExpressionAndTypeHint wrapped = entry.getValue();