dramaticlly commented on code in PR #17433:
URL: https://github.com/apache/iceberg/pull/17433#discussion_r3708542892
##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -228,6 +242,25 @@ Builder project(Schema newProjection) {
return this;
}
+ /**
+ * Reads content stats for the given table field IDs instead of for every
field. Stats for
+ * fields referenced by the {@link #filter(Expression) filter} are always
read.
+ */
+ Builder projectStats(int... fieldIds) {
+ Preconditions.checkArgument(fieldIds != null, "Invalid stats projection
for field IDs: null");
+ return projectStats(ArrayUtil.toIntList(fieldIds));
+ }
+
+ /**
+ * Reads content stats for the given table field IDs instead of for every
field. Stats for
+ * fields referenced by the {@link #filter(Expression) filter} are always
read.
+ */
+ Builder projectStats(Iterable<Integer> fieldIds) {
+ Preconditions.checkArgument(fieldIds != null, "Invalid stats projection
for field IDs: null");
Review Comment:
I think we have 4 modes relevant to stats so far, the default/CDC, scan
planing, select column by name and project schema and we conditionally add
required column depends on the filter. I am wondering if we want to add
coverage for
- empty projectStats and filter (ok to use precondition to check if this
combination does not make sense)
- valid projectStats and filter with default mode ( I think scan planning is
already covered in `projectStatsAndFilterStatsAreCombined`)
##########
core/src/main/java/org/apache/iceberg/V4ManifestReader.java:
##########
@@ -254,6 +287,8 @@ V4ManifestReader build() {
}
private Schema readSchema(boolean hasPartitionFilter) {
+ Set<Integer> requiredFieldIds = requiredStatsProjectionForFieldIds();
+ Schema fullSchema = fullSchema(requiredFieldIds);
Review Comment:
nit: curious if we shall build `requiredStatsType` for reading stats
regardless of caller's column projection ahead of time instead of passing them
to calculate multiple times later? Can potentially pass requiredStatsType to
`fullSchema()` and `addRequiredColumns()`.
--
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]
