You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "trxcllnt (via GitHub)" <gi...@apache.org> on 2023/08/11 06:24:58 UTC
[GitHub] [arrow] trxcllnt opened a new pull request, #37122: GH-31621: [JS] Fix Union null bitmaps
trxcllnt opened a new pull request, #37122:
URL: https://github.com/apache/arrow/pull/37122
This PR fixes `Union` null handling, and re-enables the disabled `SparseUnion` test.
The format doc [says](https://arrow.apache.org/docs/format/Columnar.html#union-layout):
> Unlike other data types, unions do not have their own validity bitmap.
Therefore we need to remove null masks from union types and allow them to delegate validity to their children.
Also fixes https://github.com/apache/arrow/issues/37063 for good measure.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] trxcllnt commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "trxcllnt (via GitHub)" <gi...@apache.org>.
trxcllnt commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1696027855
From the logs, it seems like the C# implementation may not be aligning everything to 4-byte boundaries. Is there any way to get the files C# produces to confirm?
```
################# FAILURES #################
FAILED TEST: primitive C# producing, JS consuming
FAILED TEST: null C# producing, JS consuming
FAILED TEST: decimal C# producing, JS consuming
FAILED TEST: datetime C# producing, JS consuming
FAILED TEST: nested C# producing, JS consuming
FAILED TEST: recursive_nested C# producing, JS consuming
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] trxcllnt commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "trxcllnt (via GitHub)" <gi...@apache.org>.
trxcllnt commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1696916776
@eerhardt I've only ever compiled and run the JS, C++, and Java. Don't really wanna install mono, is there a way to do it via docker?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1691904781
@trxcllnt Does this allow removing the skip here?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] eerhardt commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "eerhardt (via GitHub)" <gi...@apache.org>.
eerhardt commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1697499523
> Don't really wanna install mono
You don't need to install mono. .NET runs on Linux and Mac. https://dot.net/download
> is there a way to do it via docker?
Yes, this is how the integration tests run in CI. You can execute the same commands as what CI does:
https://github.com/apache/arrow/blob/29819425f428d703ad5c80cc0afaf7902dbbb224/.github/workflows/integration.yml#L85-L95
You should be able to speed it up by commenting out the languages you don't need to build here:
https://github.com/apache/arrow/blob/29819425f428d703ad5c80cc0afaf7902dbbb224/docker-compose.yml#L1709-L1715
and here:
https://github.com/apache/arrow/blob/29819425f428d703ad5c80cc0afaf7902dbbb224/ci/scripts/integration_arrow.sh#L28-L40
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] trxcllnt commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "trxcllnt (via GitHub)" <gi...@apache.org>.
trxcllnt commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1691907356
@pitrou yes, it should. I'll enable the integration tests 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] domoritz merged pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "domoritz (via GitHub)" <gi...@apache.org>.
domoritz merged PR #37122:
URL: https://github.com/apache/arrow/pull/37122
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] trxcllnt commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "trxcllnt (via GitHub)" <gi...@apache.org>.
trxcllnt commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1692380285
The integration test is failing because the unions test has duplicate field names. I have a fix for it locally, but it involves renaming the duplicated `sparse` and `dense` fields to `sparse_{1,2}` and `dense_{1,2}`.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1696589395
@eerhardt Could you take a look at this?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1711541815
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit b957847a616a82e89af988c3c922af79379f6160.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/16614819945) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1674264024
:warning: GitHub issue #31621 **has been automatically assigned in GitHub** to PR creator.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] eerhardt commented on pull request #37122: GH-31621: [JS] Fix Union null bitmaps
Posted by "eerhardt (via GitHub)" <gi...@apache.org>.
eerhardt commented on PR #37122:
URL: https://github.com/apache/arrow/pull/37122#issuecomment-1696682346
@trxcllnt - can you run the integration tests locally? The arrow file should be written to the location that `--arrow-file` specifies when the test runs.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org