sweb commented on a change in pull request #9047:
URL: https://github.com/apache/arrow/pull/9047#discussion_r550468985
##########
File path: rust/arrow/src/compute/kernels/cast.rs
##########
@@ -532,6 +533,7 @@ pub fn cast(array: &ArrayRef, to_type: &DataType) ->
Result<ArrayRef> {
(Int64, Int32) => cast_numeric_arrays::<Int64Type, Int32Type>(array),
(Int64, Float32) => cast_numeric_arrays::<Int64Type,
Float32Type>(array),
(Int64, Float64) => cast_numeric_arrays::<Int64Type,
Float64Type>(array),
+ (Int64, Decimal(p, s)) => int64_to_decimal_cast(array, *p, *s),
Review comment:
I think you stumbled upon the issue I tried to explain in the PR
description - I am now sure that it was a bad idea to use casts in this way :)
I removed the cast operations and replaced it with some logic in
`PrimitiveArrayReader::next_batch`.
##########
File path: rust/parquet/src/arrow/schema.rs
##########
@@ -591,6 +591,7 @@ impl ParquetTypeConverter<'_> {
LogicalType::INT_32 => Ok(DataType::Int32),
LogicalType::DATE => Ok(DataType::Date32(DateUnit::Day)),
LogicalType::TIME_MILLIS =>
Ok(DataType::Time32(TimeUnit::Millisecond)),
+ LogicalType::DECIMAL => self.to_decimal(),
Review comment:
I am having a hard time finding the right place to construct the decimal
type. My main concern with the changes I made is that Decimal is the only type
that needs additional info from the schema in order to define precision and
scale - all other types can be built directly from the logical type. However,
if I put it in `PrimitiveArrayReader::new` I am only able to accomplish this by
reimplementing parts of the ParquetTypeConverter specifically for Decimal. Do
you have a suggestion how I can handle this in a better way?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
