duckdb-rs
duckdb-rs copied to clipboard
Support for Decimal128 & Decimal256 without downcasting to f64
Overview
Currently Duckdb-rs supports Decimal128, but not Decimal256. However, its support for Decimal128 downcasts to f64. See decimal_array_to_vector.
/// Convert Arrow [Decimal128Array] to a duckdb vector.
fn decimal_array_to_vector(array: &Decimal128Array, out: &mut FlatVector) {
assert!(array.len() <= out.capacity());
for i in 0..array.len() {
out.as_mut_slice()[i] = array.value_as_string(i).parse::<f64>().unwrap();
}
}
This fails a check_rust_primitive_array_roundtrip, when I ran it locally.
assertion `left == right` failed
left: Float64
right: Decimal128(38, 10)
The underlying cause of the issue is that duckdb-rs explicitly converts to Double, and not Decimal see code
// duckdb/src/main/capi/helper-c.cpp does not support decimal
// DataType::Decimal128(_, _) => Decimal,
// DataType::Decimal256(_, _) => Decimal,
DataType::Decimal128(_, _) => Double,
DataType::Decimal256(_, _) => Double,
The comment above reference the fact Decimal does not appear in https://github.com/duckdb/duckdb/blob/main/src/main/capi/helper-c.cpp#L5-L61.
Feature
Support for Decimal128 and Decimal256.
Question
Is this issue in capi/helper-c.cpp an intrinisic issue with DuckDB, and if not, what work would be required to support all Decimal types in the Rust bindings?
Hi! I don't know if helper-c.cpp matters, struct, union and list types aren't present there either, ~~but It's true that the c-api doesn't expose a way to create decimal types using the standard duckdb_create_logical_type. The issue is that since decimal types are "parameterized" you'd need a different constructor function that allows you to pass the width and scale, (similar to e.g. duckdb_create_list_type), but thats definitely something we could look into adding.~~
We do have a duckdb_create_decimal_type and a duckdb_create_decimal function, so adding support for this in the rust api not be that complex.
In the case of 256-width decimals however, thats more of an expected limitation as DuckDB itself doesn't support 256-width decimals internally.
The vtab module does have some support already: https://docs.rs/duckdb/latest/duckdb/vtab/struct.LogicalType.html#method.decimal
Even if DuckDB-rs has some typing for Decimal128, I think the core issue is that duckdb_execute_prepared_arrow (i.e. https://github.com/duckdb/duckdb/blob/5b36f520df72be12f90ae31e6f8ed797a7b78e99/src/main/capi/arrow-c.cpp#L163) uses helper-c.cpp and if that doesn't support Decimal128, no change to only duckdb-rs will fix it.
I don't have the best understanding of duckdb specifically.
Where are you seeing that function used in the codepath duckdb-rs uses?
Running
let schema = Schema::new(vec![Field::new("a", input_array.data_type().clone(), false)]);
let rb = RecordBatch::try_new(Arc::new(schema), vec![Arc::new(input_array.clone())])?;
let mut stmt = db.prepare("select a from arrow(?, ?)")?;
let rb = stmt.query_arrow(param)?
stmt.query_arrow calls duckdb_execute_prepared_arrow, via:
- https://github.com/duckdb/duckdb-rs/blob/a1aa55aff22b75e149e9cf7cface6464b3dc0ccc/src/statement.rs#L107
- https://github.com/duckdb/duckdb-rs/blob/a1aa55aff22b75e149e9cf7cface6464b3dc0ccc/src/statement.rs#L63
- https://github.com/duckdb/duckdb-rs/blob/a1aa55aff22b75e149e9cf7cface6464b3dc0ccc/src/statement.rs#L510
- https://github.com/duckdb/duckdb-rs/blob/a1aa55aff22b75e149e9cf7cface6464b3dc0ccc/src/raw_statement.rs#L223
- https://github.com/duckdb/duckdb-rs/blob/a1aa55aff22b75e149e9cf7cface6464b3dc0ccc/src/raw_statement.rs#L227
As mentioned DuckDB doesn't support decimal256 or decimals with negative scale, but I've added support for positive decimal128 conversion here https://github.com/duckdb/duckdb-rs/pull/328