-
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
Adds PartiQLFunction.Aggregation and updates Plugin interface #1310
Conversation
Conformance comparison report
Number passing in both: 5384 Number failing in both: 434 Number passing in Base (5c1240c) but now fail: 0 Number failing in Base (5c1240c) but now pass: 0 |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1310 +/- ##
============================================
- Coverage 72.36% 72.16% -0.20%
+ Complexity 2099 2097 -2
============================================
Files 221 221
Lines 15973 15974 +1
Branches 2892 2892
============================================
- Hits 11559 11528 -31
- Misses 3599 3640 +41
+ Partials 815 806 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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. Good work!
a6adc59
to
f53f370
Compare
partiql-spi/src/main/kotlin/org/partiql/spi/function/PartiQLFunction.kt
Outdated
Show resolved
Hide resolved
* @param config | ||
* @return | ||
*/ | ||
public fun create(catalogName: String, config: StructElement? = null): Connector |
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.
Curious how can the config be null?
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.
See memory connector which doesn't need a config. It's just extra options to pass the factory. If you're loading from a file or something the persisting options in a struct is useful, but memory connector just closes over its additional parameters.
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. Thanks.
Relevant Issues
N/A
Description
This PR updates the Plugin inteface to use Kotlin fields and updates Plugin to return a single factory as multiple is not needed.
This PR also introduces the experimental definition of a PartiQLFunction.Aggregation which will be used for aggregation user-defined-functions.
Other Information
Updated Unreleased Section in CHANGELOG: [YES/NO]
Yes
Any backward-incompatible changes? [YES/NO]
Yes
Any new external dependencies? [YES/NO]
No
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES/NO]
Yes
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.