-
Notifications
You must be signed in to change notification settings - Fork 63
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
Initializes PartiQL's Engine #1283
Conversation
Conformance comparison report
Number passing in both: 5372 Number failing in both: 446 Number passing in Base (f8f6b2a) but now fail: 0 Number failing in Base (f8f6b2a) but now pass: 0 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## partiql-eval #1283 +/- ##
===============================================
Coverage ? 49.46%
Complexity ? 1047
===============================================
Files ? 166
Lines ? 13391
Branches ? 2500
===============================================
Hits ? 6624
Misses ? 6103
Partials ? 664
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
partiql-engine/src/main/kotlin/org/partiql/engine/PartiQLEngine.kt
Outdated
Show resolved
Hide resolved
@OptIn(PartiQLValueExperimental::class) | ||
override fun execute(plan: PartiQLPlan): PartiQLEngine.Result { | ||
val expression = PlanToPhysical.convert(plan) | ||
val value = expression.evaluate(Record(emptyList())) | ||
return PartiQLEngine.Result.Success(value) | ||
} |
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.
What customers might like is "prepare" and "execute" methods. This would enable re-use of a compiled expression which customers have asked for
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.
ACK -- in my opinion, once we determine what we should expose, prepare
will be a very nice addition. For now, I've kept all compilation/execution internal.
interface Relation : PhysicalNode { | ||
fun evaluate(): Iterator<Record> | ||
} |
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.
Record on the Iterator .. it's a reference anyways .. nice and simple.
partiql-engine/src/main/kotlin/org/partiql/engine/impl/Record.kt
Outdated
Show resolved
Hide resolved
0ffb6d9
to
cc736ac
Compare
cc736ac
to
8eb3eee
Compare
Relevant Issues
Description
Other Information
Updated Unreleased Section in CHANGELOG: [YES/NO]
Any backward-incompatible changes? NO
Any new external dependencies? 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.