-
Notifications
You must be signed in to change notification settings - Fork 17
feat(types): add comprehensive type support and stricter validation #130
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
Add wide-ranging support for additional scalar and parameterized Substrait types, improve parameter handling, and make nullability/precision checks stricter and more correct. Signed-off-by: MBWhite <[email protected]>
|
tokoko
left a comment
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 couple comments, but looks good to me. this is great, thanks.
bestbeforetoday
left a comment
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.
Content looks good to me. Like tokoko, just one query/suggestion on code clarity (see inline comment). It's not a show-stopper though. If you want to change what is there, please go ahead. If not, just say and I think it could be merged as-is.
|
I've refactor the code as you suggested @bestbeforetoday |
bestbeforetoday
left a comment
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.
Thank you for the updates. That looks much clearer to me.
Expanded type support with new scalar and parameterized types.
Improved parameter validation and stricter nullability/precision checks.