You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/07/15 00:15:58 UTC

[GitHub] [arrow] rj-atw opened a new pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

rj-atw opened a new pull request #7767:
URL: https://github.com/apache/arrow/pull/7767


   Adding compilation flag to allow rust library to compile against Wasm32 target


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] alamb edited a comment on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
alamb edited a comment on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-744004731


   @rj-atw  -- FYI @jorgecarleitao  has the appropriate CI support here: https://github.com/apache/arrow/pull/8864 which you can probably pull into this 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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r455491405



##########
File path: rust/datafusion/Cargo.toml
##########
@@ -46,18 +46,20 @@ cli = ["rustyline"]
 [dependencies]
 fnv = "1.0"
 arrow = { path = "../arrow", version = "1.0.0-SNAPSHOT" }
-parquet = { path = "../parquet", version = "1.0.0-SNAPSHOT" }
 sqlparser = "0.2.6"
-clap = "2.33"
-prettytable-rs = "0.8.0"
+paste = "0.1"
+
+[target.'cfg(not(target_arch="wasm32"))'.dependencies]
+parquet = { path = "../parquet", version = "1.0.0-SNAPSHOT" }

Review comment:
       Will look more into parquet library. I skipped many as it is primarily used to read from the file system...which I believe does not have stable support yet in WASM.
   
   If I change to use a positive gate should I restrict to just x86 and x86_64; or are there other supported targets?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] alamb commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-744004731


   @rj-atw  -- FYI @jorgecarleitao  has the appropriate CI support here: https://github.com/apache/arrow/pull/8864


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] paddyhoran edited a comment on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
paddyhoran edited a comment on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658935926


   > @paddyhoran I love to add the need CI. Do we have any documentation around this projects CI (especially how to setup env locally)?
   
   If you search for "rust" under the top level "ci" folder you will find most of the CI scripts.  We can then ask @kszucs for a quick review of that part of the 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:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658481405


   https://issues.apache.org/jira/browse/ARROW-9453


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658516845


   > 
   > 
   > @rj-atw I would like to see us get wasm support but I think we need to add CI for this. Without CI all those `cfg` attributes are destined to go stale...
   
   @paddyhoran I love to add the need CI. Do we have any documentation around this projects CI (especially how to setup env locally)?  


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] paddyhoran commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
paddyhoran commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r455256346



##########
File path: rust/Cargo.toml
##########
@@ -24,3 +24,7 @@ members = [
         "integration-testing",
 	"benchmarks",
 ]
+default-members= [

Review comment:
       Is this just so that wasm builds by default at the top level?  If so, I'd rather not do this either.  i.e. it's enough to just say that we can build the sub-crates on wasm instead of changing this global default.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r455492644



##########
File path: rust/datafusion/src/bin/repl.rs
##########
@@ -17,15 +17,29 @@
 
 #![allow(bare_trait_objects)]
 
+#[cfg(not(target_arch="wasm32"))] 
 use arrow::util::pretty;
+#[cfg(not(target_arch="wasm32"))]
 use clap::{crate_version, App, Arg};
+#[cfg(not(target_arch="wasm32"))]
 use datafusion::error::Result;
+#[cfg(not(target_arch="wasm32"))]
 use datafusion::execution::context::ExecutionContext;
+#[cfg(not(target_arch="wasm32"))]
 use rustyline::Editor;
+#[cfg(not(target_arch="wasm32"))]
 use std::env;
+#[cfg(not(target_arch="wasm32"))]
 use std::path::Path;
+#[cfg(not(target_arch="wasm32"))]
 use std::time::Instant;
 
+
+#[cfg(target_arch="wasm32")]
+pub fn main() {
+}

Review comment:
       WASM is a library only. I believe this code is only useful when creating an executable. 
   
   I know this is a bad hack, but I am actually not really sure why this file was still pickup by rustc as I should be excluding in the package dependencies. 
   
   I'll take another look at why this is still being compile. However, I am open to suggestion/discussion on alternatives.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r455490563



##########
File path: rust/Cargo.toml
##########
@@ -24,3 +24,7 @@ members = [
         "integration-testing",
 	"benchmarks",
 ]
+default-members= [

Review comment:
       Yes. This was add as I did not try and port the Flight and Parquet submodules as I believe their core functionality uses file and sockets which don't have stable support in WASM yet. 
   
   I'll try out the individual submodule build, add details to readme and revert the Cargo.toml 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:
users@infra.apache.org



[GitHub] [arrow] alamb commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-759438372


   @rj-atw  --  Given the imminent Arrow 3.0 release, I am trying to clean up older Rust PRs and see if the authors have plans to move them forward. 
   
   Do you plan on working on this PR in the near future? If not, should we close this PR until there is time to make progress? Thanks again for your contributions so far. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] kszucs commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658946812


   @rj-atw you can start here https://github.com/apache/arrow/blob/master/docs/source/developers/docker.rst
   
   I assume you'll need to adjust the [rust_build.sh](https://github.com/apache/arrow/blob/master/ci/scripts/rust_build.sh) script to accept either a target arch environment variable (preferable) or a CLI argument than add a new docker-compose entry (like debian-rust-wasm32) similar to [debian-rust](https://github.com/apache/arrow/blob/master/docker-compose.yml#L928) where you pass the wasm variable.
   
   Once that works with `archery docker run debian-rust-wasm32` you can add a matrix entry to execute debian-rust-wasm32 in the [rust github actions configuration](https://github.com/apache/arrow/blob/master/.github/workflows/rust.yml#L52).
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] nevi-me commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r506960006



##########
File path: rust/arrow/src/util/bit_util.rs
##########
@@ -366,13 +371,13 @@ mod tests {
         assert_eq!(ceil(8, 8), 1);
         assert_eq!(ceil(9, 8), 2);
         assert_eq!(ceil(9, 9), 1);
-        assert_eq!(ceil(10000000000, 10), 1000000000);
-        assert_eq!(ceil(10, 10000000000), 1);
-        assert_eq!(ceil(10000000000, 1000000000), 10);
+        assert_eq!(ceil(10000000, 10), 1000000);

Review comment:
       The numbers are 64-bit, which won't be supported in wasm32




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] nevi-me commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-759496437


   I'm closing this for now @alamb @rj-atw, we can pick it up later.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] emkornfield commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-681347717


   @rj-atw where you able to figure out CI?  It looks like this also needs to be 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:
users@infra.apache.org



[GitHub] [arrow] paddyhoran commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
paddyhoran commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658935926


   > @paddyhoran I love to add the need CI. Do we have any documentation around this projects CI (especially how to setup env locally)?
   
   If you search for "rust" under the top level "ci" folder you will find most of the CI scripts.  We can then ask @kszucs for a quick review of that part of the update.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] nevi-me commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-731527903


   Hi @rj-atw, this PR has fallen behind quite a lot. Are you still interested in or able to work on this feature? Otherwise we could close the PR in the interim.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] vertexclique commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
vertexclique commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r455177051



##########
File path: rust/arrow/src/lib.rs
##########
@@ -31,17 +31,19 @@ pub mod array;
 pub mod bitmap;
 pub mod buffer;
 pub mod compute;
+#[cfg(not(target_arch="wasm32"))]
 pub mod csv;
 pub mod datatypes;
 pub mod error;
-#[cfg(feature = "flight")]
+#[cfg(all(feature = "flight",not(target_arch="wasm32")))]

Review comment:
       Inconsistent spacing between macro features and selections. Please reorganize them consistently.

##########
File path: rust/arrow/src/compute/kernels/arithmetic.rs
##########
@@ -99,8 +99,8 @@ where
 }
 
 /// SIMD vectorized version of `math_op` above.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn simd_math_op<T, F>(
+#[cfg(all(any(target_arch = "x86", target_arch = "x86_64", target_arch="wasm32"), feature = "simd"))]
+pub fn simd_math_op<T, F>(

Review comment:
       These don't need to be public, they are internally selected based on feature gates.

##########
File path: rust/datafusion/src/bin/repl.rs
##########
@@ -17,15 +17,29 @@
 
 #![allow(bare_trait_objects)]
 
+#[cfg(not(target_arch="wasm32"))] 
 use arrow::util::pretty;
+#[cfg(not(target_arch="wasm32"))]
 use clap::{crate_version, App, Arg};
+#[cfg(not(target_arch="wasm32"))]
 use datafusion::error::Result;
+#[cfg(not(target_arch="wasm32"))]
 use datafusion::execution::context::ExecutionContext;
+#[cfg(not(target_arch="wasm32"))]
 use rustyline::Editor;
+#[cfg(not(target_arch="wasm32"))]
 use std::env;
+#[cfg(not(target_arch="wasm32"))]
 use std::path::Path;
+#[cfg(not(target_arch="wasm32"))]
 use std::time::Instant;
 
+
+#[cfg(target_arch="wasm32")]
+pub fn main() {
+}

Review comment:
       What is the point of not compiling the ported code? I don't see the benefit of porting to wasm by doing this.

##########
File path: rust/datafusion/Cargo.toml
##########
@@ -46,18 +46,20 @@ cli = ["rustyline"]
 [dependencies]
 fnv = "1.0"
 arrow = { path = "../arrow", version = "1.0.0-SNAPSHOT" }
-parquet = { path = "../parquet", version = "1.0.0-SNAPSHOT" }
 sqlparser = "0.2.6"
-clap = "2.33"
-prettytable-rs = "0.8.0"
+paste = "0.1"
+
+[target.'cfg(not(target_arch="wasm32"))'.dependencies]
+parquet = { path = "../parquet", version = "1.0.0-SNAPSHOT" }

Review comment:
       Don't not gate parquet if not wasm. The parquet should be also wasm compilable. Moreover, actual dependencies shouldn't be gated under the negated set, which makes it harder to maintain.

##########
File path: rust/arrow/src/lib.rs
##########
@@ -31,17 +31,19 @@ pub mod array;
 pub mod bitmap;
 pub mod buffer;
 pub mod compute;
+#[cfg(not(target_arch="wasm32"))]
 pub mod csv;
 pub mod datatypes;
 pub mod error;
-#[cfg(feature = "flight")]
+#[cfg(all(feature = "flight",not(target_arch="wasm32")))]

Review comment:
       Also please run cargo fmt and cargo clippy




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-740767675


   I am still intersted.
   
   Started new job, so diverted my attention. But hoping to close this out
   during holiday break.
   
   On Mon, Dec 7, 2020, 10:10 PM Jorge Leitao <no...@github.com> wrote:
   
   > @rj-atw <https://github.com/rj-atw> I am adding wasm32 compilation to the
   > CI, so that this can progress. Is this something that you are still
   > interested on, or should we close this PR?
   >
   > —
   > You are receiving this because you were mentioned.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/arrow/pull/7767#issuecomment-740402282>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AOPXAG6UVNNQB37QW64C4KDSTW7NPANCNFSM4O2AJQEA>
   > .
   >
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] nevi-me closed pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
nevi-me closed pull request #7767:
URL: https://github.com/apache/arrow/pull/7767


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] sunchao commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r454841132



