[FLINK-27580] Implement filter pushdown for TableStoreHiveStorageHandler #124
Conversation
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Thanks for the contribution, left some comments.
| public Optional<Predicate> convert() { | ||
| try { | ||
| return Optional.of(convertTree(tree)); | ||
| } catch (Throwable t) { |
There was a problem hiding this comment.
I prefer to catch a specific exception to avoid unexpected bugs.
| private static final Logger LOG = | ||
| LoggerFactory.getLogger(SearchArgumentToPredicateConverter.class); | ||
|
|
||
| private final ExpressionTree tree; |
There was a problem hiding this comment.
rename tree to root?
There are several places where the exception reference to tree is wrong, and the same name tends to cause this problem
| } | ||
| } | ||
|
|
||
| private Predicate convertNotTree(ExpressionTree tree) { |
There was a problem hiding this comment.
I think negate logical for not should in flink-table-store-core.
| Preconditions.checkArgument(idx >= 0, "Column " + columnName + " not found."); | ||
| LogicalType columnType = columnTypes.get(idx); | ||
| switch (leaf.getOperator()) { | ||
| case EQUALS: |
There was a problem hiding this comment.
Ditto (I think negate logical for not should in flink-table-store-core.)
| } else { | ||
| return FileStoreImpl.createWithValueCount( | ||
| tableLocation, new FileStoreOptions(options), user, partitionType, rowType); | ||
| private static class FileStoreWrapper { |
There was a problem hiding this comment.
Maybe we should have a FileStoreFactory in flink-table-store-core after #101
Then we can create Predicate from that. (Maybe get row type from it)
2 participants
Filter pushdown is a critical optimization for sources as it can decrease number of records to read. Hive provides a
HiveStoragePredicateHandlerinterface for this purpose. We need to implement this interface inTableStoreHiveStorageHandler.