KYLIN-2341 report error on unsupported aggr func

liyang-kylin · liyang-kylin · commit 7af5529ef566 · 2017-04-03T22:35:21.000+08:00
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java
@@ -19,7 +19,6 @@
 package org.apache.kylin.metadata.model;
 
 import java.io.Serializable;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -41,6 +40,7 @@
 
 /**
  */
+@SuppressWarnings("serial")
 @JsonAutoDetect(fieldVisibility = Visibility.NONE, getterVisibility = Visibility.NONE, isGetterVisibility = Visibility.NONE, setterVisibility = Visibility.NONE)
 public class FunctionDesc implements Serializable {
 
@@ -90,17 +90,13 @@ public void init(DataModelDesc model) {
         expression = expression.toUpperCase();
         returnDataType = DataType.getType(returnType);
 
-        ArrayList<TblColRef> colRefs = Lists.newArrayList();
         for (ParameterDesc p = parameter; p != null; p = p.getNextParameter()) {
             if (p.isColumnType()) {
                 TblColRef colRef = model.findColumn(p.getValue());
                 p.setValue(colRef.getIdentity());
-                colRefs.add(colRef);
+                p.setColRef(colRef);
             }
         }
-
-        if (parameter != null)
-            parameter.setColRefs(colRefs);
     }
 
     private void reInitMeasureType() {
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/ParameterDesc.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/ParameterDesc.java
@@ -20,6 +20,7 @@
 
 import java.io.Serializable;
 import java.io.UnsupportedEncodingException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
@@ -28,8 +29,6 @@
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 
 /**
@@ -49,20 +48,14 @@ public static ParameterDesc newInstance(Object... objs) {
             TblColRef col = (TblColRef) obj;
             r.type = FunctionDesc.PARAMETER_TYPE_COLUMN;
             r.value = col.getIdentity();
-            r.colRefs = ImmutableList.of(col);
+            r.colRef = col;
         } else {
             r.type = FunctionDesc.PARAMETER_TYPE_CONSTANT;
             r.value = (String) obj;
         }
 
         if (objs.length >= 2) {
             r.nextParameter = newInstance(Arrays.copyOfRange(objs, 1, objs.length));
-            if (r.nextParameter.colRefs.size() > 0) {
-                if (r.colRefs.isEmpty())
-                    r.colRefs = r.nextParameter.colRefs;
-                else
-                    r.colRefs = ImmutableList.copyOf(Iterables.concat(r.colRefs, r.nextParameter.colRefs));
-            }
         }
         return r;
     }
@@ -76,7 +69,8 @@ public static ParameterDesc newInstance(Object... objs) {
     @JsonInclude(JsonInclude.Include.NON_NULL)
     private ParameterDesc nextParameter;
 
-    private List<TblColRef> colRefs = ImmutableList.of();
+    private TblColRef colRef = null;
+    private List<TblColRef> allColRefsIncludingNexts = null;
     private Set<PlainParameter> plainParameters = null;
 
     // Lazy evaluation
@@ -102,13 +96,28 @@ public String getValue() {
     void setValue(String value) {
         this.value = value;
     }
+    
+    public TblColRef getColRef() {
+        return colRef;
+    }
 
-    public List<TblColRef> getColRefs() {
-        return colRefs;
+    void setColRef(TblColRef colRef) {
+        this.colRef = colRef;
     }
 
-    void setColRefs(List<TblColRef> colRefs) {
-        this.colRefs = colRefs;
+    public List<TblColRef> getColRefs() {
+        if (allColRefsIncludingNexts == null) {
+            List<TblColRef> all = new ArrayList<>(2);
+            ParameterDesc p = this;
+            while (p != null) {
+                if (p.isColumnType())
+                    all.add(p.getColRef());
+                
+                p = p.nextParameter;
+            }
+            allColRefsIncludingNexts = all;
+        }
+        return allColRefsIncludingNexts;
     }
 
     public ParameterDesc getNextParameter() {
@@ -132,17 +141,12 @@ public boolean equals(Object o) {
             return false;
 
         ParameterDesc p = this, q = that;
-        int refi = 0, refj = 0;
         for (; p != null && q != null; p = p.nextParameter, q = q.nextParameter) {
             if (p.isColumnType()) {
                 if (q.isColumnType() == false)
                     return false;
-                if (refi >= this.colRefs.size() || refj >= that.colRefs.size())
-                    return false;
-                if (this.colRefs.get(refi).equals(that.colRefs.get(refj)) == false)
+                if (this.getColRef().equals(that.getColRef()) == false)
                     return false;
-                refi++;
-                refj++;
             } else {
                 if (q.isColumnType() == true)
                     return false;
@@ -171,13 +175,14 @@ public boolean equalInArbitraryOrder(Object o) {
     @Override
     public int hashCode() {
         int result = type != null ? type.hashCode() : 0;
-        result = 31 * result + (colRefs != null ? colRefs.hashCode() : 0);
+        result = 31 * result + (colRef != null ? colRef.hashCode() : 0);
         return result;
     }
 
     @Override
     public String toString() {
-        return "ParameterDesc [type=" + type + ", value=" + value + ", nextParam=" + nextParameter + "]";
+        String thisStr = isColumnType() ? colRef.toString() : value;
+        return nextParameter == null ? thisStr : thisStr + "," + nextParameter.toString();
     }
 
     /**
@@ -201,11 +206,9 @@ public boolean isColumnType() {
         static Set<PlainParameter> createFromParameterDesc(ParameterDesc parameterDesc) {
             Set<PlainParameter> result = Sets.newHashSet();
             ParameterDesc local = parameterDesc;
-            List<TblColRef> totalColRef = parameterDesc.colRefs;
-            Integer colIndex = 0;
             while (local != null) {
                 if (local.isColumnType()) {
-                    result.add(createSingleColumnParameter(local, totalColRef.get(colIndex++)));
+                    result.add(createSingleColumnParameter(local));
                 } else {
                     result.add(createSingleValueParameter(local));
                 }
@@ -221,11 +224,11 @@ static PlainParameter createSingleValueParameter(ParameterDesc parameterDesc) {
             return single;
         }
 
-        static PlainParameter createSingleColumnParameter(ParameterDesc parameterDesc, TblColRef colRef) {
+        static PlainParameter createSingleColumnParameter(ParameterDesc parameterDesc) {
             PlainParameter single = new PlainParameter();
             single.type = parameterDesc.type;
             single.value = parameterDesc.value;
-            single.colRef = colRef;
+            single.colRef = parameterDesc.colRef;
             return single;
         }
 
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java b/core-metadata/src/main/java/org/apache/kylin/metadata/model/TblColRef.java
@@ -54,12 +54,18 @@ public static boolean contains(String name) {
 
     // used by projection rewrite, see OLAPProjectRel
     public static TblColRef newInnerColumn(String columnName, InnerDataTypeEnum dataType) {
+        return newInnerColumn(columnName, dataType, null);
+    }
+    
+    // used by projection rewrite, see OLAPProjectRel
+    public static TblColRef newInnerColumn(String columnName, InnerDataTypeEnum dataType, String parserDescription) {
         ColumnDesc column = new ColumnDesc();
         column.setName(columnName);
         TableDesc table = new TableDesc();
         column.setTable(table);
         TblColRef colRef = new TblColRef(column);
         colRef.markInnerColumn(dataType);
+        colRef.parserDescription = parserDescription;
         return colRef;
     }
     
@@ -101,6 +107,7 @@ public static TblColRef mockup(TableDesc table, int oneBasedColumnIndex, String
     private TableRef table;
     private ColumnDesc column;
     private String identity;
+    private String parserDescription;
 
     TblColRef(ColumnDesc column) {
         this.column = column;
@@ -198,6 +205,9 @@ public String getIdentity() {
 
     @Override
     public String toString() {
+        if (isInnerColumn() && parserDescription != null)
+            return parserDescription;
+        
         String alias = table == null ? "UNKNOWN_MODEL" : table.getAlias();
         String tableName = column.getTable() == null ? "NULL" : column.getTable().getName();
         String tableIdentity = column.getTable() == null ? "NULL" : column.getTable().getIdentity();
diff --git a/kylin-it/src/test/resources/query/sql_casewhen/query04.sql b/kylin-it/src/test/resources/query/sql_casewhen/query04.sql
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
@@ -262,9 +262,7 @@ private void buildAggregations() {
                 }
                 for (Integer index : aggCall.getArgList().subList(0, columnsCount)) {
                     TblColRef column = inputColumnRowType.getColumnByIndex(index);
-                    if (!column.isInnerColumn()) {
-                        columns.add(column);
-                    }
+                    columns.add(column);
                 }
                 if (!columns.isEmpty()) {
                     parameter = ParameterDesc.newInstance(columns.toArray(new TblColRef[columns.size()]));
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
@@ -204,7 +204,7 @@ private TblColRef translateRexCall(RexCall call, ColumnRowType inputColumnRowTyp
         for (RexNode operand : call.getOperands()) {
             translateRexNode(operand, inputColumnRowType, fieldName, sourceCollector);
         }
-        return TblColRef.newInnerColumn(fieldName, InnerDataTypeEnum.LITERAL);
+        return TblColRef.newInnerColumn(fieldName, InnerDataTypeEnum.LITERAL, call.toString());
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -262,9 +262,7 @@ private void buildAggregations() {`
`262`	`262`	`}`
`263`	`263`	`for (Integer index : aggCall.getArgList().subList(0, columnsCount)) {`
`264`	`264`	`TblColRef column = inputColumnRowType.getColumnByIndex(index);`
`265`		`- if (!column.isInnerColumn()) {`
`266`		`- columns.add(column);`
`267`		`- }`
	`265`	`+ columns.add(column);`
`268`	`266`	`}`
`269`	`267`	`if (!columns.isEmpty()) {`
`270`	`268`	`parameter = ParameterDesc.newInstance(columns.toArray(new TblColRef[columns.size()]));`
Original file line number	Diff line number	Diff line change
`@@ -204,7 +204,7 @@ private TblColRef translateRexCall(RexCall call, ColumnRowType inputColumnRowTyp`
`204`	`204`	`for (RexNode operand : call.getOperands()) {`
`205`	`205`	`translateRexNode(operand, inputColumnRowType, fieldName, sourceCollector);`
`206`	`206`	`}`
`207`		`- return TblColRef.newInnerColumn(fieldName, InnerDataTypeEnum.LITERAL);`
	`207`	`+ return TblColRef.newInnerColumn(fieldName, InnerDataTypeEnum.LITERAL, call.toString());`
`208`	`208`	`}`
`209`	`209`
`210`	`210`	`@Override`