Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kylin-3812 optimize the child CompareTupleFilter in a CompareTupleFilter #533

Merged
merged 3 commits into from
May 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ public CaseTupleExpression(List<Pair<TupleFilter, TupleExpression>> whenList, Tu
protected boolean ifAbleToPushDown() {
if (ifAbleToPushDown == null) {
for (Pair<TupleFilter, TupleExpression> whenEntry : whenList) {
ifAbleToPushDown = whenEntry.getSecond().ifAbleToPushDown();
ifAbleToPushDown = TupleFilter.isEvaluableRecursively(whenEntry.getFirst())
&& whenEntry.getSecond().ifAbleToPushDown();
if (!ifAbleToPushDown) {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public enum CompareResultType {
AlwaysTrue, AlwaysFalse, Unknown
}

// if the two children are both CompareTupleFilter, isNormal will be false
private boolean isNormal = true;

// operand 1 is either a column or a function
private TblColRef column;
private FunctionTupleFilter function;
Expand Down Expand Up @@ -74,6 +77,9 @@ private CompareTupleFilter(CompareTupleFilter another) {

@Override
public void addChild(TupleFilter child) {
if (child instanceof CompareTupleFilter) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz add some test case in UT or IT

Copy link
Contributor Author

@kyotoYaho kyotoYaho Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to refine our IT and add more test cases. To test this case, it's necessary to add a column with boolean type. However, it's a trivial job and may need another JIRA to work this.

child = optimizeChildCompareTupleFilter((CompareTupleFilter) child);
}
super.addChild(child);
if (child instanceof ColumnTupleFilter) {
ColumnTupleFilter columnFilter = (ColumnTupleFilter) child;
Expand Down Expand Up @@ -234,7 +240,7 @@ private boolean isConstant(TupleFilter filter) {

@Override
public boolean isEvaluable() {
return (column != null || (function != null && function.isEvaluable())) //
return isNormal && (column != null || (function != null && function.isEvaluable())) //
&& (!conditionValues.isEmpty() || operator == FilterOperatorEnum.ISNOTNULL || operator == FilterOperatorEnum.ISNULL) //
&& secondColumn == null;
}
Expand Down Expand Up @@ -283,6 +289,20 @@ public TupleFilter acceptOptimizeTransformer(FilterOptimizeTransformer transform
return transformer.visit(this);
}

private TupleFilter optimizeChildCompareTupleFilter(CompareTupleFilter child) {
FilterOptimizeTransformer transformer = new FilterOptimizeTransformer();
TupleFilter result = child.acceptOptimizeTransformer(transformer);
if (result == ConstantTupleFilter.TRUE) {
// use string instead of boolean since it's encoded as string
result = new ConstantTupleFilter("true");
} else if (result == ConstantTupleFilter.FALSE) {
result = new ConstantTupleFilter("false");
} else {
this.isNormal = false;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means that TupleFilters excepted AlwaysTrue or AlwaysFalse will not pushdown?

Copy link
Contributor Author

@kyotoYaho kyotoYaho Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think AlwaysTrue or AlwaysFalse cannot be pushed down? Current CompareTupleFilter does not well support the case that any of its children is a CompareTupleFilter and is not alway true or always false. Only in this case push down will be disabled. This behavior is not changed.

}
return result;
}

@Override
public boolean equals(Object o) {
if (this == o)
Expand Down
35 changes: 35 additions & 0 deletions kylin-it/src/test/resources/query/sql_casewhen/query04.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
--
-- 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.
--

select "TEST_KYLIN_FACT"."ORDER_ID",
case
when ("TEST_KYLIN_FACT"."LSTG_FORMAT_NAME"='Auction') = (1 = 1) then 'B'
when ("TEST_KYLIN_FACT"."LSTG_FORMAT_NAME"='FP-GTC') = (1 = 1) then 'C'
when ("TEST_KYLIN_FACT"."LSTG_FORMAT_NAME"='ABIN') = (1 = 1) then 'D'
else 'N'
end as phase
from TEST_KYLIN_FACT
where "TEST_KYLIN_FACT"."CAL_DT" between '2013-01-01' and '2013-01-02'
group by "TEST_KYLIN_FACT"."ORDER_ID",
case
when ("TEST_KYLIN_FACT"."LSTG_FORMAT_NAME"='Auction') = (1 = 1) then 'B'
when ("TEST_KYLIN_FACT"."LSTG_FORMAT_NAME"='FP-GTC') = (1 = 1) then 'C'
when ("TEST_KYLIN_FACT"."LSTG_FORMAT_NAME"='ABIN') = (1 = 1) then 'D'
else 'N'
end

Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ void buildGroups() {
TblColRef groupOutCol = inputColumnRowType.getColumnByIndex(i);
if (tupleExpression instanceof ColumnTupleExpression) {
this.groups.add(((ColumnTupleExpression) tupleExpression).getColumn());
} else if (this.context.isDynamicColumnEnabled()) {
} else if (this.context.isDynamicColumnEnabled() && tupleExpression.ifForDynamicColumn()) {
Pair<Set<TblColRef>, Set<TblColRef>> cols = ExpressionColCollector.collectColumnsPair(tupleExpression);

// push down only available for the innermost aggregation
Expand Down