[GitHub] [arrow] sunchao commented on pull request #6770: ARROW-7842: [Rust] [Parquet] implement array_reader for list type columns
sunchao commented on pull request #6770: URL: https://github.com/apache/arrow/pull/6770#issuecomment-629593692 @maxburke yes was looking at this but the change was bigger than I expected and I was not able to finish it. Still looking. 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] sunchao commented on pull request #6898: ARROW-8399: [Rust] Extend memory alignments to include other architectures
sunchao commented on pull request #6898: URL: https://github.com/apache/arrow/pull/6898#issuecomment-629593291 Build errors: ``` error: unnecessary parentheses around assigned value --> arrow\src\memory.rs:43:30 | 43 | pub const ALIGNMENT: usize = (1 << 7); | help: remove these parentheses | = note: `-D unused-parens` implied by `-D warnings` error: unnecessary parentheses around assigned value --> arrow\src\memory.rs:137:35 | 137 | const FALLBACK_ALIGNMENT: usize = (1 << 6); | help: remove these parentheses ``` 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] sunchao commented on pull request #7061: ARROW-8629: [Rust] Eliminate indirection of zero sized allocations
sunchao commented on pull request #7061:
URL: https://github.com/apache/arrow/pull/7061#issuecomment-629593043
@vertexclique hmm I'm seeing multiple tests failing:
```
ipc::reader::tests::read_generated_streams stdout
thread 'ipc::reader::tests::read_generated_streams' panicked at 'memory not
aligned', arrow/src/buffer.rs:165:9
ipc::reader::tests::read_generated_files stdout
thread 'ipc::reader::tests::read_generated_files' panicked at 'memory not
aligned', arrow/src/buffer.rs:165:9
ipc::writer::tests::read_and_rewrite_generated_streams stdout
thread 'ipc::writer::tests::read_and_rewrite_generated_streams' panicked at
'memory not aligned', arrow/src/buffer.rs:165:9
ipc::writer::tests::read_and_rewrite_generated_files stdout
thread 'ipc::writer::tests::read_and_rewrite_generated_files' panicked at
'memory not aligned', arrow/src/buffer.rs:165:9
```
these seem related. Could you take a look? Thanks.
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] github-actions[bot] commented on pull request #7199: ARROW-8820: fix date_trunc functions to return date types
github-actions[bot] commented on pull request #7199: URL: https://github.com/apache/arrow/pull/7199#issuecomment-629592927 https://issues.apache.org/jira/browse/ARROW-8820 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] pprudhvi opened a new pull request #7199: ARROW-8820: fix date_trunc functions to return date types
pprudhvi opened a new pull request #7199: URL: https://github.com/apache/arrow/pull/7199 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] emkornfield commented on pull request #7181: ARROW-8799: [C++][Parquet] NestedListReader needs to handle empty item batches
emkornfield commented on pull request #7181: URL: https://github.com/apache/arrow/pull/7181#issuecomment-629590380 @bkietz could you add a unit test? 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] emkornfield closed pull request #6622: ARROW-8121: [Java] Enhance code style checking for Java code (add spaces after commas, semi-colons and type casts)
emkornfield closed pull request #6622: URL: https://github.com/apache/arrow/pull/6622 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] eerhardt commented on pull request #7158: ARROW-8788: [C#] Introduce bit-packed builder for null support in builders
eerhardt commented on pull request #7158: URL: https://github.com/apache/arrow/pull/7158#issuecomment-629577781 Thanks for getting this out @mr-smidge. Sorry I didn’t get time to review it this week, but it is on my list for early next week to take a look. 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] paddyhoran commented on pull request #7198: ARROW-8818: [Rust] Failing to build on master due to Flatbuffers/Union issues
paddyhoran commented on pull request #7198: URL: https://github.com/apache/arrow/pull/7198#issuecomment-629559536 I don't quite know what happened to by local setup. I was getting an error due to flatbuffers and once I bumped flatbuffers to 0.6.1 is resolved. I can't re-create it now so I reverted the flatbuffer 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] github-actions[bot] commented on pull request #7198: ARROW-8818: [Rust] Failing to build on master due to Flatbuffers/Union issues
github-actions[bot] commented on pull request #7198: URL: https://github.com/apache/arrow/pull/7198#issuecomment-629557971 https://issues.apache.org/jira/browse/ARROW-8818 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] paddyhoran commented on issue #7194: Rust docs don't compile for arrow 0.17.0
paddyhoran commented on issue #7194: URL: https://github.com/apache/arrow/issues/7194#issuecomment-629557139 Hi @ritchie46, thanks for the issue report. We use JIRA for issue tracking so I opened an [issue](https://issues.apache.org/jira/browse/ARROW-8819) there. 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] paddyhoran closed issue #7194: Rust docs don't compile for arrow 0.17.0
paddyhoran closed issue #7194: URL: https://github.com/apache/arrow/issues/7194 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] paddyhoran opened a new pull request #7198: ARROW-8818: [Rust] Failing to build on master due to Flatbuffers/Union issues
paddyhoran opened a new pull request #7198: URL: https://github.com/apache/arrow/pull/7198 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] kou commented on pull request #7146: ARROW-8757: [C++][Plasma] Write Plasma header in little-endian format
kou commented on pull request #7146: URL: https://github.com/apache/arrow/pull/7146#issuecomment-629530281 Rerunning fixed the test failure. I've merged this. Sorry. I forgot to merge 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] kou closed pull request #7146: ARROW-8757: [C++][Plasma] Write Plasma header in little-endian format
kou closed pull request #7146: URL: https://github.com/apache/arrow/pull/7146 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] nealrichardson closed pull request #7197: ARROW-7967: [CI][Crossbow] Pin macOS version in autobrew job to match CRAN
nealrichardson closed pull request #7197: URL: https://github.com/apache/arrow/pull/7197 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] kou commented on pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error
kou commented on pull request #7192: URL: https://github.com/apache/arrow/pull/7192#issuecomment-629525297 Could you try the following command with this change to collect the HTTP error response on download? ```shell BINTRAY_REPOSITORY=kszucs/arrow dev/release/03-binary.sh 0.17.1 1 packages/... ``` If we can know the unhandled HTTP error response, we can improve retry logic in this pull request. Or should we work on improving retry logic as a follow-up task? 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] github-actions[bot] commented on pull request #7197: ARROW-7967: [CI][Crossbow] Pin macOS version in autobrew job to match CRAN
github-actions[bot] commented on pull request #7197: URL: https://github.com/apache/arrow/pull/7197#issuecomment-629518324 https://issues.apache.org/jira/browse/ARROW-7967 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] kou closed pull request #7191: ARROW-8814: [Dev][Release] Binary upload script keeps raising locale warnings
kou closed pull request #7191: URL: https://github.com/apache/arrow/pull/7191 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] github-actions[bot] commented on pull request #7197: ARROW-7967: [CI][Crossbow] Pin macOS version in autobrew job to match CRAN
github-actions[bot] commented on pull request #7197: URL: https://github.com/apache/arrow/pull/7197#issuecomment-629515712 Revision: 7f50e24a7ecb593ceb0e0f5fec4c13e1f05e36db Submitted crossbow builds: [ursa-labs/crossbow @ actions-266](https://github.com/ursa-labs/crossbow/branches/all?query=actions-266) |Task|Status| ||--| |homebrew-r-autobrew|[](https://travis-ci.org/ursa-labs/crossbow/branches)| 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] nealrichardson commented on pull request #7197: ARROW-7967: [CI][Crossbow] Pin macOS version in autobrew job to match CRAN
nealrichardson commented on pull request #7197: URL: https://github.com/apache/arrow/pull/7197#issuecomment-629514818 @github-actions crossbow submit homebrew-r-autobrew 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] nealrichardson commented on a change in pull request #7197: ARROW-7967: [CI][Crossbow] Pin macOS version in autobrew job to match CRAN
nealrichardson commented on a change in pull request #7197: URL: https://github.com/apache/arrow/pull/7197#discussion_r426063851 ## File path: dev/tasks/homebrew-formulae/travis.osx.r.yml ## @@ -15,10 +15,8 @@ # limitations under the License. os: osx -# CRAN builds on macOS 10.11, so make sure that's what we're testing -# See https://issues.apache.org/jira/browse/ARROW-7923; this is currently disabled -# osx_image: xcode8 -language: r +# CRAN builds on macOS 10.13, so make sure that's what we're testing +osx_image: xcode9.4language: r Review comment: ```suggestion osx_image: xcode9.4 language: r ``` 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] nealrichardson opened a new pull request #7197: ARROW-7967: [CI][Crossbow] Pin macOS version in autobrew job to match CRAN
nealrichardson opened a new pull request #7197: URL: https://github.com/apache/arrow/pull/7197 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] nealrichardson closed pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
nealrichardson closed pull request #7195: URL: https://github.com/apache/arrow/pull/7195 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] github-actions[bot] commented on pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd
github-actions[bot] commented on pull request #7196: URL: https://github.com/apache/arrow/pull/7196#issuecomment-629501150 Revision: d59fdcaa78ba3ec38c314027648cacd09eab Submitted crossbow builds: [ursa-labs/crossbow @ actions-265](https://github.com/ursa-labs/crossbow/branches/all?query=actions-265) |Task|Status| ||--| |test-r-linux-as-cran|[](https://github.com/ursa-labs/crossbow/actions?query=branch:actions-265-github-test-r-linux-as-cran)| 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] nealrichardson commented on pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd
nealrichardson commented on pull request #7196: URL: https://github.com/apache/arrow/pull/7196#issuecomment-629500237 @github-actions crossbow submit *as-cran* 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] nealrichardson commented on pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd
nealrichardson commented on pull request #7196: URL: https://github.com/apache/arrow/pull/7196#issuecomment-629493581 @github-actions crossbow-submit *as-cran* 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] github-actions[bot] commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
github-actions[bot] commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629491844 Revision: a4bcd1081fffca271389bf611ad7f5737e671f8f Submitted crossbow builds: [ursa-labs/crossbow @ actions-264](https://github.com/ursa-labs/crossbow/branches/all?query=actions-264) |Task|Status| ||--| |homebrew-cpp-autobrew|[](https://travis-ci.org/ursa-labs/crossbow/branches)| 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] github-actions[bot] commented on pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd
github-actions[bot] commented on pull request #7196: URL: https://github.com/apache/arrow/pull/7196#issuecomment-629490554 https://issues.apache.org/jira/browse/ARROW-8556 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] nealrichardson commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
nealrichardson commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629490945 @github-actions crossbow submit homebrew-cpp-autobrew 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] nealrichardson opened a new pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd
nealrichardson opened a new pull request #7196: URL: https://github.com/apache/arrow/pull/7196 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] github-actions[bot] commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
github-actions[bot] commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629478994 Revision: dad17e9312a234a27b9560569cd96f7fa44f1ccb Submitted crossbow builds: [ursa-labs/crossbow @ actions-263](https://github.com/ursa-labs/crossbow/branches/all?query=actions-263) |Task|Status| ||--| |homebrew-cpp|[](https://travis-ci.org/ursa-labs/crossbow/branches)| |homebrew-cpp-autobrew|[](https://travis-ci.org/ursa-labs/crossbow/branches)| |homebrew-r-autobrew|[](https://travis-ci.org/ursa-labs/crossbow/branches)| 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] nealrichardson commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
nealrichardson commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629478277 @github-actions crossbow submit homebrew* 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] nealrichardson commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
nealrichardson commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629473653 @github-actions crossbow submit homebrew* 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] andygrove commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types
andygrove commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r426027665
##
File path: rust/arrow/src/compute/kernels/sort.rs
##
@@ -283,6 +285,30 @@ mod tests {
None,
vec![3, 1, 4, 2, 0, 5],
);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
+],
+None,
+vec![3, 1, 4, 2, 0, 5],
+);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
Review comment:
The problem with this approach is that the NaN could be a, or b, or both
so this comparison is now non-deterministic and inconsistent. I think
implementing this specifically for Float32Array and Float64Array and checking
for NaN on both values is the only way we can handle this correctly.
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] nealrichardson closed pull request #6879: ARROW-8377: [CI][C++][R] Build and run C++ tests on Rtools build
nealrichardson closed pull request #6879: URL: https://github.com/apache/arrow/pull/6879 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] nealrichardson commented on pull request #6879: ARROW-8377: [CI][C++][R] Build and run C++ tests on Rtools build
nealrichardson commented on pull request #6879: URL: https://github.com/apache/arrow/pull/6879#issuecomment-629460156 I'm closing this; don't think it's worth our time right now to pursue further. We can revisit later if we want. 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] nealrichardson closed pull request #7080: ARROW-8662: [CI] Consolidate appveyor scripts
nealrichardson closed pull request #7080: URL: https://github.com/apache/arrow/pull/7080 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] nealrichardson commented on pull request #7080: ARROW-8662: [CI] Consolidate appveyor scripts
nealrichardson commented on pull request #7080: URL: https://github.com/apache/arrow/pull/7080#issuecomment-629458850 Interpreting that as an approval. Appveyor passed so I'm merging. 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] github-actions[bot] commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
github-actions[bot] commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629455217 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] nealrichardson commented on pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
nealrichardson commented on pull request #7195: URL: https://github.com/apache/arrow/pull/7195#issuecomment-629454630 @github-actions crossbow submit homebrew* 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] nealrichardson opened a new pull request #7195: ARROW-7803: [R][CI] Autobrew/homebrew tests should not always install from master
nealrichardson opened a new pull request #7195: URL: https://github.com/apache/arrow/pull/7195 The crossbow jobs now append `, :revision => "the_current_sha"` to the git URL in the formula, which makes brew install checkout that commit to build from. Relevant homebrew docs: https://docs.brew.sh/Formula-Cookbook#unstable-versions-devel-head 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] nevi-me commented on pull request #7186: ARROW-8808: [Rust] Fix divide by zero error in builder
nevi-me commented on pull request #7186: URL: https://github.com/apache/arrow/pull/7186#issuecomment-629442658 > Thanks, I hadn't seen that. It might be faster to get this one merged since it is small. It looks like there are no reviews yet on #7159. I'm fine either way, but would like to unblock integration testing as much as possible. @andygrove may you please rebase so we can merge 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] nevi-me commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types
nevi-me commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r426001210
##
File path: rust/arrow/src/compute/kernels/sort.rs
##
@@ -283,6 +285,30 @@ mod tests {
None,
vec![3, 1, 4, 2, 0, 5],
);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
+],
+None,
+vec![3, 1, 4, 2, 0, 5],
+);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
Review comment:
PTAL at the solution that I've just pushed. Given that the `unwrap()`
will only fail if there's a `NAN`, I've replaced it by a default
`std::cmp::Ordering::Greater` to treat `NAN` as the highest value. In the
descending sort path, it ends up being inverted to the lowest value.
I suppose a better approach might be to let the `nulls_first` sort option
drive the behaviour, with the below sort options:
- ascending, nulls last: `Ordering::Greater` placing NaNs before the first
null
- ascending, nulls first: `Ordering::Less` placing NaNs after the last null
- descending, nulls last: `Ordering::Greater`
- descending, nulls first: `Ordering::Less`
... so the ordering being determined by the null behaviour (it helped to
write it out :smile:)
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] ritchie46 opened a new issue #7194: Rust docs don't compile for arrow 0.17.0
ritchie46 opened a new issue #7194: URL: https://github.com/apache/arrow/issues/7194 The arrow docs for Rust don't compile for version 0.17.0. ``` [INFO] [stderr] Error: "Failed to locate format/Flight.proto in any parent directory" ``` See the build logs in https://docs.rs/crate/arrow/0.17.0/builds/242085 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] pitrou commented on a change in pull request #7172: ARROW-8763: [C++] Add RandomAccessFile::WillNeed
pitrou commented on a change in pull request #7172:
URL: https://github.com/apache/arrow/pull/7172#discussion_r425997507
##
File path: cpp/src/arrow/util/io_util.cc
##
@@ -1072,6 +1072,61 @@ Status MemoryMapRemap(void* addr, size_t old_size,
size_t new_size, int fildes,
#endif
}
+Status MemoryAdviseWillNeed(const std::vector& regions) {
+ const auto page_size = static_cast(GetPageSize());
+ DCHECK_GT(page_size, 0);
+ const size_t page_mask = ~(page_size - 1);
+ DCHECK_EQ(page_mask & page_size, page_size);
+
+ auto align_region = [=](const MemoryRegion& region) -> MemoryRegion {
+const auto addr = reinterpret_cast(region.addr);
+const auto aligned_addr = addr & page_mask;
+DCHECK_LT(addr - aligned_addr, page_size);
+return {reinterpret_cast(aligned_addr),
+region.size + static_cast(addr - aligned_addr)};
+ };
+
+#ifdef _WIN32
+ // PrefetchVirtualMemory() is available on Windows 8 or later
+ struct PrefetchEntry { // Like WIN32_MEMORY_RANGE_ENTRY
+void* VirtualAddress;
+size_t NumberOfBytes;
+
+PrefetchEntry(const MemoryRegion& region) // NOLINT runtime/explicit
+: VirtualAddress(region.addr), NumberOfBytes(region.size) {}
+ };
+ using PrefetchVirtualMemoryFunc = BOOL (*)(HANDLE, ULONG_PTR,
PrefetchEntry*, ULONG);
+ static const auto prefetch_virtual_memory =
reinterpret_cast(
+ GetProcAddress(GetModuleHandleW(L"kernel32.dll"),
"PrefetchVirtualMemory"));
Review comment:
Right. In a couple of years we can probably drop Windows 7
compatibility...
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] pitrou commented on a change in pull request #7172: ARROW-8763: [C++] Add RandomAccessFile::WillNeed
pitrou commented on a change in pull request #7172: URL: https://github.com/apache/arrow/pull/7172#discussion_r425997334 ## File path: cpp/src/arrow/io/file.cc ## @@ -636,6 +665,9 @@ Result> MemoryMappedFile::ReadAt(int64_t position, ARROW_ASSIGN_OR_RAISE( nbytes, internal::ValidateReadRange(position, nbytes, memory_map_->size())); + // Arrange to page data in + RETURN_NOT_OK(::arrow::internal::MemoryAdviseWillNeed( Review comment: I think it doesn't make sense to do so separately. If you're using `ReadRangeCache` you're requesting those areas to be available. 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] fsaintjacques commented on a change in pull request #7172: ARROW-8763: [C++] Add RandomAccessFile::WillNeed
fsaintjacques commented on a change in pull request #7172: URL: https://github.com/apache/arrow/pull/7172#discussion_r425995665 ## File path: cpp/src/arrow/io/file.cc ## @@ -636,6 +665,9 @@ Result> MemoryMappedFile::ReadAt(int64_t position, ARROW_ASSIGN_OR_RAISE( nbytes, internal::ValidateReadRange(position, nbytes, memory_map_->size())); + // Arrange to page data in + RETURN_NOT_OK(::arrow::internal::MemoryAdviseWillNeed( Review comment: We have to change Parquet's reader PreBuffer to do so, and CSV/JSON reader. 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] andygrove commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types
andygrove commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r425993804
##
File path: rust/arrow/src/compute/kernels/sort.rs
##
@@ -283,6 +285,30 @@ mod tests {
None,
vec![3, 1, 4, 2, 0, 5],
);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
+],
+None,
+vec![3, 1, 4, 2, 0, 5],
+);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
Review comment:
I can look at this in more detail over the weekend but I think having
special impl for f32/f64 is probably the way to go, then you can add specific
checks for `f32::NAN` and `f64::NAN` before calling `partial_cmp` and we'll
need to decide if `NaN` comes before or after valid numbers.
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] nevi-me commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types
nevi-me commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r425988415
##
File path: rust/arrow/src/compute/kernels/sort.rs
##
@@ -283,6 +285,30 @@ mod tests {
None,
vec![3, 1, 4, 2, 0, 5],
);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
+],
+None,
+vec![3, 1, 4, 2, 0, 5],
+);
+test_sort_to_indices_primitive_arrays::(
+vec![
+None,
+Some(-0.05),
+Some(2.225),
+Some(-1.01),
+Some(-0.05),
+None,
Review comment:
I see that we do get the failure if we have a `f{32|64}::NAN`. Handling
`NAN` might complicate the integer case, so should I separate the float
implementation?
How do we treat NaN in general across the Rust implementation?
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] emkornfield commented on pull request #6425: ARROW-6111: [Java] Support LargeVarChar and LargeBinary types
emkornfield commented on pull request #6425: URL: https://github.com/apache/arrow/pull/6425#issuecomment-629421269 @BryanCutler I believe the integration tests couple LargeList with LargeVarChar/LargeBinary, so both need to be implemented to enable the integration tests. 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] BryanCutler commented on pull request #6425: ARROW-6111: [Java] Support LargeVarChar and LargeBinary types
BryanCutler commented on pull request #6425: URL: https://github.com/apache/arrow/pull/6425#issuecomment-629419409 I think you should be removing the skip Java here https://github.com/apache/arrow/blob/2f72713446b04f8979b04f907e7185985028b0a8/dev/archery/archery/integration/datagen.py#L1480 to enable integration testing for this. I'll try to take another review pass early next week. 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] nevi-me commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types
nevi-me commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r425981364
##
File path: rust/arrow/src/compute/kernels/sort.rs
##
@@ -149,9 +151,9 @@ where
.collect::>();
let mut nulls = null_indices;
if !options.descending {
-valids.sort_by_key(|a| a.1);
+valids.sort_by(|a, b| a.1.partial_cmp().unwrap());
Review comment:
Hmm, do we ever store NaN, or do we represent it as a null? I've never
tried creating an array with NaN
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] andygrove commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types
andygrove commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r425980474
##
File path: rust/arrow/src/compute/kernels/sort.rs
##
@@ -149,9 +151,9 @@ where
.collect::>();
let mut nulls = null_indices;
if !options.descending {
-valids.sort_by_key(|a| a.1);
+valids.sort_by(|a, b| a.1.partial_cmp().unwrap());
Review comment:
Won't this panic if the vector contains any `NaN` values? partial_cmp
would return `None` in that case.
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] maxburke commented on pull request #6770: ARROW-7842: [Rust] [Parquet] implement array_reader for list type columns
maxburke commented on pull request #6770: URL: https://github.com/apache/arrow/pull/6770#issuecomment-629401330 (FWIW it seems that this build is broken because of the merge of ARROW-3827 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] nealrichardson commented on pull request #7146: ARROW-8757: [C++][Plasma] Write Plasma header in little-endian format
nealrichardson commented on pull request #7146: URL: https://github.com/apache/arrow/pull/7146#issuecomment-629370089 @github-actions rebase 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] nealrichardson closed pull request #7159: ARROW-8777: [Rust] Parquet.rs does not support reading fixed-size binary fields.
nealrichardson closed pull request #7159: URL: https://github.com/apache/arrow/pull/7159 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] github-actions[bot] commented on pull request #7193: ARROW-7924: [Rust] Add sort for float types
github-actions[bot] commented on pull request #7193: URL: https://github.com/apache/arrow/pull/7193#issuecomment-629364386 https://issues.apache.org/jira/browse/ARROW-7924 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] nevi-me opened a new pull request #7193: ARROW-7924: [Rust] Add sort for float types
nevi-me opened a new pull request #7193: URL: https://github.com/apache/arrow/pull/7193 This relaxes the trait bound of `std::cmp::Ord` to `std::cmp::PartialOrd` to enable sorting by floats 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] nevi-me closed pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated
nevi-me closed pull request #7004: URL: https://github.com/apache/arrow/pull/7004 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] nealrichardson closed pull request #7184: ARROW-8734: [R] improve nightly build installation
nealrichardson closed pull request #7184: URL: https://github.com/apache/arrow/pull/7184 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] andygrove commented on pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated
andygrove commented on pull request #7004: URL: https://github.com/apache/arrow/pull/7004#issuecomment-629283277 @nevi-me Sorry, I forgot about this one. Please go ahead. 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] nevi-me commented on pull request #7004: ARROW-3827: [Rust] Implement UnionArray Updated
nevi-me commented on pull request #7004: URL: https://github.com/apache/arrow/pull/7004#issuecomment-629277167 > @nevi-me @andygrove this is ready for re-review. @andygrove do you want to have a look, or can we merge 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] maxburke commented on a change in pull request #7159: ARROW-8777: [Rust] Parquet.rs does not support reading fixed-size binary fields.
maxburke commented on a change in pull request #7159:
URL: https://github.com/apache/arrow/pull/7159#discussion_r425853878
##
File path: rust/parquet/src/arrow/array_reader.rs
##
@@ -1042,16 +1079,22 @@ mod tests {
",
$physical_type, $logical_type_str
);
+eprintln!("1");
Review comment:
Yup, my mistake. Removed.
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] nevi-me commented on a change in pull request #7159: ARROW-8777: [Rust] Parquet.rs does not support reading fixed-size binary fields.
nevi-me commented on a change in pull request #7159:
URL: https://github.com/apache/arrow/pull/7159#discussion_r425845566
##
File path: rust/parquet/src/arrow/array_reader.rs
##
@@ -1042,16 +1079,22 @@ mod tests {
",
$physical_type, $logical_type_str
);
+eprintln!("1");
Review comment:
I think you committed these by mistake?
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] andygrove closed pull request #7164: ARROW-8783: [Rust] [DataFusion] Add ParquetScan and CsvScan variants in LogicalPlan
andygrove closed pull request #7164: URL: https://github.com/apache/arrow/pull/7164 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] andygrove commented on pull request #7186: ARROW-8808: [Rust] Fix divide by zero error in builder
andygrove commented on pull request #7186: URL: https://github.com/apache/arrow/pull/7186#issuecomment-629269342 Thanks, I hadn't seen that. It might be faster to get this one merged since it is small. It looks like there are no reviews yet on #7159. I'm fine either way, but would like to unblock integration testing as much as possible. 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] andygrove closed pull request #7187: ARROW-8809: [Rust] Fix JSON schema bug
andygrove closed pull request #7187: URL: https://github.com/apache/arrow/pull/7187 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] vertexclique commented on pull request #6898: ARROW-8399: [Rust] Extend memory alignments to include other architectures
vertexclique commented on pull request #6898: URL: https://github.com/apache/arrow/pull/6898#issuecomment-629234482 Hi, @paddyhoran I've written extensive documentation and all corresponding values and their references inside this PR. Also rebased! 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] wesm commented on pull request #7178: ARROW-8568: [C++] Fix decimal to decimal cast issues
wesm commented on pull request #7178: URL: https://github.com/apache/arrow/pull/7178#issuecomment-629210514 No problem. I've been burning the midnight oil this week so it shouldn't delay too much longer 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] vertexclique commented on pull request #7061: ARROW-8629: [Rust] Eliminate indirection of zero sized allocations
vertexclique commented on pull request #7061: URL: https://github.com/apache/arrow/pull/7061#issuecomment-629201947 @sunchao rebased. 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] pitrou commented on pull request #7178: ARROW-8568: [C++] Fix decimal to decimal cast issues
pitrou commented on pull request #7178: URL: https://github.com/apache/arrow/pull/7178#issuecomment-629197677 Woops, sorry, I had forgotten about the refactor. But, yeah, the changes are quite localized 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error
github-actions[bot] commented on pull request #7192: URL: https://github.com/apache/arrow/pull/7192#issuecomment-629184621 https://issues.apache.org/jira/browse/ARROW-8815 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] github-actions[bot] commented on pull request #7191: ARROW-8814: [Dev][Release] Binary upload script keeps raising locale warnings
github-actions[bot] commented on pull request #7191: URL: https://github.com/apache/arrow/pull/7191#issuecomment-629184622 https://issues.apache.org/jira/browse/ARROW-8814 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] kszucs commented on a change in pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error
kszucs commented on a change in pull request #7192: URL: https://github.com/apache/arrow/pull/7192#discussion_r42574 ## File path: dev/release/binary-task.rb ## @@ -610,7 +610,7 @@ def download_url(url, output_path) $stderr.puts(build_download_error_message(url, response)) return else - raise message Review comment: @kou The original error was caused by undefined var `message`, but once I re-raised the exception the script was also exiting because certain kind of http error was not being handled so I worked around the problem by not re-raising just unconditionally retrying. So I think we need to handle other exceptions during the retry, but can't recall the type of the raised exception. 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] kszucs commented on a change in pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error
kszucs commented on a change in pull request #7192: URL: https://github.com/apache/arrow/pull/7192#discussion_r42574 ## File path: dev/release/binary-task.rb ## @@ -610,7 +610,7 @@ def download_url(url, output_path) $stderr.puts(build_download_error_message(url, response)) return else - raise message Review comment: @kou The original error was caused by undefined var `message`, but once I re-raised the exception the script was also exiting because certain kind of http error was not being handled so I worked around the problem by not re-raising just unconditionally retrying. 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] kszucs opened a new pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error
kszucs opened a new pull request #7192: URL: https://github.com/apache/arrow/pull/7192 During uploading the binaries to bintray the script exited multiple times because of unhandled HTTP errors. 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] kszucs opened a new pull request #7191: ARROW-8814: [Dev][Release] Binary upload script keeps raising locale warnings
kszucs opened a new pull request #7191: URL: https://github.com/apache/arrow/pull/7191 Makes the output readable. 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] rongma1997 commented on pull request #7188: ARROW-8803: [Java] Row count should be set before loading buffers in VectorLoader
rongma1997 commented on pull request #7188: URL: https://github.com/apache/arrow/pull/7188#issuecomment-629175106 Close this based on the discussions in jira. 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] rongma1997 closed pull request #7188: ARROW-8803: [Java] Row count should be set before loading buffers in VectorLoader
rongma1997 closed pull request #7188: URL: https://github.com/apache/arrow/pull/7188 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] nevi-me commented on pull request #7186: ARROW-8808: [Rust] Fix divide by zero error in builder
nevi-me commented on pull request #7186: URL: https://github.com/apache/arrow/pull/7186#issuecomment-629139299 @maxburke had already picked this up in #7159 (https://github.com/apache/arrow/pull/7159/files#diff-f2e30b8a413036fbb623c52d0ffd519dR723). Perhaps we could port the unit test there, and fix this as part of that PR? 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 #7100: ARROW-8696: [Java] Convert tests to maven failsafe
rymurr commented on pull request #7100: URL: https://github.com/apache/arrow/pull/7100#issuecomment-629138257 > @rymurr could you add a section to the Readme with the maven command to run integration tests? Otherwise I think this should be mergeable. @kszucs do the machines the nightlies run on have high enough memory that we could turn integration tests on as nightly buids? thanks @emkornfield I have updated `README.md`. A separate integration test job would be great. I would be interested in helping set that up as it would give me broader exposure to the arrow ecosystem. 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] liyafan82 commented on pull request #6622: ARROW-8121: [Java] Enhance code style checking for Java code (add spaces after commas, semi-colons and type casts)
liyafan82 commented on pull request #6622: URL: https://github.com/apache/arrow/pull/6622#issuecomment-629137341 > @liyafan82 sorry for the delay would you mind rebasing once more? I'll merge later tonight if I'm online or first thing tomorrow, so you won't need to do it again. Rebased. Thanks for your effort. 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] scampi commented on a change in pull request #6402: ARROW-7831: [Java] do not allocate a new offset buffer if the slice starts at 0 since the relative offset pointer would be unchange
scampi commented on a change in pull request #6402:
URL: https://github.com/apache/arrow/pull/6402#discussion_r425610823
##
File path:
java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
##
@@ -740,10 +740,16 @@ private void splitAndTransferOffsetBuffer(int startIndex,
int length, BaseVariab
final int start = offsetBuffer.getInt(startIndex * OFFSET_WIDTH);
final int end = offsetBuffer.getInt((startIndex + length) * OFFSET_WIDTH);
final int dataLength = end - start;
-target.allocateOffsetBuffer((length + 1) * OFFSET_WIDTH);
-for (int i = 0; i < length + 1; i++) {
- final int relativeSourceOffset = offsetBuffer.getInt((startIndex + i) *
OFFSET_WIDTH) - start;
- target.offsetBuffer.setInt(i * OFFSET_WIDTH, relativeSourceOffset);
+
+if (startIndex == 0) {
+ target.offsetBuffer = offsetBuffer.slice(0, (1 + length) * OFFSET_WIDTH);
+ target.offsetBuffer.getReferenceManager().retain();
Review comment:
@emkornfield I based the logic of that branch (relying on `retain()`) on
the same case below for splitting the vailidty buffer:
https://github.com/apache/arrow/blob/29545bcdef35f4151379e72e69ef7ce619f1a517/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java#L772
However, for the value buffer, `transferOwnership` is used, which is called
from `transferBuffer`:
https://github.com/apache/arrow/blob/29545bcdef35f4151379e72e69ef7ce619f1a517/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java#L752
I am not sure about the difference, but from the documentation I understand
that the ownership is not transferred to the new allocator when using `retain`.
Does that mean the validity buffer case needs to be updated and use
`transferOwnership` as well ?
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] projjal commented on pull request #5947: ARROW-7300: [C++][Gandiva] Implement functions to cast from strings to integers/floats
projjal commented on pull request #5947: URL: https://github.com/apache/arrow/pull/5947#issuecomment-629051199 ping @praveenbingo 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] emkornfield commented on pull request #7188: ARROW-8803: [Java] Row count should be set before loading buffers in VectorLoader
emkornfield commented on pull request #7188: URL: https://github.com/apache/arrow/pull/7188#issuecomment-629047852 master should be fixed 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] emkornfield closed pull request #7190: ARROW-8811: [Java] Fix CI
emkornfield closed pull request #7190: URL: https://github.com/apache/arrow/pull/7190 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
