Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-7227][Table API & SQL]Fix the the TableSource predicate push… #4608

Closed
wants to merge 1 commit into from

Conversation

uybhatti
Copy link
Contributor

@uybhatti uybhatti commented Aug 28, 2017

… down issue for OR and AND expression with more than 2 predicates

What is the purpose of the change

Currently, we can't push more than 2 predicates for OR expression in TableSource. In this PR, we want to solve this issue.

Brief change log

  • Made changes in RexNodeToExpressionConverter

For predicate push down, first we are converting expression to conjunctive normal form then passing to RexNodeToExpressionConverter. So issue was only generated for OR expression, but we handle this issue for non-conjunctive normal form to solve for AND expression.

Verifying this change

  • Add tests to verify that RexNodeToExpressionConverter is handling conjunctive and non-conjunctive normal forms successfully.

    • RexProgramExtractorTest#testExtractORForPredicatePushDown
    • RexProgramExtractorTest#testExtractNonCnfANDForPredicatePushDown
    • RexProgramExtractorTest#testExtractCnfANDForPredicatePushDown
  • To verify predicate push down for OR and AND expression following test is added

    • AddTableSourceTest#testBatchFilterableFullyPushedDownORANDExpressions

    this test depends on TestFilterableTableSource#applyPredicate function, so we made changes in applyPredicate function to verify this test.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving):no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

… down issue for OR and AND expression with more than 2 predicates
@fhueske
Copy link
Contributor

fhueske commented Aug 30, 2017

Hi @uybhatti, thanks for opening a PR. Please read and fill out the template in the description of the pull request. Thank you.

fhueske pushed a commit to fhueske/flink that referenced this pull request Sep 5, 2017
@fhueske
Copy link
Contributor

fhueske commented Sep 5, 2017

thanks for the PR. I'll adjust some tests and will merge it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants