replace arrowrs with arrow2 #224

Renkai · 2022-10-07T06:19:02Z

No description provided.

# Conflicts: # .gitignore

Renkai · 2022-10-07T06:43:55Z

rust/src/encodings/plain.rs

+        (*self.file).seek(SeekFrom::Start(self.position + offset as u64))?;
+        // let mut mutable_buf = Buffer::new(read_len * T::get_byte_width());
+        let byte_size = std::mem::size_of::<T>();
+        let mut buf = vec![0u8; read_len * byte_size];//TODO is it right?


These lines are obviously different from @eddyxu 's arrow-rs version because there are missing equivalent things in arrow2, need deep review.

T is native rust type? Does it support bool?

T is an Arrow2 trait that extends native rust Type. From this list I think bool is not included, but maybe bool is just UInt8?

eddyxu · 2022-10-07T06:48:23Z

@Renkai let's target this PR to rust branch?

Renkai · 2022-10-07T07:05:51Z

@eddyxu retargeted, but the diff is too large, would you update branch rust with newest main branch?

# Conflicts: # rust/Cargo.toml # rust/src/encodings.rs # rust/src/encodings/plain.rs # rust/src/schema.rs

Renkai · 2022-10-08T02:15:32Z

branch updated, now the diff page is clearer

eddyxu · 2022-10-08T04:04:45Z

rust/src/schema.rs

+        use DataType::*;
+        matches!(
+            t,
+            UInt8


those indents are not right?

Aren't arrow2 has similar check?

Actually it's copied from arrow-rs and keeps the original indents, at least Clion believe it indents properly.

Arrow2 also has a similar one, but it's not public.

Should we not put this as public interface on Schema as well?

Good idea, private it.

eddyxu · 2022-10-08T04:05:38Z

rust/src/schema.rs

@@ -100,6 +100,44 @@ impl Field {
        }
    }

