pvary commented on code in PR #17320:
URL: https://github.com/apache/iceberg/pull/17320#discussion_r3978564152
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java:
##########
@@ -129,12 +132,103 @@ public static Type fieldType(GroupType group, String
name) {
public static MessageType pruneColumns(MessageType fileSchema, Schema
expectedSchema) {
// column order must match the incoming type, so it doesn't matter that
the ids are unordered
- Set<Integer> selectedIds = TypeUtil.getProjectedIds(expectedSchema);
+ Set<Integer> selectedIds =
Sets.newHashSet(TypeUtil.getProjectedIds(expectedSchema));
+ // retain one real leaf under each struct that projects only constants
like default values,
+ // so its definition level still shows whether the struct is present
+ TypeWithSchemaVisitor.visit(
+ expectedSchema.asStruct(), fileSchema, new
PresenceColumnSelector(fileSchema, selectedIds));
return (MessageType)
TypeWithSchemaVisitor.visit(
expectedSchema.asStruct(), fileSchema, new
PruneColumns(selectedIds));
}
+ /**
+ * Adds one leaf id under each projected struct whose fields are all
constants and would otherwise
+ * retain no file leaf. That leaf's definition level is what still shows
whether the struct is
+ * present.
+ */
+ private static class PresenceColumnSelector extends
TypeWithSchemaVisitor<Void> {
+ private final MessageType fileSchema;
+ private final Set<Integer> selectedIds;
+
+ private PresenceColumnSelector(MessageType fileSchema, Set<Integer>
selectedIds) {
+ this.fileSchema = fileSchema;
+ this.selectedIds = selectedIds;
+ }
+
+ @Override
+ public Void struct(Types.StructType expected, GroupType struct, List<Void>
fields) {
+ // nothing projected under this struct, so there is nothing to track
+ if (expected == null || expected.fields().isEmpty()) {
+ return null;
+ }
+
+ String[] path = currentPath();
+ // add a presence column only if no real leaf under the struct is
already read
+ boolean readsRealLeaf =
+ leafColumns(fileSchema, path).stream()
+ .anyMatch(leaf -> selectedIds.contains(leafId(leaf)));
+ if (!readsRealLeaf) {
+ ColumnDescriptor presence = selectPresenceColumn(fileSchema, path);
+ if (presence != null) {
+ selectedIds.add(leafId(presence));
+ }
+ }
+
+ return null;
+ }
+
+ @Override
+ public Void variant(Types.VariantType expected, GroupType variantGroup,
Void result) {
+ return null;
+ }
+ }
+
+ /** Shallowest leaf under path; its definition level shows whether the
struct is present. */
+ static ColumnDescriptor selectPresenceColumn(MessageType fileSchema,
String[] path) {
+ if (fileSchema.getMaxDefinitionLevel(path) <= 0) {
+ return null;
+ }
+
+ return leafColumns(fileSchema, path).stream()
+ .min(Comparator.comparingInt(ColumnDescriptor::getMaxRepetitionLevel))
+ .orElse(null);
+ }
+
+ /** Returns the leaf columns with ids under the given path. */
+ static List<ColumnDescriptor> leafColumns(MessageType fileSchema, String[]
path) {
+ List<ColumnDescriptor> columns = Lists.newArrayList();
+ for (ColumnDescriptor column : fileSchema.getColumns()) {
+ // a presence column is kept in the read set by id, so a leaf without
one cannot be used
Review Comment:
nit: Shall we try to optimize and choose a "cheap" field to read (not a
binary or such)
Maybe a follow-up if we decide to go down this road.
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetSchemaUtil.java:
##########
@@ -129,12 +132,103 @@ public static Type fieldType(GroupType group, String
name) {
public static MessageType pruneColumns(MessageType fileSchema, Schema
expectedSchema) {
// column order must match the incoming type, so it doesn't matter that
the ids are unordered
- Set<Integer> selectedIds = TypeUtil.getProjectedIds(expectedSchema);
+ Set<Integer> selectedIds =
Sets.newHashSet(TypeUtil.getProjectedIds(expectedSchema));
+ // retain one real leaf under each struct that projects only constants
like default values,
+ // so its definition level still shows whether the struct is present
+ TypeWithSchemaVisitor.visit(
+ expectedSchema.asStruct(), fileSchema, new
PresenceColumnSelector(fileSchema, selectedIds));
return (MessageType)
TypeWithSchemaVisitor.visit(
expectedSchema.asStruct(), fileSchema, new
PruneColumns(selectedIds));
}
+ /**
+ * Adds one leaf id under each projected struct whose fields are all
constants and would otherwise
+ * retain no file leaf. That leaf's definition level is what still shows
whether the struct is
+ * present.
+ */
+ private static class PresenceColumnSelector extends
TypeWithSchemaVisitor<Void> {
+ private final MessageType fileSchema;
+ private final Set<Integer> selectedIds;
+
+ private PresenceColumnSelector(MessageType fileSchema, Set<Integer>
selectedIds) {
+ this.fileSchema = fileSchema;
+ this.selectedIds = selectedIds;
+ }
+
+ @Override
+ public Void struct(Types.StructType expected, GroupType struct, List<Void>
fields) {
+ // nothing projected under this struct, so there is nothing to track
+ if (expected == null || expected.fields().isEmpty()) {
+ return null;
+ }
+
+ String[] path = currentPath();
+ // add a presence column only if no real leaf under the struct is
already read
+ boolean readsRealLeaf =
+ leafColumns(fileSchema, path).stream()
+ .anyMatch(leaf -> selectedIds.contains(leafId(leaf)));
+ if (!readsRealLeaf) {
+ ColumnDescriptor presence = selectPresenceColumn(fileSchema, path);
+ if (presence != null) {
+ selectedIds.add(leafId(presence));
+ }
+ }
+
+ return null;
+ }
+
+ @Override
+ public Void variant(Types.VariantType expected, GroupType variantGroup,
Void result) {
+ return null;
+ }
+ }
+
+ /** Shallowest leaf under path; its definition level shows whether the
struct is present. */
+ static ColumnDescriptor selectPresenceColumn(MessageType fileSchema,
String[] path) {
+ if (fileSchema.getMaxDefinitionLevel(path) <= 0) {
+ return null;
+ }
+
+ return leafColumns(fileSchema, path).stream()
+ .min(Comparator.comparingInt(ColumnDescriptor::getMaxRepetitionLevel))
+ .orElse(null);
+ }
+
+ /** Returns the leaf columns with ids under the given path. */
+ static List<ColumnDescriptor> leafColumns(MessageType fileSchema, String[]
path) {
Review Comment:
private
--
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]
