-
Notifications
You must be signed in to change notification settings - Fork 71
GH-655: Failure in UnionReader.read after DecimalVector promotion to UnionVector #656
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
…o UnionVector (apache#61) When a DecimalVector is promoted to a UnionVector via a PromotableWriter, the UnionVector will have the decimal vector in it's internal struct vector, but the decimalVector field will not be set. If UnionReader.read is then used to read from the UnionVector, it will fail when it tries to read one of the promoted decimal values, due to decimalVector being null, and the exact decimal type not being provided. This failure is unnecessary though as we have a pre-existing decimal vector, the caller just does not know the exact type - and it shouldn't be required to. The change here is to check for a pre-existing decimal vector in the internal struct when getDecimalVector() is called. If one exists, set the decimalVector field and return. Otherwise, if none exists, throw the exception.
This comment has been minimized.
This comment has been minimized.
@@ -306,6 +307,9 @@ public StructVector getStruct() { | |||
|
|||
public ${name}Vector get${name}Vector(String name) { | |||
if (${uncappedName}Vector == null) { | |||
${uncappedName}Vector = internalStruct.getChild(fieldName(MinorType.${name?upper_case}), ${name}Vector.class); |
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.
Should we apply to only child, or also on parameterized types ?
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'm not sure, I'm submitting a change someone else made in our repo. Would it be any different if used the parameterized type instead?
It seems there's a syntax error still |
I fixed the syntax error but not sure whats causing the latest Dev PR task error: |
That's separate (I think I need to grant that step permissions to update issues, too) |
Are you able to |
Thanks! |
### What changes were proposed in this pull request? This pr aims to upgrade `arrow-java` from 18.2.0 to 18.3.0. ### Why are the changes needed? The new version bring some bug fixes, like: - apache/arrow-java#627 - apache/arrow-java#654 - apache/arrow-java#656 - apache/arrow-java#693 - apache/arrow-java#705 - apache/arrow-java#707 - apache/arrow-java#722 In addition, the new version introduces a cascading upgrade for flatbuffers-java([ from 24.3.25 to 25.1.24 ](apache/arrow-java#600)) the full release note as follows: - https://github.com/apache/arrow-java/releases/tag/v18.3.0 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Pass GitHub Acitons ### Was this patch authored or co-authored using generative AI tooling? No Closes #50892 from LuciferYang/arrow-java-18.3.0. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
### What changes were proposed in this pull request? This pr aims to upgrade `arrow-java` from 18.2.0 to 18.3.0. ### Why are the changes needed? The new version bring some bug fixes, like: - apache/arrow-java#627 - apache/arrow-java#654 - apache/arrow-java#656 - apache/arrow-java#693 - apache/arrow-java#705 - apache/arrow-java#707 - apache/arrow-java#722 In addition, the new version introduces a cascading upgrade for flatbuffers-java([ from 24.3.25 to 25.1.24 ](apache/arrow-java#600)) the full release note as follows: - https://github.com/apache/arrow-java/releases/tag/v18.3.0 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Pass GitHub Acitons ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#50892 from LuciferYang/arrow-java-18.3.0. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
When a DecimalVector is promoted to a UnionVector via a PromotableWriter, the UnionVector will have the decimal vector in it's internal struct vector, but the decimalVector field will not be set. If UnionReader.read is then used to read from the UnionVector, it will fail when it tries to read one of the promoted decimal values, due to decimalVector being null, and the exact decimal type not being provided. This failure is unnecessary though as we have a pre-existing decimal vector, the caller just does not know the exact type - and it shouldn't be required to.
The change here is to check for a pre-existing decimal vector in the internal struct when getDecimalVector() is called. If one exists, set the decimalVector field and return. Otherwise, if none exists, throw the exception.
What's Changed
Updated UnionVector to handle this case.
Closes GH-655