Re: [PR] Change default Parquet writer settings to match arrow-rs (except for compression & statistics) [datafusion]

["alamb (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org