talatuyarer commented on code in PR #17878:
URL: https://github.com/apache/iceberg/pull/17878#discussion_r3896592918
##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/source/IcebergTableSource.java:
##########
@@ -228,6 +251,30 @@ public Optional<Integer> getParallelism() {
};
}
+ @Override
+ public TableStats reportStatistics() {
+ try {
+ if (!FlinkSource.isBounded(properties)) {
+ return TableStats.UNKNOWN;
+ }
+
+ if (TIME_TRAVEL_OPTIONS.stream().anyMatch(properties::containsKey)) {
+ return TableStats.UNKNOWN;
+ }
+
+ boolean columnStatsEnabled =
+
readableConfig.get(FlinkConfigOptions.TABLE_EXEC_ICEBERG_REPORT_COLUMN_STATISTICS);
+ try (TableLoader tableLoader = loader.clone()) {
+ tableLoader.open();
+ Table table = tableLoader.loadTable();
+ return FlinkTableStatistics.reportStatistics(table, filters,
columnStatsEnabled);
+ }
+ } catch (Exception e) {
+ LOG.warn("Failed to report statistics for Iceberg table, returning
unknown stats", e);
+ return TableStats.UNKNOWN;
+ }
Review Comment:
I updated table loading with a single loading appaorch as you suggested.
For config, I agree it's safe to have this on by default, and it stays on by
default. I'd still prefer to keep the option for at least one release as an
operational escape hatch: the `column-stats` path reads manifest metadata
during planning, so on tables with very large metadata it adds planning latency
to every batch query. The global
`table.optimizer.source.report-statistics-enabled` switch is all-or-nothing.
turning it off also loses the row-count estimate, which is nearly free
(snapshot summary only). This option disables only the expensive part while
keeping row counts flowing to the planner. If it proves unnecessary after a
release in the wild, I'm happy to deprecate and remove it.
--
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]
