[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-657194095 Thanks for the rebate @wesm! Glad we have finished this one! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-656731970 It is indeed the V4 patch that has been submitted on the c++ side and not on the Java side. I think this means we need to submit your patch first @lidavidm? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-656664816 > Seems like Java needs to be excluded from testing the 0.17.1 "golden" reference union file. Also, Java produces data that C++ can't read, but Java can read the C++ files; this fails the IPC tests with Java producing, and the Flight tests with both Java producing and consuming (since Flight tests upload and download). Wonder if its the MetadataVersion change that went in for c++ today? Looking now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-656636105 Just rebased to include the memory related changes. Avoid any surprise broken master builds. @liyafan82 think its ready to go? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-652516529 This has been modified to incorporate the changes to Unions as proposed on the mailing list This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-651630439 I _think_ this is now inline with the spec. The Union/DenseUnion types now uses logical type ids in Java. Which is the same as in c++. The difference in a java created Union is that the type ids are chosen from a known index (minor type). But the java implementation doesn't rely on minor type ordinals strict ordering like before. Or at least the intention was to be fully in line with spec while maintaining a convenient default choice for type id. Drawing any meaning from type buffers alone in client code would be an error in my opinion. Hope that clarifies my intention w/ this change. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-648810719 Thanks @wesm and @jacques-n for the review. I will leave this up until consensus is reached on the format change. Please let me know if I can help w/ the c++ patch, would be happy to contribute! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] rymurr commented on pull request #7290: ARROW-1692: [Java] UnionArray round trip not working
rymurr commented on pull request #7290: URL: https://github.com/apache/arrow/pull/7290#issuecomment-642647124 > It looks like the integration tests are still failing for union arrays between C++ and Java. yeah, something w/ Flight, though the IPC works fine. Checking what is different in flight now... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
