-
Notifications
You must be signed in to change notification settings - Fork 63
Inferencer removal #1302
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
Inferencer removal #1302
Conversation
Conformance comparison report
Number passing in both: 5384 Number failing in both: 434 Number passing in Base (ebfc93f) but now fail: 0 Number failing in Base (ebfc93f) but now pass: 0 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1302 +/- ##
============================================
+ Coverage 72.21% 72.37% +0.15%
+ Complexity 2100 2099 -1
============================================
Files 222 221 -1
Lines 16021 15971 -50
Branches 2892 2889 -3
============================================
- Hits 11570 11559 -11
+ Misses 3635 3597 -38
+ Partials 816 815 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -59,6 +59,7 @@ Thank you to all who have contributed! | |||
- **Breaking** Removed IR factory in favor of static top-level functions. Change `Ast.foo()` | |||
to `foo()` | |||
- **Breaking** Removed `org.partiql.lang.planner.transforms.AstToPlan`. Use `org.partiql.planner.PartiQLPlanner`. | |||
- **Breaking** Removed `org.partiql.lang.planner.transforms.PartiQLSchemaInferencer`. In order to achieve the same functionality, one would need to use the `org.partiql.planner.PartiQLPlanner`. |
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.
Maybe add an example? For "same functionality" it's really just getting the query return type.
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. Just echoing Robert's comment about an example.
Relevant Issues
Description
Other Information
Updated Unreleased Section in CHANGELOG: [YES/NO]
Any backward-incompatible changes? [YES/NO]
Planner
API.Any new external dependencies? [YES/NO]
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES/NO]
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.