+    /// Return Arrow Data Type name.
+    pub fn type_str(t: &DataType) -> String {


Why do we need this method? Can we use #Debug() or format traits?

It's used for keeping compatible with the names in protobuf. I'm not sure if the results of #Debug() or format are compatible with the content from protobuf.

https://docs.rs/arrow2/latest/arrow2/datatypes/enum.DataType.html#impl-Debug-for-DataType

What are the returns from here?

This implementation lacks support for quite some data types and causes panic in production code.

The result is

println!("UInt8: {:?}",DataType::UInt8); println!("UInt16: {:?}",DataType::UInt16); println!("Binary: {:?}",DataType::Binary); println!("Extension: {:?}", DataType::Extension("the_str".to_string(), Box::new(DataType::Date64), Option::Some("option_string".into()))); ----- UInt8: UInt8 UInt16: UInt16 Binary: Binary Extension: Extension("the_str", Date64, Some("option_string"))

I think what you said about panic is right but maybe we can start with simple cases first?

Can we get a parity to https://github.com/eto-ai/lance/blob/ef3a53cfdedb39569b75e4c2343290452b2b8f30/cpp/src/lance/arrow/type.cc#L222 ?

These are the types we are currently using.

Updated. Referred to this, but the C++ version of Timestamp doesn't have a timezone field, while the rust version has one, for compatibility reasons I ignored the timezone field in rust.

eddyxu · 2022-10-08T04:07:31Z

rust/src/encodings.rs

    type ArrowType;

-    fn decode(&mut self, offset: i32, length: &Option<i32>) -> Result<ArrayRef>;
+    fn decode(&mut self, offset: i32, length: &Option<i32>) -> Result<Arc<dyn Array>> ;


Why some of them return Arc<dyn Array> some of them return Box<dyn Array>

The Arc is a mimic of the original arrow-rs version, the Box is deduced from this arrow2 function. It's very similar to the arrow-rs version except for the signature. I don't know the design intention of Arrow2 here, just keep it unchanged when it works.

Can we make our interface consistent? either all return Box<dyn Array> or all <Arc<dyn Array>>?
Conceptually, decode / take should all return "array" from user API's aspect.

Switched to box

eddyxu · 2022-10-08T04:08:52Z

rust/src/encodings/plain.rs

+        let byte_size = std::mem::size_of::<T>();
+        let mut buf = vec![0u8; read_len * byte_size];//TODO is it right?
+        (*self.file).read_exact(&mut buf)?;
+        let mut builder = MutablePrimitiveArray::with_capacity(read_len);


is there an arrow2 api to directly pass the continuous memory buffer into Array?

This assignment of each element is not efficient.

I don't see the API that can batch convert raw bytes to PrimitiveArray in Arrow2. If we want batch convert, maybe we can use ReadBytesExt? The code would become similar to blow

match T::PRIMITIVE { PrimitiveType::Int8 => { let mut vec = vec![0i8; read_len]; (*self.file).read_i8_into(&mut vec)?; PrimitiveArray::from_slice(vec).arced(); } PrimitiveType::Int16 => { let mut vec = vec![0i16; read_len]; (*self.file).read_i16::<LittleEndian>(&mut vec)?; PrimitiveArray::from_slice(vec).arced(); } /** Boilerplate code ... **/ PrimitiveType::Int32 => {} PrimitiveType::Int64 => {} PrimitiveType::Int128 => {} PrimitiveType::Int256 => {} PrimitiveType::UInt8 => {} PrimitiveType::UInt16 => {} PrimitiveType::UInt32 => {} PrimitiveType::UInt64 => {} PrimitiveType::Float16 => {} PrimitiveType::Float32 => {} PrimitiveType::Float64 => {} PrimitiveType::DaysMs => {} PrimitiveType::MonthDayNano => {} }

This way is not applicable for DaysMs and MonthDayNano.

The reason that we are using arrow is that arrow is a "zero-copy" representation of arrays. Copying / constructing an array should be just setting a few pointers.

https://arrow.apache.org/docs/format/Columnar.html

So as an implementation of Arrow, Arrow2 should definitely have a function that can convert [u8] to PrimitiveArray without copy data? I would take a further look into it.

One more question, Arrow2 said it's transmute-free.
In my understanding, the meaning of transmute-free is:
We don't provide such a way that "Copying / constructing an array should be just setting a few pointers."
Wrong?

i dont think that's the meaning of transmute-free.

@changhiskhan could you help to clarify ?

So arrow is zero-copy representation of arrays. If we copy each elements, we can rather just use a Vec<> , right?

I've found some very similar question in the Arrow2 issues, will copy from it.

eddyxu · 2022-10-16T17:35:52Z

rust/src/schema.rs

@@ -118,6 +166,15 @@ impl Field {
    }
 }

+fn to_str(unit: &TimeUnit) -> &'static str {


Can we implement this a fmt::Display trait over TimeUnit?

eddyxu · 2022-10-19T01:42:44Z

rust/src/bin/lance.rs

+            println!("Schema: {}\n", reader.schema());
+            use std::any::TypeId;
+            let is_little_endian = TypeId::of::<byteorder::NativeEndian>() == TypeId::of::<byteorder::LittleEndian>();
+            println!("is little endian {:?}", is_little_endian)


is => Is

Co-authored-by: Lei Xu <lei@eto.ai>

eddyxu and others added 11 commits September 19, 2022 14:28

