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

refactor: move expressions into ksql-execution #3194

Merged

Conversation

rodesai
Copy link
Contributor

@rodesai rodesai commented Aug 12, 2019

Description

This patch looks big, but its really just moving the expression tree into ksql-execution
under the package io.confluent.ksql.execution.expression.tree.

The only major code change required to do this was to make the reserved word check done
in ExpressionFormatter a parameter, and to write a thin wrapper in ksql-parser that passes
in the predicate that checks for words in the KSQL grammar. ExpressionFormatter is now in
ksql-execution, and the wrappers are in a new util class called ExpressionFormatterUtil.

Testing done

No new tests since we're just moving stuff around.

@rodesai rodesai requested a review from a team as a code owner August 12, 2019 00:48
This patch looks big, but its really just moving the expression tree into ksql-execution
under the package io.confluent.ksql.execution.expression.tree.

The only big code change required to do this was to make the reserved word check done
in ExpressionFormatter a parameter, and to write a thin wrapper in ksql-parser that passes
in the predicate that checks for words in the KSQL grammar.
@rodesai rodesai force-pushed the move-expressions-to-execution-rebase branch from d74a0c3 to 7c83377 Compare August 12, 2019 00:52
Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! two copyrights

@rodesai rodesai merged commit 147336a into confluentinc:master Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants