comphead commented on code in PR #7227:
URL: https://github.com/apache/datafusion-comet/pull/7227#discussion_r4379459779


##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -678,13 +677,45 @@ fn spark_has_updater(
         return false;
     }
 
+    // A timestamp leaf is INT96, or INT64 annotated as a timestamp. Spark's 
INT96 updaters
+    // produce only timestamps, and its INT64 updaters produce a timestamp or 
a long (a decimal
+    // needs a decimal annotation), so a timestamp read as a string, a 
narrower integer, a float
+    // or a decimal has no updater. Without this, the cast returns text, raw 
numbers or NULL.
+    // See #7214.
+    if matches!(physical_type, DataType::Timestamp(_, _))
+        && (is_string_or_binary(target_type)
+            || matches!(
+                target_type,
+                DataType::Int8
+                    | DataType::Int16
+                    | DataType::Int32
+                    | DataType::Float32
+                    | DataType::Float64
+                    | DataType::Decimal128(_, _)
+                    | DataType::Decimal256(_, _)
+            ))
+    {
+        return false;

Review Comment:
   This block splits across two rules that `spark_has_updater` already has. The 
string and binary half is the numeric-to-string rule above 
(`physical_is_primitive_numeric && is_string_or_binary(target_type)`). The 
numeric half fits the `is_spark_rejected_conversion` table, which already lists 
`(Timestamp, Date32)`. Adding `Timestamp(_, _)` and `Date32` to that first 
list, and the `(Timestamp, Int8 | Int16 | Int32 | Float32 | Float64 | 
Decimal*)` pairs to the table, keeps one place to read the rejections.
   
   It would also cover DATE, which has the same gap. Spark's INT32 case has no 
`StringType` or `FloatType` branch on any version, and `isDecimalTypeMatched` 
refuses a date annotation. Here `Date32` to `Utf8`, `Float32` or `Decimal128` 
passes every rule, so I expect a DATE column read as `string` to return text 
natively where Spark raises `PARQUET_COLUMN_DATA_TYPE_MISMATCH`. I traced this 
from the code and have not run it. If it is out of scope for #7214, a follow-up 
issue would do.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to