kumarUjjawal commented on code in PR #24227:
URL: https://github.com/apache/datafusion/pull/24227#discussion_r4119052171
##########
datafusion/datasource-parquet/src/schema_coercion.rs:
##########
@@ -84,6 +84,183 @@ pub fn apply_file_schema_type_coercions(
))
}
+/// Like [`apply_file_schema_type_coercions`], but also coerces compatible
+/// string/binary file fields to dictionary types already present in the table
+/// schema.
+pub(crate) fn apply_file_schema_type_coercions_with_rle(
+ table_schema: &Schema,
+ file_schema: &Schema,
+ enable_rle_to_dictionary: bool,
+) -> Option<Schema> {
+ let mut needs_view_transform = false;
+ let mut needs_string_transform = false;
+ let mut needs_nested_transform = false;
+ let mut needs_dict_transform = false;
+
+ // Create a mapping of table field names to their data types for fast
lookup
+ // and simultaneously check if we need any transformations
+ let table_fields: HashMap<_, _> = table_schema
+ .fields()
+ .iter()
+ .map(|field| {
+ let data_type = field.data_type();
+ // Check if we need view type transformation
+ if matches!(data_type, &DataType::Utf8View |
&DataType::BinaryView) {
+ needs_view_transform = true;
+ }
+ // Check if we need string type transformation
+ if matches!(
+ data_type,
+ &DataType::Utf8 | &DataType::LargeUtf8 | &DataType::Utf8View
+ ) {
+ needs_string_transform = true;
+ }
+ // Nested fields can need transformations even when their parent
does not.
+ if matches!(
+ data_type,
+ DataType::Struct(_)
+ | DataType::List(_)
+ | DataType::LargeList(_)
+ | DataType::ListView(_)
+ | DataType::LargeListView(_)
+ | DataType::FixedSizeList(_, _)
+ | DataType::Map(_, _)
+ ) {
+ needs_nested_transform = true;
+ }
+ if enable_rle_to_dictionary
+ && matches!(data_type, &DataType::Dictionary(_, _))
+ {
+ needs_dict_transform = true;
+ }
+
+ (field.name(), data_type)
+ })
+ .collect();
+
+ // Early return if no transformation needed
+ if !needs_view_transform
+ && !needs_string_transform
+ && !needs_nested_transform
+ && !needs_dict_transform
+ {
+ return None;
+ }
+
+ let fields: Vec<Arc<Field>> = file_schema
Review Comment:
This allocates a new vector and clones every field for common string or
nested schemas. It also does this when `enable_rle_to_dictionary` is false and
no field changes. The existing `coerce_fields` helper allocates only after the
first change. Please add dictionary promotion to the existing coercion path
instead of copying the full implementation.
--
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]
