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 <[email protected]>
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;
+ }
+}
