You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "gaborkaszab (via GitHub)" <gi...@apache.org> on 2026/05/27 14:48:58 UTC
[PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
gaborkaszab opened a new pull request, #16582:
URL: https://github.com/apache/iceberg/pull/16582
Also implement caseSensitive(), that we can use when binding the unbound field name to the schema.
Used Claude Opus 4.7 to generate the tests, that I review, cleaned up, and adjusted to my taste.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3333432225
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
+
+ requiredFieldIds.addAll(TypeUtil.getProjectedIds(projection));
Review Comment:
Do we need to handle case sensitivity here?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3333487201
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
Review Comment:
Well, either could be fine: requested IDs through projection, plus the required one to evaluate the filter.
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
Review Comment:
Well, either could be fine: requested IDs through projection, plus the required ones to evaluate the filter.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "kamcheungting-db (via GitHub)" <gi...@apache.org>.
kamcheungting-db commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3312434614
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -71,16 +91,46 @@ public CloseableIterable<PartitionStatistics> scan() {
return CloseableIterable.empty();
}
+ // Always read with the latest schema
Types.StructType partitionType = Partitioning.partitionType(table);
- Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
+ Schema schema = PartitionStatistics.latestSchema(partitionType);
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(schema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
Review Comment:
could we add a filter method into ReadBuilder?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3334719061
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
Review Comment:
Thx
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "kamcheungting-db (via GitHub)" <gi...@apache.org>.
kamcheungting-db commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3312455070
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -71,16 +91,46 @@ public CloseableIterable<PartitionStatistics> scan() {
return CloseableIterable.empty();
}
+ // Always read with the latest schema
Types.StructType partitionType = Partitioning.partitionType(table);
- Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
+ Schema schema = PartitionStatistics.latestSchema(partitionType);
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(schema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
Review Comment:
This is no a blocker for this PR but to check any concern for introduce filter on ReadBuilder
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3324455055
##########
api/src/main/java/org/apache/iceberg/PartitionStatistics.java:
##########
@@ -84,7 +92,11 @@ private static Schema v2Schema(Types.StructType unifiedPartitionType) {
LAST_UPDATED_SNAPSHOT_ID);
}
- private static Schema v3Schema(Types.StructType unifiedPartitionType) {
+ private static Schema v3WriteSchema(Types.StructType unifiedPartitionType) {
+ return latestSchema(unifiedPartitionType);
+ }
+
+ static Schema latestSchema(Types.StructType unifiedPartitionType) {
Review Comment:
FYI, this change was for the dependency project() PR. That got merged, but we ended up not introducing the `latestSchema` function.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "kamcheungting-db (via GitHub)" <gi...@apache.org>.
kamcheungting-db commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3312467249
##########
api/src/main/java/org/apache/iceberg/PartitionStatistics.java:
##########
@@ -84,7 +92,11 @@ private static Schema v2Schema(Types.StructType unifiedPartitionType) {
LAST_UPDATED_SNAPSHOT_ID);
}
- private static Schema v3Schema(Types.StructType unifiedPartitionType) {
+ private static Schema v3WriteSchema(Types.StructType unifiedPartitionType) {
+ return latestSchema(unifiedPartitionType);
+ }
+
+ static Schema latestSchema(Types.StructType unifiedPartitionType) {
Review Comment:
add javac doc: `returns the schema for the highest supported format version`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3333414658
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
Review Comment:
is it `requestedFieldIds`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "kamcheungting-db (via GitHub)" <gi...@apache.org>.
kamcheungting-db commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3312443944
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -71,16 +91,46 @@ public CloseableIterable<PartitionStatistics> scan() {
return CloseableIterable.empty();
}
+ // Always read with the latest schema
Types.StructType partitionType = Partitioning.partitionType(table);
- Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
+ Schema schema = PartitionStatistics.latestSchema(partitionType);
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(schema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
Review Comment:
same comment above, if we support filter on the read builder, we can reduce the unnecessary cost of projecting the columns inside filters.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3324437964
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -71,16 +91,46 @@ public CloseableIterable<PartitionStatistics> scan() {
return CloseableIterable.empty();
}
+ // Always read with the latest schema
Types.StructType partitionType = Partitioning.partitionType(table);
- Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
+ Schema schema = PartitionStatistics.latestSchema(partitionType);
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(schema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
Review Comment:
Thanks for bringing this up, @kamcheungting-db!
I explored this as well, apparently not all of the file formats have a filter method. But even if they had, I observed that Parquet uses the filter only to skip row groups but it won't filter the rows after row group filtering. For stats files row group filtering doesn't seem very useful, probable all the stats fit into the same row group with the current content.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#issuecomment-4648503729
Merged to main.
Thanks @gaborkaszab for the PR and @kamcheungting-db for the review!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #16582:
URL: https://github.com/apache/iceberg/pull/16582
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3324450204
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -71,16 +91,46 @@ public CloseableIterable<PartitionStatistics> scan() {
return CloseableIterable.empty();
}
+ // Always read with the latest schema
Types.StructType partitionType = Partitioning.partitionType(table);
- Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
+ Schema schema = PartitionStatistics.latestSchema(partitionType);
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(schema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
Review Comment:
See my answer above.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#issuecomment-4576303689
cc @pvary @ajantha-bhat
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3333401425
##########
api/src/main/java/org/apache/iceberg/PartitionStatisticsScan.java:
##########
@@ -42,6 +42,17 @@ public interface PartitionStatisticsScan {
*/
PartitionStatisticsScan filter(Expression filter);
+ /**
+ * Create a new scan from this that will configure whether column name matches in filter
+ * expressions are case-sensitive.
Review Comment:
Do we want to mention the default value in the javadoc?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3333468984
##########
api/src/main/java/org/apache/iceberg/PartitionStatisticsScan.java:
##########
@@ -42,6 +42,17 @@ public interface PartitionStatisticsScan {
*/
PartitionStatisticsScan filter(Expression filter);
+ /**
+ * Create a new scan from this that will configure whether column name matches in filter
+ * expressions are case-sensitive.
Review Comment:
Done
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
Review Comment:
Well, either could be fine: requested IDs through projection, pus the required one to evaluate the filter.
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
+
+ requiredFieldIds.addAll(TypeUtil.getProjectedIds(projection));
Review Comment:
No, because projection is done via field IDs not field names. The input for that is a Schema of fields, where we use the field IDs.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#discussion_r3333935573
##########
core/src/main/java/org/apache/iceberg/BasePartitionStatisticsScan.java:
##########
@@ -77,16 +93,43 @@ public CloseableIterable<PartitionStatistics> scan() {
Types.StructType partitionType = Partitioning.partitionType(table);
Schema schema = PartitionStatistics.schema(partitionType, TableUtil.formatVersion(table));
- Schema readSchema =
- projection == null ? schema : TypeUtil.select(schema, TypeUtil.getProjectedIds(projection));
+ Schema readSchema = readSchema(schema);
FileFormat fileFormat = FileFormat.fromFileName(statsFile.get().path());
Preconditions.checkNotNull(
fileFormat != null, "Unable to determine format of file: %s", statsFile.get().path());
- return InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
- .project(readSchema)
- .setRootType(BasePartitionStatistics.class)
- .build();
+ CloseableIterable<PartitionStatistics> result =
+ InternalData.read(fileFormat, table.io().newInputFile(statsFile.get().path()))
+ .project(readSchema)
+ .setRootType(BasePartitionStatistics.class)
+ .build();
+
+ if (filter != Expressions.alwaysTrue()) {
+ Evaluator evaluator = new Evaluator(readSchema.asStruct(), filter, caseSensitive);
+ result = CloseableIterable.filter(result, evaluator::eval);
+ }
+
+ return result;
+ }
+
+ /**
+ * Resolves the schema to read. When a projection is set, all columns referenced by the filter are
+ * added to the result not just the projected fields.
+ */
+ private Schema readSchema(Schema schema) {
+ if (projection == null) {
+ return schema;
+ }
+
+ Set<Integer> requiredFieldIds = Sets.newHashSet();
Review Comment:
so it is `fieldIds` or `fieldIdsToRead`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core, Orc: Implement filter() for partition statistics scan API [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on PR #16582:
URL: https://github.com/apache/iceberg/pull/16582#issuecomment-4612332326
@ajantha-bhat maybe you'd like to take a look
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org