[GitHub] [hudi] alexeykudinkin commented on a diff in pull request #6725: [HUDI-4881] Push down filters if possible when syncing partitions to Hive

[GitBox <gi...@apache.org>]

alexeykudinkin commented on code in PR #6725:
URL: https://github.com/apache/hudi/pull/6725#discussion_r997365241


##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java:
##########
@@ -307,14 +315,161 @@ private boolean syncSchema(String tableName, boolean tableExists, boolean useRea
     return schemaChanged;
   }
 
+  /**
+   * Combine filter with existing filters.
+   *
+   * 1. If no filter left in the filterBuilder, will simply add it.
+   * 2. If filterBuilder is not empty, will combine the existing filters
+   *    and new adding filter with operator if the filter is not empty,
+   *    otherwise return empty for OR operator; while return left side
+   *    for ADD operator. Wrap these filters with parenthesis.
+   */
+  private void combineFilter(StringBuilder filterBuilder, String operator, Option<String> filter) {

Review Comment:
   +1
   
   I actually would like to extend on @xushiyan proposal and recommend following: let's decouple filter generation from actual serialization, meaning
   
   1. Create some basic tree-structure w/ the expressions capturing filtering conditions (And, Or, EqualTo, etc)
   2. Serialize that tree-structure using the rules/restrictions applicable to Hive
   
   This will help to 
   
   - Decouple filter generation from actual serialization
   - Will make it easier to modify one w/o affecting the other



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org