You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by sh...@apache.org on 2018/05/18 09:53:18 UTC

[kylin] 08/08: KYLIN-3358 add a trigger kylin.query.enable-dynamic-column with default value false for coprocessor backward compatibility

This is an automated email from the ASF dual-hosted git repository.

shaofengshi pushed a commit to branch KYLIN-3359
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit ef051501371acb396077486b0870bd6ce12e6725
Author: Zhong <nj...@apache.org>
AuthorDate: Tue May 15 10:19:27 2018 +0800

    KYLIN-3358 add a trigger kylin.query.enable-dynamic-column with default value false for coprocessor backward compatibility
    
    Signed-off-by: shaofengshi <sh...@apache.org>
---
 .../org/apache/kylin/common/KylinConfigBase.java   |  6 +++++
 .../org/apache/kylin/query/ITKylinQueryTest.java   |  4 +++-
 .../kylin/query/relnode/OLAPAggregateRel.java      | 27 +++++++++++++++-------
 .../apache/kylin/query/relnode/OLAPContext.java    |  4 ++++
 .../apache/kylin/query/relnode/OLAPProjectRel.java |  2 +-
 5 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 7b24864..b939a75 100644
--- a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -1227,6 +1227,12 @@ abstract public class KylinConfigBase implements Serializable {
         return Lists.newArrayList(rules.split(","));
     }
 
+    // check KYLIN-3358, need deploy coprocessor if enabled
+    // finally should be deprecated
+    public boolean isDynamicColumnEnabled() {
+        return Boolean.valueOf(getOptional("kylin.query.enable-dynamic-column", "false"));
+    }
+
     //check KYLIN-1684, in most cases keep the default value
     public boolean isSkippingEmptySegments() {
         return Boolean.valueOf(getOptional("kylin.query.skip-empty-segments", "true"));
diff --git a/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java b/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java
index 269c65f..2d0c7b5 100644
--- a/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java
+++ b/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java
@@ -417,7 +417,9 @@ public class ITKylinQueryTest extends KylinTestBase {
 
     @Test
     public void testExpressionQuery() throws Exception {
-        batchExecuteQuery(getQueryFolderPrefix() + "src/test/resources/query/sql_expression");
+        if (config.isDynamicColumnEnabled()) {
+            batchExecuteQuery(getQueryFolderPrefix() + "src/test/resources/query/sql_expression");
+        }
     }
 
     @Test
diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPAggregateRel.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPAggregateRel.java
index 84f7676..ab8be1b 100644
--- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPAggregateRel.java
+++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPAggregateRel.java
@@ -266,25 +266,27 @@ public class OLAPAggregateRel extends Aggregate implements OLAPRel {
         this.groups = Lists.newArrayList();
         for (int i = getGroupSet().nextSetBit(0); i >= 0; i = getGroupSet().nextSetBit(i + 1)) {
             TupleExpression tupleExpression = inputColumnRowType.getSourceColumnsByIndex(i);
+            TblColRef groupOutCol = inputColumnRowType.getColumnByIndex(i);
             if (tupleExpression instanceof ColumnTupleExpression) {
                 this.groups.add(((ColumnTupleExpression) tupleExpression).getColumn());
-            } else {
-                TblColRef groupOutCol = inputColumnRowType.getColumnByIndex(i);
+            } else if (this.context.isDynamicColumnEnabled()) {
                 Pair<Set<TblColRef>, Set<TblColRef>> cols = ExpressionColCollector.collectColumnsPair(tupleExpression);
 
                 // push down only available for the innermost aggregation
                 boolean ifPushDown = !afterAggregate;
 
                 // if measure columns exist, don't do push down
-                if (!cols.getSecond().isEmpty()) {
+                if (ifPushDown && !cols.getSecond().isEmpty()) {
                     ifPushDown = false;
                 }
 
                 // if existing a dimension which is a derived column, don't do push down
-                for (TblColRef dimCol : cols.getFirst()) {
-                    if (!this.context.belongToFactTableDims(dimCol)) {
-                        ifPushDown = false;
-                        break;
+                if (ifPushDown) {
+                    for (TblColRef dimCol : cols.getFirst()) {
+                        if (!this.context.belongToFactTableDims(dimCol)) {
+                            ifPushDown = false;
+                            break;
+                        }
                     }
                 }
 
@@ -296,6 +298,13 @@ public class OLAPAggregateRel extends Aggregate implements OLAPRel {
                     this.groups.addAll(cols.getSecond());
                     this.context.dynamicFields.remove(groupOutCol);
                 }
+            } else {
+                Set<TblColRef> srcCols = ExpressionColCollector.collectColumns(tupleExpression);
+                // if no source columns, use target column instead
+                if (srcCols.isEmpty()) {
+                    srcCols.add(groupOutCol);
+                }
+                this.groups.addAll(srcCols);
             }
         }
     }
@@ -330,11 +339,12 @@ public class OLAPAggregateRel extends Aggregate implements OLAPRel {
                 }
             }
             // Check dynamic aggregation
-            if (!afterAggregate && !rewriting && argList.size() == 1) {
+            if (this.context.isDynamicColumnEnabled() && !afterAggregate && !rewriting && argList.size() == 1) {
                 int iRowIdx = argList.get(0);
                 TupleExpression tupleExpr = inputColumnRowType.getSourceColumnsByIndex(iRowIdx);
                 if (aggCall.getAggregation() instanceof SqlSumAggFunction
                         || aggCall.getAggregation() instanceof SqlSumEmptyIsZeroAggFunction) {
+                    // sum (expression)
                     if (!(tupleExpr instanceof NumberTupleExpression || tupleExpr instanceof ColumnTupleExpression)) {
                         ColumnTupleExpression cntExpr = new ColumnTupleExpression(SumDynamicFunctionDesc.mockCntCol);
                         ExpressionCountDistributor cntDistributor = new ExpressionCountDistributor(cntExpr);
@@ -344,6 +354,7 @@ public class OLAPAggregateRel extends Aggregate implements OLAPRel {
                         continue;
                     }
                 } else if (aggCall.getAggregation() instanceof SqlCountAggFunction && !aggCall.isDistinct()) {
+                    // count column
                     if (tupleExpr instanceof ColumnTupleExpression) {
                         TblColRef srcCol = ((ColumnTupleExpression) tupleExpr).getColumn();
                         if (this.context.belongToFactTableDims(srcCol)) {
diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java
index f3dcd1b..c03bd0b 100644
--- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java
+++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java
@@ -200,6 +200,10 @@ public class OLAPContext {
         return sqlDigest;
     }
 
+    public boolean isDynamicColumnEnabled() {
+        return olapSchema != null && olapSchema.getConfig().isDynamicColumnEnabled();
+    }
+
     public boolean hasPrecalculatedFields() {
         return realization instanceof CubeInstance || realization instanceof HybridInstance;
     }
diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPProjectRel.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPProjectRel.java
index 05b27ea..39f4bb0 100644
--- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPProjectRel.java
+++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPProjectRel.java
@@ -164,7 +164,7 @@ public class OLAPProjectRel extends Project implements OLAPRel {
                 }
                 this.context.allColumns.addAll(srcCols);
 
-                if (tupleExpr.ifForDynamicColumn()) {
+                if (this.context.isDynamicColumnEnabled() && tupleExpr.ifForDynamicColumn()) {
                     SqlTypeName fSqlType = columnField.getType().getSqlTypeName();
                     String dataType = OLAPTable.DATATYPE_MAPPING.get(fSqlType);
                     // upgrade data type for number columns

-- 
To stop receiving notification emails like this one, please contact
shaofengshi@apache.org.