alamb commented on code in PR #11558:
URL: https://github.com/apache/datafusion/pull/11558#discussion_r1685388560
##########
datafusion/common/src/config.rs:
##########
@@ -314,6 +314,85 @@ config_namespace! {
}
}
+/// When using the parquet feature,
+/// use the same default writer settings as the extern parquet.
+#[cfg(feature = "parquet")]
Review Comment:
I think the idea of defining these constants is to try and keep the values
in sync with what is in the arrow-rs writer.
However, since we now have a test that verifies the values it is my opinion
that this additional level of indirection (and the stubs required with a
different feature flag) make the code harder to read for minimal additional
benefit.
In other words, I think keeping the config values as hard coded constants
with the tests is sufficient and we could remove the `parquet_defaults` modules
--
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]
