[GitHub] carbondata pull request #2139: [CARBONDATA-2267] [Presto] Support Reading Ca...

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2139#discussion_r180553823
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java ---
    @@ -69,6 +72,13 @@
     
       private static Map<Integer, Expression> filterMap = new HashMap<>();
     
    +  private final static String HIVE_DEFAULT_DYNAMIC_PARTITION = "__HIVE_DEFAULT_PARTITION__";
    --- End diff --
   
    can you explain, why need to add "HIVE_DEFAULT_DYNAMIC_PARTITION" ? what purpose?
    if (value == null) {
            filter.add(carbonDataColumnHandle.getColumnName() + "=" + HIVE_DEFAULT_DYNAMIC_PARTITION);


---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2139#discussion_r180554217
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java ---
    @@ -69,6 +72,13 @@
     
       private static Map<Integer, Expression> filterMap = new HashMap<>();
     
    +  private final static String HIVE_DEFAULT_DYNAMIC_PARTITION = "__HIVE_DEFAULT_PARTITION__";
    +  private final static String PARTITION_VALUE_WILDCARD = "";
    --- End diff --
   
    Why need to add "wildcard" for partition filter?


---

[qiuchenjian-2]

Github user geetikagupta16 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2139#discussion_r180649360
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java ---
    @@ -69,6 +72,13 @@
     
       private static Map<Integer, Expression> filterMap = new HashMap<>();
     
    +  private final static String HIVE_DEFAULT_DYNAMIC_PARTITION = "__HIVE_DEFAULT_PARTITION__";
    --- End diff --
   
    HIVE_DEFAULT_DYNAMIC_PARTITION is created when the partition value for that column is null so the data goes to __HIVE_DEFAULT_PARTITION__ partition. This check is added so that we can query the data from __HIVE_DEFAULT_PARTITION__ as well.


---

[qiuchenjian-2]

Github user geetikagupta16 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2139#discussion_r180672102
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java ---
    @@ -69,6 +72,13 @@
     
       private static Map<Integer, Expression> filterMap = new HashMap<>();
     
    +  private final static String HIVE_DEFAULT_DYNAMIC_PARTITION = "__HIVE_DEFAULT_PARTITION__";
    +  private final static String PARTITION_VALUE_WILDCARD = "";
    --- End diff --
   
    In hive PARTITION_VALUE_WILDCARD is used to select all the partitions from the table when we don't have any filter conditions on the partition column in our query, but in our code we are already quering all the partitions for the above case. So we can remove this part.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2139
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/4940/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2139
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3721/



---

[qiuchenjian-2]

Github user chenliang613 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2139#discussion_r180789005
 
    --- Diff: integration/presto/src/main/java/org/apache/carbondata/presto/PrestoFilterUtil.java ---
    @@ -69,6 +72,13 @@
     
       private static Map<Integer, Expression> filterMap = new HashMap<>();
     
    +  private final static String HIVE_DEFAULT_DYNAMIC_PARTITION = "__HIVE_DEFAULT_PARTITION__";
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user chenliang613 commented on the issue:

    https://github.com/apache/carbondata/pull/2139
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/2139


---