[WIP] updates for delta-rs #189
Conversation
| @@ -206,11 +206,19 @@ impl TryFrom<&ArrowDataType> for DataType { | |||
| ArrowDataType::Binary => Ok(DataType::Primitive(PrimitiveType::Binary)), | |||
| ArrowDataType::FixedSizeBinary(_) => Ok(DataType::Primitive(PrimitiveType::Binary)), | |||
| ArrowDataType::LargeBinary => Ok(DataType::Primitive(PrimitiveType::Binary)), | |||
| ArrowDataType::Decimal128(p, s) => Ok(DataType::decimal(*p, *s)), | |||
| ArrowDataType::Decimal128(p, s) => { | |||
| if p > &38 || s > &38 { | |||
There was a problem hiding this comment.
As far as I know Arrow Decimal128 can't hold larger than 38 precision/scale, so this check seems unnecessary
There was a problem hiding this comment.
the input can hold i8, u8 values, which of course would be wrong, but users could construct it. since we a re fallible here, I though why not check ...
in other cases where we are currently not on a fallible code path I omitted the checks thus far, where there may be more need for it. for instance the DataType::decimal function below does not check internally so far, but I'll check how much that bubbles up ...
There was a problem hiding this comment.
Interesting, I remember in pyarrow this is checked, I guess wasnt safe to assume the same behavior translates to arrow-rs ^^
| @@ -62,7 +64,7 @@ impl Scalar { | |||
| PrimitiveType::String => Arc::new(StringArray::new_null(num_rows)), | |||
| PrimitiveType::Boolean => Arc::new(BooleanArray::new_null(num_rows)), | |||
| PrimitiveType::Timestamp | PrimitiveType::TimestampNtz => { | |||
There was a problem hiding this comment.
Btw, this will create an array with timezone UTC for timestampNtz as well
Just tracking some work required to integrate kernel into delta-rs.
Specifically the serde support may not necessarily something we want to keep supporting indefinitely but right now it would require some massive refactors in delta-rs ...
also includes changes from #190