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
[BEAM-4044] [SQL] Make BeamCalciteTable self planning #5154
Conversation
R: @kennknowles This is a refactor PR contained entirely in |
25636a7
to
553b41c
Compare
run java precommit |
4 similar comments
run java precommit |
run java precommit |
run java precommit |
run java precommit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a nice simplification, leveraging Calcite.
@Override | ||
public void flattenRel(RelStructuredTypeFlattener flattener) { | ||
// rewriteGeneric calls this.copy. Setting isFlattining passes | ||
// this context into copy for modificaiton of the flattened flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Thanks.
15f82d1
to
f873611
Compare
The set of findbugs tests run by maven and gradle are not the same. |
Personally, I think findbugs is not worthwhile once we have Errorprone or Checker. So anything expedient to get past this is fine by me. |
I dropped the Serializable from the table class, which should fix it. The error message suggests adding readObject which makes other code odities make more sense. |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,
CalciteSchema
is a better option to hold all the environment information.
This is some cleanup moving us towards only using the calcite environment. This is required for calcite's builtin DDL support. There is no test changes, functional changes, or public API changes. The core nugget of this change is that the planner rules no longer require access to the BeamSqlEnv.
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue.mvn clean verify
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.