alamb commented on code in PR #23189:
URL: https://github.com/apache/datafusion/pull/23189#discussion_r3554849635


##########
datafusion/datasource/src/file_scan_config/mod.rs:
##########


Review Comment:
   It seems to me like this thread is raising an excellent point (we could use 
a more precise partitioning in this case of hive partitioning)
   
   I think main also chooses this Hash partitioning for HIVE tables
   
   Thus I think we should merge this PR in and then file a ticket to discuss/ 
plan for potentially using Value partitioning for HIVE partitioned tables in 
the future. 



##########
datafusion/proto/src/physical_plan/to_proto.rs:
##########
@@ -562,7 +562,9 @@ pub fn serialize_file_scan_config(
         constraints: Some(conf.constraints.clone().into()),
         batch_size: conf.batch_size.map(|s| s as u64),
         projection_exprs,
-        partitioned_by_file_group: Some(conf.partitioned_by_file_group),
+        // Partition grouping is now encoded in `output_partitioning`; this 
legacy

Review Comment:
   FWIW we don't typically guartantee wire format compatibility: 
https://docs.rs/datafusion-proto/latest/datafusion_proto/#version-compatibility
   
   Maybe we can remove the backward compatiblity shim in a follow on 



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