[GitHub] [arrow] sunchao commented on pull request #6770: ARROW-7842: [Rust] [Parquet] implement array_reader for list type columns

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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)

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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] nealrichardson closed pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd

2020-05-15 Thread GitBox


nealrichardson closed 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] kou commented on pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error

2020-05-15 Thread GitBox


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] kou commented on a change in pull request #7192: ARROW-8815: [Dev][Release] Binary upload script should retry on unexpected bintray request error

2020-05-15 Thread GitBox


kou commented on a change in pull request #7192:
URL: https://github.com/apache/arrow/pull/7192#discussion_r426068491



##
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:
   Oh, sorry.
   We should show request/response details to know what error is occurred.
   
   I've pushed a commit that raises an exception with the request/response 
details.
   We'll be able to know what response should be handled here to retry 
download. Then we can improve our retry code.





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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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|[![TravisCI](https://img.shields.io/travis/ursa-labs/crossbow/actions-266-travis-homebrew-r-autobrew.svg)](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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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|[![Github 
Actions](https://github.com/ursa-labs/crossbow/workflows/Crossbow/badge.svg?branch=actions-265-github-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

2020-05-15 Thread GitBox


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] github-actions[bot] commented on pull request #7196: ARROW-8556: [R] zstd symbol not found if there are multiple installations of zstd

2020-05-15 Thread GitBox


github-actions[bot] commented on pull request #7196:
URL: https://github.com/apache/arrow/pull/7196#issuecomment-629494157


   ```
   No such command 'crossbow-submit'.
   ```



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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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|[![TravisCI](https://img.shields.io/travis/ursa-labs/crossbow/actions-264-travis-homebrew-cpp-autobrew.svg)](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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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|[![TravisCI](https://img.shields.io/travis/ursa-labs/crossbow/actions-263-travis-homebrew-cpp.svg)](https://travis-ci.org/ursa-labs/crossbow/branches)|
   
|homebrew-cpp-autobrew|[![TravisCI](https://img.shields.io/travis/ursa-labs/crossbow/actions-263-travis-homebrew-cpp-autobrew.svg)](https://travis-ci.org/ursa-labs/crossbow/branches)|
   
|homebrew-r-autobrew|[![TravisCI](https://img.shields.io/travis/ursa-labs/crossbow/actions-263-travis-homebrew-r-autobrew.svg)](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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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] nevi-me commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types

2020-05-15 Thread GitBox


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] andygrove commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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] andygrove commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types

2020-05-15 Thread GitBox


andygrove commented on a change in pull request #7193:
URL: https://github.com/apache/arrow/pull/7193#discussion_r425981877



##
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:
   Could you add `f64::NAN` to the test values.





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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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] maxburke commented on pull request #6770: ARROW-7842: [Rust] [Parquet] implement array_reader for list type columns

2020-05-15 Thread GitBox


maxburke commented on pull request #6770:
URL: https://github.com/apache/arrow/pull/6770#issuecomment-629397472


   > Sorry for the late response @maxburke . I'll take a look at this today.
   
   I hate to keep poking but were you able to have a look at this? We're really 
hoping to get back onto the master branch of Arrow



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

2020-05-15 Thread GitBox


nealrichardson commented on pull request #7146:
URL: https://github.com/apache/arrow/pull/7146#issuecomment-629374066


   Sorry, auto-rebase didn't work because there are changes to 
.github/workflows in master. IDK if the "C GLib & Ruby / AMD64 Windows MinGW 64 
GLib & Ruby" failure is real or not, or if rebase would fix it, but that's what 
I was trying. 



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

2020-05-15 Thread GitBox


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.

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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.

2020-05-15 Thread GitBox


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.

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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)

2020-05-15 Thread GitBox


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] pitrou commented on pull request #6811: [DO NOT MERGE] [Python] Reformat using autopep8

2020-05-15 Thread GitBox


pitrou commented on pull request #6811:
URL: https://github.com/apache/arrow/pull/6811#issuecomment-629125249


   I can update this PR and make it ready to merge. I think people agreed that 
having `autopep8` is better than nothing (well, at least the people who wanted 
a Python reformatter :-)).



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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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

2020-05-15 Thread GitBox


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