Data Inspection CLI (lancedb#119)

5344664

Merge branch 'main' into rust_arrow2

3602c48

# Conflicts: # .gitignore

Merge branch 'main' into rust_arrow2

794b8fa

changing

825893a

changing

414e953

changing

8c9e4ca

decode

40f5d1e

changing

9af4a8c

debugging

19c09cf

make compile passed

a9948cf

cleaning

99e5e7f

Renkai added the WIP work in progress label Oct 7, 2022

Renkai added 2 commits October 7, 2022 14:31

optmize import

8d21f26

remove arrow-rs

588ad1d

Renkai commented Oct 7, 2022

View reviewed changes

Renkai requested a review from eddyxu October 7, 2022 06:44

Renkai removed the WIP work in progress label Oct 7, 2022

Renkai marked this pull request as ready for review October 7, 2022 06:44

Renkai changed the base branch from main to rust October 7, 2022 07:04

eddyxu force-pushed the rust branch from 5344664 to 94b1906 Compare October 7, 2022 16:29

Merge branch 'rust' into rust_arrow2

fca6098

# Conflicts: # rust/Cargo.toml # rust/src/encodings.rs # rust/src/encodings/plain.rs # rust/src/schema.rs

polish after merge

7bc0823

eddyxu reviewed Oct 8, 2022

View reviewed changes

Renkai added 4 commits October 12, 2022 20:13

remove pub of is_number

05276d7

arc to box

fb6b3d4

arc to box

88ba333

bulk made primitive array

3b71ea3

Renkai added 3 commits October 16, 2022 15:12

bulk made primitive array

b27fc99

bulk made primitive array

e3f2b0c

type_str

9cf1016

Renkai requested a review from eddyxu October 16, 2022 07:57

eddyxu approved these changes Oct 19, 2022

View reviewed changes

capitalize

2d0b913

Renkai merged commit 3e457ba into lancedb:rust Oct 19, 2022

eddyxu added a commit that referenced this pull request Oct 19, 2022

replace arrowrs with arrow2 (#224)

2529885

Co-authored-by: Lei Xu <lei@eto.ai>

eddyxu added a commit that referenced this pull request Oct 28, 2022

replace arrowrs with arrow2 (#224)

aeb0305

Co-authored-by: Lei Xu <lei@eto.ai>

eddyxu added a commit that referenced this pull request Oct 28, 2022

replace arrowrs with arrow2 (#224)

207bb1e

Co-authored-by: Lei Xu <lei@eto.ai>

eddyxu added a commit that referenced this pull request Nov 10, 2022

replace arrowrs with arrow2 (#224)

51f35d4

Co-authored-by: Lei Xu <lei@eto.ai>

eddyxu added a commit that referenced this pull request Nov 11, 2022

replace arrowrs with arrow2 (#224)

0e8d2ed

Co-authored-by: Lei Xu <lei@eto.ai>

eddyxu added a commit that referenced this pull request Dec 9, 2022

replace arrowrs with arrow2 (#224)

3f9135b

Co-authored-by: Lei Xu <lei@eto.ai>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

replace arrowrs with arrow2 #224

replace arrowrs with arrow2 #224

Renkai commented Oct 7, 2022

Renkai Oct 7, 2022

eddyxu Oct 8, 2022

Renkai Oct 12, 2022

eddyxu commented Oct 7, 2022

Renkai commented Oct 7, 2022

Renkai commented Oct 8, 2022

eddyxu Oct 8, 2022

Renkai Oct 8, 2022

eddyxu Oct 8, 2022

Renkai Oct 12, 2022

eddyxu Oct 8, 2022

Renkai Oct 8, 2022

eddyxu Oct 8, 2022

Renkai Oct 12, 2022

eddyxu Oct 12, 2022

Renkai Oct 16, 2022

eddyxu Oct 8, 2022

Renkai Oct 8, 2022

eddyxu Oct 8, 2022

Renkai Oct 12, 2022

eddyxu Oct 8, 2022

Renkai Oct 8, 2022

Renkai Oct 8, 2022

eddyxu Oct 8, 2022

Renkai Oct 12, 2022

Renkai Oct 12, 2022

eddyxu Oct 12, 2022

Renkai Oct 16, 2022

eddyxu Oct 16, 2022

eddyxu Oct 19, 2022

replace arrowrs with arrow2 #224

replace arrowrs with arrow2 #224

Conversation

Renkai commented Oct 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyxu commented Oct 7, 2022

Renkai commented Oct 7, 2022

Renkai commented Oct 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment