martin-g commented on code in PR #19931:
URL: https://github.com/apache/datafusion/pull/19931#discussion_r2716765009
##########
datafusion/proto/src/physical_plan/from_proto.rs:
##########
@@ -737,6 +737,8 @@ impl TryFrom<&protobuf::FileSinkConfig> for FileSinkConfig {
insert_op,
keep_partition_by_columns: conf.keep_partition_by_columns,
file_extension: conf.file_extension.clone(),
+ // For deserialized plans, use extension heuristic (backward
compatible)
Review Comment:
For how long this backward compatible approach will be used ? At some point
it should start using the new config property.
IMO the proper way would be to add a new optional field to [the Protobuf
format](https://github.com/kumarUjjawal/datafusion/blob/ab870580248d8d6ab885f9aa56296b4ec6682f50/datafusion/proto/proto/datafusion.proto#L773),
then try to read it and use `None` only if it is missing, but if it is not
missing then use its value.
--
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]
