-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update Core to 2.18 #325
Update Core to 2.18 #325
Conversation
This pull request has been linked to Shortcut Story #29964: [CSharp] Support aggregate pushdown. |
`tiledb_channel_operator` does not have a free function so it does not need a safe handle.
39437b4
to
4822e62
Compare
/// <remarks> | ||
/// This class abstracts the <c>tiledb_channel_operation_t</c> type of the TileDB C API. | ||
/// </remarks> | ||
public abstract class AggregateOperation |
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 named this AggregateOperation
instead of ChannelOperation
or QueryChannelOperation
. I believe it conveys its meaning better, and the first alternative is confusing (what is a channel?), and the second alternative is long.
This is a managed class that does not directly contain a tiledb_channel_operation_t*
. Instead it encapsulates the logic to create it, which gets done when actually passed to the C API.
/// <remarks> | ||
/// This class abstracts the <c>tiledb_channel_operator_t</c> type of the TileDB C API. | ||
/// </remarks> | ||
public abstract class AggregateOperator |
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.
Same as AggregateOperation
.
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.
Left a few comments, mostly small fixes for docs or questions on error handling. Looks really great overall 👍
Feedback addressed, CI is green, I'm going to merge it… |
SC-29964