##########
File path: rust/Cargo.toml
##########
@@ -24,3 +24,7 @@ members = [
         "integration-testing",
 	"benchmarks",
 ]
+default-members= [

Review comment:
       Why add this? is this related to the 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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r454760959



##########
File path: rust/arrow/src/util/bit_util.rs
##########
@@ -366,13 +371,13 @@ mod tests {
         assert_eq!(ceil(8, 8), 1);
         assert_eq!(ceil(9, 8), 2);
         assert_eq!(ceil(9, 9), 1);
-        assert_eq!(ceil(10000000000, 10), 1000000000);
-        assert_eq!(ceil(10, 10000000000), 1);
-        assert_eq!(ceil(10000000000, 1000000000), 10);
+        assert_eq!(ceil(10000000, 10), 1000000);

Review comment:
       I will double check these changes. I made them in response to a test that was failing; however, I found it suspect at the time 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:
users@infra.apache.org



[GitHub] [arrow] rj-atw commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
rj-atw commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r455489455



##########
File path: rust/arrow/src/compute/kernels/arithmetic.rs
##########
@@ -99,8 +99,8 @@ where
 }
 
 /// SIMD vectorized version of `math_op` above.
-#[cfg(all(any(target_arch = "x86", target_arch = "x86_64"), feature = "simd"))]
-fn simd_math_op<T, F>(
+#[cfg(all(any(target_arch = "x86", target_arch = "x86_64", target_arch="wasm32"), feature = "simd"))]
+pub fn simd_math_op<T, F>(

Review comment:
       Thanks. I made this modification to explicitly test that simd op call was not failing in WASM. Forgot to revert while pushing




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] alippai commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
alippai commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r506963338



##########
File path: rust/arrow/src/util/bit_util.rs
##########
@@ -366,13 +371,13 @@ mod tests {
         assert_eq!(ceil(8, 8), 1);
         assert_eq!(ceil(9, 8), 2);
         assert_eq!(ceil(9, 9), 1);
-        assert_eq!(ceil(10000000000, 10), 1000000000);
-        assert_eq!(ceil(10, 10000000000), 1);
-        assert_eq!(ceil(10000000000, 1000000000), 10);
+        assert_eq!(ceil(10000000, 10), 1000000);

Review comment:
       I think wasm32 supports i64 and f64 https://webassembly.github.io/spec/core/syntax/types.html




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] paddyhoran commented on a change in pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
paddyhoran commented on a change in pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#discussion_r454755766



##########
File path: rust/arrow/src/util/bit_util.rs
##########
@@ -366,13 +371,13 @@ mod tests {
         assert_eq!(ceil(8, 8), 1);
         assert_eq!(ceil(9, 8), 2);
         assert_eq!(ceil(9, 9), 1);
-        assert_eq!(ceil(10000000000, 10), 1000000000);
-        assert_eq!(ceil(10, 10000000000), 1);
-        assert_eq!(ceil(10000000000, 1000000000), 10);
+        assert_eq!(ceil(10000000, 10), 1000000);

Review comment:
       Why the changes 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:
users@infra.apache.org



[GitHub] [arrow] paddyhoran commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
paddyhoran commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658514635


   @rj-atw I would like to see us get wasm support but I think we need to add CI for this.  Without CI all those `cfg` attributes are destined to go stale...


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] jorgecarleitao commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-740402282


   @rj-atw I am adding wasm32 compilation to the CI, so that this can progress. Is this something that you are still interested on, or should we close this 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:
users@infra.apache.org



[GitHub] [arrow] paddyhoran commented on pull request #7767: ARROW-9453: [Rust] Wasm32 compilation support

Posted by GitBox <gi...@apache.org>.
paddyhoran commented on pull request #7767:
URL: https://github.com/apache/arrow/pull/7767#issuecomment-658515334


   Also, I assigned the JIRA to you and changed your permissions so you can self-assign in future


----------------------------------------------------------------
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:
users@infra.apache.org