alamb commented on code in PR #15841:
URL: https://github.com/apache/datafusion/pull/15841#discussion_r2069576418
##########
benchmarks/src/util/options.rs:
##########
@@ -70,14 +71,12 @@ impl CommonOpt {
}
/// Modify the existing config appropriately
- pub fn update_config(&self, mut config: SessionConfig) -> SessionConfig {
- if let Some(batch_size) = self.batch_size {
- config = config.with_batch_size(batch_size)
- }
-
- if let Some(partitions) = self.partitions {
- config = config.with_target_partitions(partitions)
- }
+ pub fn update_config(&self, config: SessionConfig) -> SessionConfig {
+ let mut config = config
+ .with_target_partitions(
+ self.partitions.unwrap_or_else(get_available_parallelism),
Review Comment:
the original code didn't call get_available_parallelism -- this doesn't look
equivalent ot me
I also think the original version of this code was easier to understad --
what is the rationale to change it?
##########
datafusion/common/src/column.rs:
##########
@@ -130,8 +130,8 @@ impl Column {
/// where `"foo.BAR"` would be parsed to a reference to column named
`foo.BAR`
pub fn from_qualified_name(flat_name: impl Into<String>) -> Self {
let flat_name = flat_name.into();
- Self::from_idents(parse_identifiers_normalized(&flat_name,
false)).unwrap_or(
- Self {
+ Self::from_idents(parse_identifiers_normalized(&flat_name,
false)).unwrap_or_else(
Review Comment:
this makes sense to me
--
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-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
