-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Core: Read content stats from v4 Manifest #17433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
da50e80
6cf0fd0
fa6be67
93cbddc
f9a7dd6
ad7a0fe
531ae2e
8397f89
bbbecae
33d52b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
| import java.util.Collection; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import org.apache.iceberg.expressions.Binder; | ||
| import org.apache.iceberg.expressions.Evaluator; | ||
| import org.apache.iceberg.expressions.Expression; | ||
| import org.apache.iceberg.expressions.Expressions; | ||
|
|
@@ -32,11 +33,14 @@ | |
| import org.apache.iceberg.io.InputFile; | ||
| import org.apache.iceberg.metrics.ScanMetrics; | ||
| import org.apache.iceberg.relocated.com.google.common.base.Preconditions; | ||
| import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; | ||
| import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; | ||
| import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet; | ||
| import org.apache.iceberg.relocated.com.google.common.collect.Maps; | ||
| import org.apache.iceberg.relocated.com.google.common.collect.Sets; | ||
| import org.apache.iceberg.types.TypeUtil; | ||
| import org.apache.iceberg.types.Types; | ||
| import org.apache.iceberg.util.ArrayUtil; | ||
| import org.apache.iceberg.util.Pair; | ||
| import org.apache.iceberg.util.StructProjection; | ||
|
|
||
|
|
@@ -63,8 +67,9 @@ private V4ManifestReader( | |
| this.scanMetrics = scanMetrics; | ||
| } | ||
|
|
||
| static Builder builder(InputFile file, Map<Integer, PartitionSpec> specsById) { | ||
| return new Builder(file, specsById); | ||
| static Builder builder( | ||
| InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> specsById) { | ||
| return new Builder(file, tableSchema, specsById); | ||
| } | ||
|
|
||
| /** Returns copies of the tracked files that match this reader's configured filters. */ | ||
|
|
@@ -122,16 +127,27 @@ private CloseableIterable<TrackedFile> open() { | |
| Preconditions.checkArgument( | ||
| format != null, "Cannot determine format of manifest: %s", file.location()); | ||
|
|
||
| CloseableIterable<TrackedFile> reader = | ||
| InternalData.ReadBuilder readBuilder = | ||
| InternalData.read(format, file) | ||
| .project(readSchema) | ||
| .setRootType(TrackedFileStruct.class) | ||
| .setCustomType(TrackedFile.TRACKING.fieldId(), TrackingStruct.class) | ||
| .setCustomType(TrackedFile.DELETION_VECTOR.fieldId(), DeletionVectorStruct.class) | ||
| .setCustomType(TrackedFile.MANIFEST_INFO.fieldId(), ManifestInfoStruct.class) | ||
| .setCustomType(TrackedFile.PARTITION_ID, PartitionData.class) | ||
| .reuseContainers() | ||
| .build(); | ||
| .reuseContainers(); | ||
|
|
||
| // content_stats is missing from the read schema when no stats are read | ||
| Types.NestedField statsField = readSchema.findField(TrackedFile.CONTENT_STATS_ID); | ||
| if (statsField != null) { | ||
| readBuilder.setCustomType(TrackedFile.CONTENT_STATS_ID, ContentStatsStruct.class); | ||
| // content_stats holds one stats struct per projected column | ||
| for (Types.NestedField fieldStats : statsField.type().asStructType().fields()) { | ||
| readBuilder.setCustomType(fieldStats.fieldId(), FieldStatsStruct.class); | ||
| } | ||
| } | ||
|
|
||
| CloseableIterable<TrackedFile> reader = readBuilder.build(); | ||
| addCloseable(reader); | ||
| return reader; | ||
| } | ||
|
|
@@ -153,26 +169,24 @@ private static boolean isManifest(TrackedFile trackedFile) { | |
|
|
||
| static class Builder { | ||
| private final InputFile file; | ||
| private final Schema tableSchema; | ||
| private final Types.StructType unionPartitionType; | ||
| private final Map<Integer, PartitionSpec> specsById; | ||
| private final Schema fullSchema; | ||
| private Expression rowFilter = Expressions.alwaysTrue(); | ||
| private boolean caseSensitive = true; | ||
| private boolean includeAll = false; | ||
| private boolean scanPlanning = false; | ||
| private Collection<String> columns = null; | ||
| private Schema requestedProjection = null; | ||
| private Set<Integer> statsProjectionForFieldIds = null; | ||
| private ScanMetrics scanMetrics = ScanMetrics.noop(); | ||
|
|
||
| private Builder(InputFile file, Map<Integer, PartitionSpec> specsById) { | ||
| private Builder(InputFile file, Schema tableSchema, Map<Integer, PartitionSpec> specsById) { | ||
| Preconditions.checkArgument(tableSchema != null, "Invalid table schema: null"); | ||
| this.file = file; | ||
| this.tableSchema = tableSchema; | ||
| this.specsById = specsById; | ||
| this.unionPartitionType = Partitioning.unionPartitionTypes(specsById.values()); | ||
| Schema base = TrackedFile.schema(unionPartitionType, Types.StructType.of()); | ||
| // the read schema carries row_position (via BASE_TYPE) so the reader can fill manifestPos | ||
| this.fullSchema = | ||
| TypeUtil.replaceFieldTypes( | ||
| base, ImmutableMap.of(TrackedFile.TRACKING.fieldId(), TrackingStruct.BASE_TYPE)); | ||
| } | ||
|
|
||
| /** Sets a filter; files that cannot match the expression are skipped. */ | ||
|
|
@@ -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"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||
| this.statsProjectionForFieldIds = ImmutableSet.copyOf(fieldIds); | ||
| return this; | ||
| } | ||
|
|
||
| Builder scanMetrics(ScanMetrics newScanMetrics) { | ||
| Preconditions.checkArgument(newScanMetrics != null, "Invalid scan metrics: null"); | ||
| this.scanMetrics = newScanMetrics; | ||
|
|
@@ -254,6 +287,8 @@ V4ManifestReader build() { | |
| } | ||
|
|
||
| private Schema readSchema(boolean hasPartitionFilter) { | ||
| Set<Integer> requiredFieldIds = requiredStatsProjectionForFieldIds(); | ||
| Schema fullSchema = fullSchema(requiredFieldIds); | ||
|
Comment on lines
+290
to
+291
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: curious if we shall build |
||
| if (scanPlanning) { | ||
| // scan planning does not read the change-tracking fields omitted by SCAN_TYPE | ||
| return TypeUtil.replaceFieldTypes( | ||
|
|
@@ -263,17 +298,65 @@ private Schema readSchema(boolean hasPartitionFilter) { | |
| if (columns != null) { | ||
| Schema selected = | ||
| caseSensitive ? fullSchema.select(columns) : fullSchema.caseInsensitiveSelect(columns); | ||
| return addRequiredColumns(selected, hasPartitionFilter); | ||
| return addRequiredColumns(fullSchema, selected, requiredFieldIds, hasPartitionFilter); | ||
| } | ||
|
|
||
| if (requestedProjection != null) { | ||
| return addRequiredColumns(requestedProjection, hasPartitionFilter); | ||
| return addRequiredColumns( | ||
| fullSchema, requestedProjection, requiredFieldIds, hasPartitionFilter); | ||
| } | ||
|
|
||
| return fullSchema; | ||
| } | ||
|
|
||
| private Schema addRequiredColumns(Schema projection, boolean hasPartitionFilter) { | ||
| /** Returns the schema of everything this reader may read, including content stats. */ | ||
| private Schema fullSchema(Set<Integer> requiredStatsProjectionFieldIds) { | ||
| Types.StructType contentStatsType = contentStatsType(requiredStatsProjectionFieldIds); | ||
| Schema base = TrackedFile.schema(unionPartitionType, contentStatsType); | ||
| if (contentStatsType.fields().isEmpty()) { | ||
| // schema uses the unknown type for empty stats, which cannot be paired with the stats | ||
| // struct in the manifest, so drop the field instead of reading it as unknown | ||
| base = TypeUtil.selectNot(base, ImmutableSet.of(TrackedFile.CONTENT_STATS_ID)); | ||
| } | ||
|
|
||
| // the read schema carries row_position (via BASE_TYPE) so the reader can fill manifestPos | ||
| return TypeUtil.replaceFieldTypes( | ||
| base, ImmutableMap.of(TrackedFile.TRACKING.fieldId(), TrackingStruct.BASE_TYPE)); | ||
| } | ||
|
|
||
| /** Returns the stats type to read, which is empty when no stats are needed. */ | ||
| private Types.StructType contentStatsType(Set<Integer> requiredStatsProjectionForFieldIds) { | ||
| if (scanPlanning || statsProjectionForFieldIds != null) { | ||
| // scan planning and projectStats(fieldIds) both narrow the set of stats that are read | ||
| return StatsUtil.statsReadSchema(tableSchema, requiredStatsProjectionForFieldIds); | ||
| } | ||
|
|
||
| return StatsUtil.statsReadSchema( | ||
| tableSchema, TypeUtil.indexById(tableSchema.asStruct()).keySet()); | ||
| } | ||
|
|
||
| /** Returns the IDs of table fields whose stats are read regardless of the projection. */ | ||
| private Set<Integer> requiredStatsProjectionForFieldIds() { | ||
| Set<Integer> fieldIds = Sets.newHashSet(); | ||
| if (statsProjectionForFieldIds != null) { | ||
| fieldIds.addAll(statsProjectionForFieldIds); | ||
| } | ||
|
|
||
| if (rowFilter != Expressions.alwaysTrue()) { | ||
| // stats for filter references are read so that the filter can be evaluated against them | ||
| fieldIds.addAll( | ||
| Binder.boundReferences( | ||
| tableSchema.asStruct(), ImmutableList.of(rowFilter), caseSensitive)); | ||
| } | ||
|
|
||
| return fieldIds; | ||
| } | ||
|
|
||
| private Schema addRequiredColumns( | ||
| Schema fullSchema, | ||
| Schema projection, | ||
| Set<Integer> requiredStatsProjectionFieldIds, | ||
| boolean hasPartitionFilter) { | ||
| Set<Integer> projectedIds = Sets.newHashSet(TypeUtil.getProjectedIds(projection)); | ||
|
|
||
| // fields the reader consumes internally: status for liveness filtering, row_position for | ||
|
|
@@ -293,6 +376,15 @@ private Schema addRequiredColumns(Schema projection, boolean hasPartitionFilter) | |
| projectedIds.addAll(TypeUtil.getProjectedIds(unionPartitionType)); | ||
| } | ||
|
|
||
| // stats needed by the filter or requested by projectStats are read even when the caller's | ||
| // projection omits them | ||
| Types.StructType requiredStatsType = | ||
| StatsUtil.statsReadSchema(tableSchema, requiredStatsProjectionFieldIds); | ||
| if (!requiredStatsType.fields().isEmpty()) { | ||
| projectedIds.add(TrackedFile.CONTENT_STATS_ID); | ||
| projectedIds.addAll(TypeUtil.getProjectedIds(requiredStatsType)); | ||
| } | ||
|
|
||
| // project instead of select to preserve narrow struct projections from the caller | ||
| return TypeUtil.project(fullSchema, projectedIds); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does geo and variant need any special handling here? Can we add a test to make sure those types work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, I've added tests for geo + variant types with single/multiple files and this uncovered a bug around Geo types copying, which I've fixed in
FieldStatsStruct