-
Notifications
You must be signed in to change notification settings - Fork 9
Add GPML logical plan and plan -> evaluator. #549
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
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (73.20%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #549 +/- ##
==========================================
- Coverage 81.91% 81.56% -0.35%
==========================================
Files 106 107 +1
Lines 22386 22487 +101
Branches 22386 22487 +101
==========================================
+ Hits 18337 18342 +5
- Misses 3536 3632 +96
Partials 513 513 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Conformance comparison report
Number passing in both: 5603 Number failing in both: 826 Number passing in Base (eb01344) but now fail: 0 Number failing in Base (eb01344) but now pass: 0 |
bdc9ad3
to
d3f90a8
Compare
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. One comment about if what's moved from partiql-eval/src/lib.rs is the same as what's now in partiql-eval/src/eval/expr/graph_match.rs.
@@ -2435,301 +2435,4 @@ mod tests { | |||
assert_eq!(Value::Bag(Box::new(expected)), res); | |||
} | |||
} | |||
|
|||
mod graph { |
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.
I assume this is basically the same as what's now in partiql-eval/src/eval/expr/graph_match.rs
?
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.
yes
f595ebc
to
2bf9bbf
Compare
This PR:
SimpleGraph
; minimal graph needed to read experimental tests #546 into anEvalExpr
rather than aBindOp
(REX
rather than aREL
in PartiQL plan lingo)SimpleGraph
; minimal graph needed to read experimental tests #546Graph
typing supportBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.