-
Notifications
You must be signed in to change notification settings - Fork 13
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
Initial commit of substrait-cpp #2
Conversation
Due to can't re-open PR for #1, so create this new PR |
Thanks for addressing my earlier comments. I made another pass today and appreciate all the work! 😃 |
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
@westonpace, Thank you for your patience to reviewing again, and I have updated this PR according your reviews. |
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.
Apologies for the delayed review. I have one more nitpick about variant vs. implementation terminology but we can address that in a follow-up. This is getting rather large to continue reviewing and it is close enough I want to merge now.
void addScalarFunctionVariant(const FunctionImplementationPtr& functionVariant); | ||
|
||
/// Add a aggregate function variant. | ||
void addAggregateFunctionVariant(const FunctionImplementationPtr& functionVariant); | ||
|
||
/// Add a window function variant. | ||
void addWindowFunctionVariant(const FunctionImplementationPtr& functionVariant); |
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.
We should probably rename all these addXyzVariant
to addXyzImplementation
.
This PR is an Initial commit of substrait-cpp which includes following features: