uros-b commented on code in PR #17461:
URL: https://github.com/apache/iceberg/pull/17461#discussion_r3700434548
##########
parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java:
##########
@@ -363,7 +363,11 @@ private void setColumnStatsConfig(
LOG.warn("Skipping column statistics config for missing
field: {}", colPath);
return;
}
- withColumnStatsEnabled.accept(parquetColumnPath,
Boolean.valueOf(isEnabled));
+ boolean enabled = Boolean.parseBoolean(isEnabled);
+ withColumnStatsEnabled.accept(parquetColumnPath, enabled);
+ conf.set(
+ "parquet.column.statistics.enabled#" + parquetColumnPath,
Review Comment:
The added conf.set("parquet.column.statistics.enabled#" + parquetColumnPath,
String.valueOf(enabled)) is dead code. The key
parquet.column.statistics.enabled#<col> is consumed only by
ParquetOutputFormat.ColumnConfigParser (the Hadoop MapReduce write path);
Iceberg never invokes ParquetOutputFormat, so this key is never read back by
either the createWriterFunc path (ParquetProperties.Builder, line 487) or the
ParquetWriteBuilder path (line 532). The real per-column setting routes through
withColumnStatsEnabled.accept(parquetColumnPath, enabled) at line 367 →
propsBuilder::withStatisticsEnabled /
parquetWriteBuilder::withStatisticsEnabled → ColumnProperty.Builder HashMap,
which already handles multiple columns independently. The production change is
based on a false root-cause diagnosis and should be reverted entirely.
--
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]
