feat: adopt kernel schema types #2495
Conversation
github-actions
bot
added
binding/python
| ArrowDataType::Decimal128(p, s) => { | ||
| Ok(DataType::Primitive(PrimitiveType::Decimal(*p, *s))) | ||
| } | ||
| ArrowDataType::Decimal256(p, s) => DataType::decimal(*p, *s).map_err(|_| { |
There was a problem hiding this comment.
This arrow type is missing in kernel, I think that should be upstreamed there as well.
There was a problem hiding this comment.
hmm, given that precision/scale are limited to 38, does that make sense? not entirely sure anymore but I think the 256 bit type only makes sense for larger p/s values.
There was a problem hiding this comment.
It's mostly for convenience. Users might have their source data using decimal256, but still precision/scale below 38.
| @@ -1458,7 +1458,7 @@ def test_invalid_decimals(tmp_path: pathlib.Path, engine): | |||
|
|
|||
| with pytest.raises( | |||
| SchemaMismatchError, | |||
| match=re.escape("Invalid data type for Delta Lake: decimal(39,1)"), | |||
| match=re.escape("Invalid data type for Delta Lake: Decimal256(39, 1)"), | |||
There was a problem hiding this comment.
this change is a bit more significant then meets the eye - i.e. we are no longer trying to convert 256 bit decimals to a compliant type in delta. main reason being, for precisions that fit into 128 bit decimals, the user should really be using these, if the larger type is required, we cannot store it in the table anyhow.
| Struct(fields) => { | ||
| let struct_fields = fields | ||
| .iter() | ||
| .flat_map(|f| TryFrom::try_from(f.as_ref())) | ||
| .collect::<Vec<_>>(); | ||
| let values = arr | ||
| .as_any() | ||
| .downcast_ref::<StructArray>() | ||
| .and_then(|struct_arr| { | ||
| struct_fields | ||
| .iter() | ||
| .map(|f: &StructField| { | ||
| struct_arr | ||
| .column_by_name(f.name()) | ||
| .and_then(|c| Self::from_array(c.as_ref(), index)) | ||
| }) | ||
| .collect::<Option<Vec<_>>>() | ||
| })?; | ||
| if struct_fields.len() != values.len() { | ||
| return None; | ||
| } | ||
| Some(Self::Struct( | ||
| StructData::try_new(struct_fields, values).ok()?, | ||
| )) | ||
| } |
There was a problem hiding this comment.
@scovich - here is an example of creating a struct scalar in the wild.
Description
First pass adopting
delta_kernelindelta-rs.depends on delta-incubator/delta-kernel-rs#189 being merged and released.
This PR focusses on the schema types. Adopting the action types would be a follow up, but might have a even greater blast radius then this one.
Related Issue(s)
part of #2489
Documentation