This is an automated email from the ASF dual-hosted git repository. nju_yaho pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/master by this push: new b1f1b7b KYLIN-3794 fix mergeToInClause not working properly in some edge cases b1f1b7b is described below commit b1f1b7bdf04a00fd9fd96e35e700f4ee7aeb455e Author: kyotoYaho <nju_y...@apache.org> AuthorDate: Wed Jan 30 10:07:35 2019 +0800 KYLIN-3794 fix mergeToInClause not working properly in some edge cases --- .../query/relnode/visitor/TupleFilterVisitor.java | 78 ++++++++++++-------- .../relnode/visitor/TupleFilterVisitorTest.java | 86 ++++++++++++++++++++++ 2 files changed, 132 insertions(+), 32 deletions(-) diff --git a/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java b/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java index faa9988..7634023 100644 --- a/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java +++ b/query/src/main/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitor.java @@ -18,13 +18,12 @@ package org.apache.kylin.query.relnode.visitor; -import java.math.BigDecimal; -import java.util.GregorianCalendar; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; - +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.apache.calcite.avatica.util.TimeUnitRange; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rex.RexCall; @@ -39,6 +38,7 @@ import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.util.NlsString; import org.apache.kylin.common.util.DateFormat; +import org.apache.kylin.common.util.Pair; import org.apache.kylin.metadata.filter.CaseTupleFilter; import org.apache.kylin.metadata.filter.ColumnTupleFilter; import org.apache.kylin.metadata.filter.CompareTupleFilter; @@ -52,9 +52,11 @@ import org.apache.kylin.metadata.filter.function.Functions; import org.apache.kylin.metadata.model.TblColRef; import org.apache.kylin.query.relnode.ColumnRowType; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; +import java.math.BigDecimal; +import java.util.GregorianCalendar; +import java.util.List; +import java.util.Map; +import java.util.Set; public class TupleFilterVisitor extends RexVisitorImpl<TupleFilter> { @@ -146,10 +148,7 @@ public class TupleFilterVisitor extends RexVisitorImpl<TupleFilter> { } if (op.getKind() == SqlKind.OR) { - CompareTupleFilter inFilter = mergeToInClause(filter); - if (inFilter != null) { - filter = inFilter; - } + filter = mergeToInClause(filter); } else if (op.getKind() == SqlKind.NOT) { assert (filter.getChildren().size() == 1); filter = filter.getChildren().get(0).reverse(); @@ -221,36 +220,51 @@ public class TupleFilterVisitor extends RexVisitorImpl<TupleFilter> { return constFilter; } - private CompareTupleFilter mergeToInClause(TupleFilter filter) { + @VisibleForTesting + static TupleFilter mergeToInClause(TupleFilter filter) { List<? extends TupleFilter> children = filter.getChildren(); - TblColRef inColumn = null; - List<Object> inValues = new LinkedList<Object>(); - Map<String, Object> dynamicVariables = new HashMap<>(); + if (children.isEmpty()) { + return filter; + } + // key: inColumn + // Value: first: inValues + // Value: second: dynamicVariables + Map<TblColRef, Pair<Set<Object>, Map<String, Object>>> inColumnMap = Maps.newHashMap(); + List<TupleFilter> extraFilters = Lists.newLinkedList(); for (TupleFilter child : children) { if (child.getOperator() == TupleFilter.FilterOperatorEnum.EQ) { CompareTupleFilter compFilter = (CompareTupleFilter) child; TblColRef column = compFilter.getColumn(); - if (inColumn == null) { - inColumn = column; - } + if (column != null) { + Pair<Set<Object>, Map<String, Object>> tmpValue = inColumnMap.get(column); + if (tmpValue == null) { + Set<Object> inValues = Sets.newHashSet(); + Map<String, Object> dynamicVariables = Maps.newHashMap(); + tmpValue = new Pair<>(inValues, dynamicVariables); + inColumnMap.put(column, tmpValue); + } - if (column == null || !column.equals(inColumn)) { - return null; + tmpValue.getFirst().addAll(compFilter.getValues()); + tmpValue.getSecond().putAll(compFilter.getVariables()); + continue; } - inValues.addAll(compFilter.getValues()); - dynamicVariables.putAll(compFilter.getVariables()); - } else { - return null; } + extraFilters.add(child); } children.clear(); - CompareTupleFilter inFilter = new CompareTupleFilter(TupleFilter.FilterOperatorEnum.IN); - inFilter.addChild(new ColumnTupleFilter(inColumn)); - inFilter.addChild(new ConstantTupleFilter(inValues)); - inFilter.getVariables().putAll(dynamicVariables); - return inFilter; + TupleFilter ret = new LogicalTupleFilter(TupleFilter.FilterOperatorEnum.OR); + ret.addChildren(extraFilters); + for (Map.Entry<TblColRef, Pair<Set<Object>, Map<String, Object>>> entry : inColumnMap.entrySet()) { + CompareTupleFilter inFilter = new CompareTupleFilter(TupleFilter.FilterOperatorEnum.IN); + inFilter.addChild(new ColumnTupleFilter(entry.getKey())); + inFilter.addChild(new ConstantTupleFilter(entry.getValue().getFirst())); + inFilter.getVariables().putAll(entry.getValue().getSecond()); + ret.addChild(inFilter); + } + + return ret.getChildren().size() == 1 ? ret.getChildren().get(0) : ret; } @Override diff --git a/query/src/test/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitorTest.java b/query/src/test/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitorTest.java new file mode 100644 index 0000000..352dfab --- /dev/null +++ b/query/src/test/java/org/apache/kylin/query/relnode/visitor/TupleFilterVisitorTest.java @@ -0,0 +1,86 @@ +/* + * 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.kylin.query.relnode.visitor; + +import com.google.common.collect.Lists; +import org.apache.kylin.metadata.filter.ColumnTupleFilter; +import org.apache.kylin.metadata.filter.CompareTupleFilter; +import org.apache.kylin.metadata.filter.ConstantTupleFilter; +import org.apache.kylin.metadata.filter.LogicalTupleFilter; +import org.apache.kylin.metadata.filter.TupleFilter; +import org.apache.kylin.metadata.model.TblColRef; +import org.junit.Assert; +import org.junit.Test; + +public class TupleFilterVisitorTest { + + @Test + public void testMergeToInClause1() { + TupleFilter originFilter = getMockFilter1(); + TupleFilter resultFilter = TupleFilterVisitor.mergeToInClause(originFilter); + Assert.assertNotNull(resultFilter); + Assert.assertEquals(2, resultFilter.getChildren().size()); + } + + @Test + public void testMergeToInClause2() { + TupleFilter originFilter = getMockFilter2(); + TupleFilter resultFilter = TupleFilterVisitor.mergeToInClause(originFilter); + Assert.assertNotNull(resultFilter); + Assert.assertEquals(2, resultFilter.getChildren().size()); + } + + private TupleFilter getMockFilter1() { + LogicalTupleFilter ret = new LogicalTupleFilter(TupleFilter.FilterOperatorEnum.OR); + + TblColRef colRef1 = TblColRef.newInnerColumn("DEFAULT.TEST_KYLIN_FACT.LSTG_FORMAT_NAME", + TblColRef.InnerDataTypeEnum.LITERAL); + ret.addChildren(getCompareEQFilter(colRef1, "ABIN")); + ret.addChildren(getCompareEQFilter(colRef1, "Auction")); + + TblColRef colRef2 = TblColRef.newInnerColumn("DEFAULT.TEST_KYLIN_FACT.DEAL_YEAR", + TblColRef.InnerDataTypeEnum.LITERAL); + ret.addChildren(getCompareEQFilter(colRef2, "2012")); + ret.addChildren(getCompareEQFilter(colRef2, "2013")); + + return ret; + } + + private TupleFilter getMockFilter2() { + LogicalTupleFilter ret = new LogicalTupleFilter(TupleFilter.FilterOperatorEnum.OR); + + TblColRef colRef = TblColRef.newInnerColumn("DEFAULT.TEST_KYLIN_FACT.LSTG_FORMAT_NAME", + TblColRef.InnerDataTypeEnum.LITERAL); + ret.addChildren(getCompareEQFilter(colRef, "ABIN")); + ret.addChildren(getCompareEQFilter(colRef, "Auction")); + + CompareTupleFilter notInFilter = new CompareTupleFilter(TupleFilter.FilterOperatorEnum.NOTIN); + notInFilter.addChildren(getCompareEQFilter(colRef, "Auction", "Others")); + ret.addChildren(notInFilter); + + return ret; + } + + private CompareTupleFilter getCompareEQFilter(TblColRef colRef, String... values) { + CompareTupleFilter ret = new CompareTupleFilter(TupleFilter.FilterOperatorEnum.EQ); + ret.addChild(new ColumnTupleFilter(colRef)); + ret.addChild(new ConstantTupleFilter(Lists.newArrayList(values))); + return ret; + } +}