feat: support convert arrow value and type to AnyValue, Any #141
Conversation
Yes. If this sketch looks well, I complete it more later. |
icelake/src/error.rs
Outdated
| /// Arrow data type is not supported. | ||
| /// | ||
| /// This error is returned when fail to convert from or convert to arrow data or type. | ||
| ArrowUnsupported, |
There was a problem hiding this comment.
How about rename it to DataTypeUnspported to be more clear?
icelake/src/types/mod.rs
Outdated
| @@ -7,6 +7,7 @@ pub use in_memory::*; | |||
| mod on_disk; | |||
| pub use on_disk::*; | |||
|
|
|||
| mod from_arrow; | |||
There was a problem hiding this comment.
How about put all arrow related conversion in one mod called arrow?
|
Have integrate the arrow and add more test. |
icelake/src/error.rs
Outdated
| @@ -48,6 +52,7 @@ impl From<ErrorKind> for &'static str { | |||
| ErrorKind::Unexpected => "Unexpected", | |||
| ErrorKind::IcebergDataInvalid => "IcebergDataInvalid", | |||
| ErrorKind::IcebergFeatureUnsupported => "IcebergFeatureUnsupported", | |||
| ErrorKind::DataTypeUnsupported => "ArrowUnsupported", | |||
There was a problem hiding this comment.
| ErrorKind::DataTypeUnsupported => "ArrowUnsupported", | |
| ErrorKind::DataTypeUnsupported => "DataTypeUnsupported", |
|
|
||
| /// This interface is used to convert the struct array. We pass he target type instead of | ||
| /// converting it internally is to save the extra cost. | ||
| fn to_anyvalue_array_with_type(&self, target_type: Any) -> Result<Vec<Option<AnyValue>>>; |
There was a problem hiding this comment.
Maybe we don't need this trait? We can implement TryFrom<PrimitiveArray> and TryFrom<BoolArray> for Vec<Option<AnyValue>>, and have two methods to convert ArrayRef and StructArray for type safety.
There was a problem hiding this comment.
We can't directly implement TryFrom becase it don't define in our crate, we will get the error like:
only traits defined in the current crate can be implemented for types defined outside of the crate
define and implement a trait or new type instead
There was a problem hiding this comment.
Oh, I see. I still want to remove to_anyvalue_array_with_type to keep type safety. We can implement standalone method for StructArray and ArrayRef
ZENOTME
force-pushed
the
from_arrow_value
branch
from
2064993
to
a716950
Compare
2. add more test for from_arrow
ZENOTME
force-pushed
the
from_arrow_value
branch
from
a716950
to
c0f02b8
Compare
This PR design the sketch to convert arrow value to AnyValue, arrow type to Any.
I'm not sure whether this design and abstraction is clear enogh, I will modify it if there is better way. PTAL. @Xuanwo @liurenjie1024