You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kylin.apache.org by GitBox <gi...@apache.org> on 2018/12/15 13:17:50 UTC

[GitHub] shaofengshi commented on a change in pull request #394: KYLIN-3720 add column family check when save/update cube desc

shaofengshi commented on a change in pull request #394: KYLIN-3720 add column family check when save/update cube desc
URL: https://github.com/apache/kylin/pull/394#discussion_r241949448
 
 

 ##########
 File path: server-base/src/main/java/org/apache/kylin/rest/controller/CubeController.java
 ##########
 @@ -608,6 +613,26 @@ public CubeRequest saveCubeDesc(@RequestBody CubeRequest cubeRequest) {
         return cubeRequest;
     }
 
+    //column family metrics may not match the real metrics when editing cube by json
+    private void validateColumnFamily(CubeDesc cubeDesc){
+        Set<String> columnFamilyMetricsSet = Sets.newHashSet();
+        for(HBaseColumnFamilyDesc hBaseColumnFamilyDesc : cubeDesc.getHbaseMapping().getColumnFamily()) {
+            for(HBaseColumnDesc hBaseColumnDesc : hBaseColumnFamilyDesc.getColumns()){
+                for(String columnName : hBaseColumnDesc.getMeasureRefs()){
+                    columnFamilyMetricsSet.add(columnName);
+                }
+            }
+        }
+        for(MeasureDesc measureDesc : cubeDesc.getMeasures()){
 
 Review comment:
   Hi bo, the function looks good, but it seems the code is not formatted. Could you reformat it by following the "IDE code formatter" in https://kylin.apache.org/development/dev_env.html ?
   
   Besides, the "merge" commit should be avoid.
   
   Thank you for making Kylin better!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services