[GitHub] [kylin] xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation
xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation URL: https://github.com/apache/kylin/pull/528#discussion_r268373480 ## File path: core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java ## @@ -316,12 +321,22 @@ public boolean equals(Object obj) { } else { return parameter.equalInArbitraryOrder(other.parameter); } -} else if (!isCount()) { // NOTE: don't check the parameter of count() +} else { if (parameter == null) { -if (other.parameter != null) +if (isCount()) { +if (other.parameter != null && !other.parameter.getType().equals("constant")) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [kylin] xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation
xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation URL: https://github.com/apache/kylin/pull/528#discussion_r268373494 ## File path: core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java ## @@ -289,7 +293,8 @@ public int hashCode() { final int prime = 31; int result = 1; result = prime * result + ((expression == null) ? 0 : expression.hashCode()); -result = prime * result + ((isCount() || parameter == null) ? 0 : parameter.hashCode()); +result = prime * result + ((parameter == null || (isCount() && parameter.getType().equals("constant"))) Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [kylin] xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation
xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation URL: https://github.com/apache/kylin/pull/528#discussion_r268373487 ## File path: core-metadata/src/main/java/org/apache/kylin/metadata/model/FunctionDesc.java ## @@ -146,7 +146,11 @@ public boolean needRewriteField() { public String getRewriteFieldName() { if (isCount()) { -return "_KY_" + "COUNT__"; // ignores parameter, count(*), count(1), count(col) are all the same +if (parameter == null || parameter.getType().equals("constant")) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [kylin] xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation
xbirbird commented on a change in pull request #528: KYLIN-3883 Kylin supports column count aggregation URL: https://github.com/apache/kylin/pull/528#discussion_r268405678 ## File path: core-cube/src/main/java/org/apache/kylin/cube/util/KeyValueBuilder.java ## @@ -91,12 +91,18 @@ private String getCell(int i, String[] flatRow) { int colParamIdx = 0; // index among parameters of column type for (int i = 0; i < paramCount; i++, param = param.getNextParameter()) { String value; -if (function.isCount()) { -value = "1"; -} else if (param.isColumnType()) { +if (param.isColumnType()) { value = getCell(colIdxOnFlatTable[colParamIdx++], row); +if (function.isCount() && value == null) { +value = "0"; Review comment: Every cell of the base cuboid needs a value, otherwise it will cause the IndexOutOfBoundsException. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services