Repository navigation
[Java] Type-ids in UnionVector are erroneously coupled to the Arrow types of the underlying vectors #108
Description
Activity
Would there be an appetite for (potentially relatively significant) changes to UnionVector to make it behave this way? We could perhaps consider bringing it more in line with DenseUnionVector which seems closer to the spec.
+1. It would of course be better if a way of doing this without breaking compatibility with existing Java code is found.
Reacted by Vibhatha Lakmal AbeykoonAre you speaking of significant API changes, or significant implementation changes? The latter is fine so long as existing tests pass.
I'm thinking of significant API changes.
I think there need to be significant (breaking) API changes if you want to make it spec compliant.
Reacted by Vibhatha Lakmal Abeykoon+1 for implementation changes preserving compatibility and current test coverage.
Any idea on the maginitude of this change?If this is spec compliant, meaning it changes the existing spec, would it require a vote on ML?
This may sound harsh, but for me the current UnionVector API is broken in the light of the mailing list discussion above. Even looking at the first test of UnionVector, a line like
https://github.com/apache/arrow/blob/41a989c81616aea103e554521fa6d6209ffa248d/java/vector/src/test/java/org/apache/arrow/vector/TestUnionVector.java#L78
doesn't make sense. You could probably make a lot of the methods that exist work with only internal changes, but I would still argue for deprecation to discourage future use.Any idea on the maginitude of this change?
A significant rewrite of UnionVector. I might even start from scratch basing it more on the DenseUnionVector implementation.
Reacted by Vibhatha Lakmal Abeykoon and Martin TraverseYes, we should definitely deprecate the ill-designed APIs. But it's better if they can still work as originally until people migrate their code to the new APIs.
Reacted by Finn Völkel, Vibhatha Lakmal Abeykoon and James HendersonThat's what I'm suggesting, yes.
Reacted by Vibhatha Lakmal AbeykoonAre there any plans on fixing this? It's inconvenient that
UnionVector#setTypeandDenseUnionVector#setTypeIddeviate from one another.Unfortunately there's just very little maintainer power for arrow-java and what's left is mostly focused on trying to keep things running (e.g. trying to set up release processes again now that we've split the repo) :/
It frustrates me too and I wish I had time to help out and fix things, improve the library, keep Arrow abreast of things like the new native memory interface, etc. but that's just not happening with my current development time/goals
Thanks for sharing; I totally understand the very real issue of not having enough resources.
It's doubly frustrating because I feel like the Java library gets certain design decisions right vs the other implementations 😬
Unfortunately we probably need some sponsor or champion to fund development...The lack of maintainers is also sort of self-perpetuating since there's not many people to review things in the first place
Maybe I can take a look after the releases. I think arrow-Java needs some love (and I have a lot of love to give :)).
Reacted by Gang Wu and James HendersonJust to add to this, the union vector writers could also use revisiting as part of this work. The dense writer seems to have a bug where the child writers are not initialized. But more generally both writers have a lot of things in them that the format spec doesn't talk about.
- added a commit that references this issue
on Apr 23, 2025
Describe the bug, including details regarding any error messages, version, and platform.
re: https://lists.apache.org/thread/z89xlvw7v1rwq89gknflhsj3c65x20kd
It seems that the UnionVector implementation (particularly
initializeChildrenFromFields(#29848),getVector,getVectorByType,setSafeetc) assumes that the type-id is always based on the ArrowType, but the Schema.fbs spec is more lenient - users have the choice to use whatever type-ids they require.For example, in XTDB, we're trying to represent an algebraic data type (ADT) of 'put', 'delete' and 'erase' events as a sparse union. Delete and erase have the same type, so UnionVector currently expects them to be the same type-id (whereas, in DenseUnionVector, we can use type-ids 0, 1 and 2).
Would there be an appetite for (potentially relatively significant) changes to UnionVector to make it behave this way? We could perhaps consider bringing it more in line with DenseUnionVector which seems closer to the spec. Would be happy to work on it if so.
Cheers,
James/Finn (@FiV0)
Component(s)
Java