-
Notifications
You must be signed in to change notification settings - Fork 539
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
WIP: Support union types #1011
WIP: Support union types #1011
Conversation
For Unhandled type uniontype<int,string>
(Expected to fail)
byte mask = 0; | ||
|
||
mask |= value.isBoolean() ? BOOLEAN_MASK : 0; | ||
mask |= value.isNumber() ? NUMBER_MASK : 0; |
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.
Why continuously |=? You can only has one type mask
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.
Thanks for the quick response. When I started working on this, at the beginning I thought for reason that two of the JSON types could co-exist in a single value. I was probably thinking of a case where a JSON value may be either null or not-null (in a sense that both {"key":"value"}
and {"key":null}
are valid), which clearly does not apply here. That was me being silly. Perhaps I should replace the whole mask stuff with enum
.
a9cb26b
to
a832d58
Compare
lgtm |
Follow-up of #1011 (Support union types)
While working on #982, I've gained some knowledge on how ORC format works. So I might as well implement
UnionColumnConverter
to support union types.This is more like a proposal/brainstorming rather than a complete solution. As marked as
TODO
orFIXME
there are some pending tasks as well. Any feedback and discussions are welcomed.