-
Notifications
You must be signed in to change notification settings - Fork 62
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
Bitwise and plan typer and WIki. #1232
Conversation
Conformance comparison report
Number passing in both: 5372 Number failing in both: 446 Number passing in Base (4e2a184) but now fail: 0 Number failing in Base (4e2a184) but now pass: 0 |
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.
Looks good! Minor comments and some additional tests you could add
: Else return `INT2` type. | ||
|
||
Note: | ||
: Type precedence of the bitwise operator is lower than the plus/minus operator and higher than the predicates. |
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.
Could link the partiql-docs issue since this could change: https://github.com/partiql/partiql-docs/issues/50.
query = "CAST(1 AS INT8) & 2", | ||
expected = StaticType.unionOf(StaticType.INT, MISSING) | ||
), | ||
|
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.
nice set of tests! could also add
- test w/ null propagation (1 & NULL) -> NULL
- test w/ missing propagation (1 & MISSING) -> MISSING and (NULL & MISSING) -> MISSING
- data type mismatch (1 & 'non-int') -> error and output type of union of int types
2722bac
to
642c6d1
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1232 +/- ##
============================================
+ Coverage 70.67% 70.80% +0.12%
- Complexity 2335 2353 +18
============================================
Files 229 229
Lines 17382 17393 +11
Branches 3199 3201 +2
============================================
+ Hits 12285 12315 +30
+ Misses 4109 4092 -17
+ Partials 988 986 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
I'm a bit late to the party, but |
+1 on that. Loop in @johnedquinn for more context on the error |
Yeah, it's an artifact from the behavior of the |
Relevant Issues
Description
Other Information
Updated Unreleased Section in CHANGELOG: [YES/NO]
Any backward-incompatible changes? [YES/NO]
Any new external dependencies? [YES/NO]
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES/NO]
No.
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.