anuragmantri commented on code in PR #3424:
URL: https://github.com/apache/iceberg-rust/pull/3424#discussion_r4299950544
##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -211,6 +232,27 @@ impl<'a> TableScanBuilder<'a> {
self
}
+ /// Retains the data file's column statistics for the named columns on
each planned
+ /// [`FileScanTask`], exposed as [`FileScanTask::column_stats`].
+ ///
+ /// Names resolve against the scan's schema with the scan's case
sensitivity and may name
+ /// nested fields. By default, tasks carry no statistics.
+ ///
+ /// [`build`](Self::build) fails if a name matches no field, or more than
one field when the
+ /// scan is case-insensitive.
+ pub fn with_column_stats(
Review Comment:
Yes, that's intended for now. Naming the columns keeps the default cheap,
which was the concern about task size in #607, and the engines I have in mind
only need the sort columns. If one needs every column, a
`with_all_column_stats()` is a small follow-up.
##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -211,6 +232,27 @@ impl<'a> TableScanBuilder<'a> {
self
}
+ /// Retains the data file's column statistics for the named columns on
each planned
+ /// [`FileScanTask`], exposed as [`FileScanTask::column_stats`].
+ ///
+ /// Names resolve against the scan's schema with the scan's case
sensitivity and may name
+ /// nested fields. By default, tasks carry no statistics.
Review Comment:
Good catch. In fe1bf907a, `build()` rejects a struct, list or map name with
`Column s is not a primitive field and has no column statistics`, and the doc
now says names must be primitive fields.
`test_collect_column_stats_field_ids_rejects_struct_field` covers it.
##########
crates/iceberg/src/scan/mod.rs:
##########
@@ -103,6 +107,21 @@ fn collect_scan_field_ids(
.collect()
}
+fn collect_column_stats_field_ids(
Review Comment:
Agreed. In fe1bf907a, a metadata column now fails with `Metadata column
_file has no column statistics`, and the planning test asserts that message.
##########
crates/iceberg/src/scan/task.rs:
##########
@@ -760,6 +826,106 @@ impl From<FileScanTaskDeleteFile> for
Result<FileScanTaskDeleteFile> {
}
}
+/// Column statistics of a [`FileScanTask`]'s data file, keyed by field id and
limited to the
+/// columns requested for the scan.
+///
+/// A requested column missing from a map has unknown statistics of that kind,
for example when
+/// the table's metrics mode records no bounds. Bounds are inclusive even when
truncated, and are
+/// typed with the schema the manifest was written with, so after type
promotion an `int` bound
Review Comment:
Added `test_plan_files_keeps_bounds_typed_with_manifest_schema` in
fe1bf907a. It writes the manifest with `y` as an `int` while the table schema
says `long`, and checks the bounds come back as `Datum::int`. This holds
because the manifest reader parses bounds with the schema stored in the
manifest.
--
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]
