Github user aljoscha commented on the pull request:
https://github.com/apache/flink/pull/1127#issuecomment-143703367
Hi,
I like the work. :smile:
Some remarks about Scala style. We are trying to make our Scala code more
consistent. Mostly, I didn't comment everything, just one particular pattern of
code that could be changed.
Now, for the technical part, you are not actually removing the predicates
that are pushed down from an expression, correct? This should be alright, since
the result will still be correct. It might just be some future optimization,
but I also think that the cost of evaluating the predicate is negligible. The
real improvement comes from early filtering in the sources, as you implemented.
Then, why do you have the `supports*` methods in `AdaptiveTableSource`.
Couldn't these methods just do nothing in case the source does not support the
feature. Or maybe return false if the pushdown was not successful. (I also
wonder why you have the differentiation between AdaptiveSources that support
pushdown and those that don't in `JavaBatchTranslator.createTable`. (I think
you do it so that stuff does not get pushed to sources that don't support it,
but this distinction might not be necessary, as mentioned above.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
