rdblue commented on code in PR #18147:
URL: https://github.com/apache/iceberg/pull/18147#discussion_r4031539959
##########
core/src/test/java/org/apache/iceberg/TestV4ManifestReader.java:
##########
@@ -732,6 +739,274 @@ public void statsFilterMissingColumnFailure() {
.hasMessageContaining("Cannot find field 'missing' in struct: %s",
TABLE_SCHEMA.asStruct());
}
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void statsFilterRecordCountFiltering(FileFormat format) throws
IOException {
+ TrackedFile emptyTrackedFile =
+ new TrackedFileStruct(
+ ADDED_TRACKING,
+ FileContent.DATA,
+ FORMAT_VERSION_V4,
+ "s3://bucket/table/empty-file.parquet",
+ FileFormat.PARQUET,
+ 0, // file contains no records
+ 100L,
+ null,
+ null,
+ null,
+ SortOrder.unsorted().orderId(),
+ null,
+ null,
+ null,
+ List.of(4L),
+ null);
+
+ ManifestFile manifest =
+ writeManifest(format, UNPARTITIONED_TYPE,
ImmutableList.of(emptyTrackedFile, FILE_D));
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
UNPARTITIONED_SPECS)
+ .metricsConfig(METRICS_CONFIG);
+
+ List<TrackedFile> actualFiles = read(builder);
+
+ assertThat(actualFiles)
+ .usingComparatorForType(FILE_COMPARATOR, TrackedFile.class)
+ .containsExactly(FILE_D);
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void statsFilterInvalidRecordCountNotFiltered(FileFormat format)
throws IOException {
+ // A bug in old writers produced Avro files with record_count=-1 (unknown)
+ TrackedFile invalidRecordCountFile =
+ new TrackedFileStruct(
+ ADDED_TRACKING,
+ FileContent.DATA,
+ FORMAT_VERSION_V4,
+ "s3://bucket/table/very-old.avro",
+ FileFormat.AVRO,
+ -1, // mimic invalid record count in old Avro metadata
+ 100L,
+ null,
+ null,
+ null,
+ SortOrder.unsorted().orderId(),
+ null,
+ null,
+ null,
+ List.of(4L),
+ null);
+
+ ManifestFile manifest =
+ writeManifest(format, UNPARTITIONED_TYPE,
ImmutableList.of(invalidRecordCountFile, FILE_D));
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
UNPARTITIONED_SPECS)
+ .metricsConfig(METRICS_CONFIG);
+
+ List<TrackedFile> actualFiles = read(builder);
+
+ assertThat(actualFiles)
+ .usingComparatorForType(FILE_COMPARATOR, TrackedFile.class)
+ .containsExactly(invalidRecordCountFile, FILE_D);
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void statsFilterDataFileBoundsFiltering(FileFormat format) throws
IOException {
+ // FILE_C has stats {id in [0, 99], data in [a, z]}, FILE_D has no stats
+ ManifestFile manifest =
+ writeManifest(format, UNPARTITIONED_TYPE, ImmutableList.of(FILE_C,
FILE_D));
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
UNPARTITIONED_SPECS)
+ .filter(Expressions.equal("id", 105)) // eliminates FILE_C
+ .metricsConfig(METRICS_CONFIG);
+
+ List<TrackedFile> actualFiles = read(builder);
+
+ assertThat(actualFiles)
+ .usingComparatorForType(FILE_COMPARATOR, TrackedFile.class)
+ .containsExactly(FILE_D);
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void statsFilterManifestBoundsFiltering(FileFormat format) throws
IOException {
+ // DATA_MANIFEST_WITH_STATS_REF has stats {id in [0, 99], data in [a, z]}
+ ManifestFile manifest =
+ writeManifest(
+ format,
+ UNPARTITIONED_TYPE,
+ ImmutableList.of(DATA_MANIFEST_REF, DATA_MANIFEST_WITH_STATS_REF));
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
UNPARTITIONED_SPECS)
+ .filter(Expressions.equal("id", 105)) // eliminates the manifest
with stats
+ .metricsConfig(METRICS_CONFIG);
+
+ List<TrackedFile> actualFiles = read(builder);
+
+ assertThat(actualFiles)
+ .usingComparatorForType(FILE_COMPARATOR, TrackedFile.class)
+ .containsExactly(DATA_MANIFEST_REF);
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void statsFilterBoundsFilteringWithForScanPlanning(FileFormat format)
throws IOException {
+ // FILE_C has stats {id in [0, 99], data in [a, z]}, FILE_D has no stats
+ ManifestFile manifest =
+ writeManifest(format, UNPARTITIONED_TYPE, ImmutableList.of(FILE_C,
FILE_D));
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
UNPARTITIONED_SPECS)
+ .forScanPlanning() // does not project unused stats
+ .filter(Expressions.equal("id", 105)) // eliminates FILE_C
+ .metricsConfig(METRICS_CONFIG);
+
+ List<TrackedFile> actualFiles = read(builder);
+
+ assertThat(actualFiles)
+ .usingComparatorForType(FILE_COMPARATOR, TrackedFile.class)
+ .containsExactly(FILE_D);
+ }
+
+ @ParameterizedTest
+ @FieldSource("PROJECTION_CASES")
+ public void
statsFilterBoundsFilteringWithProjection(Consumer<V4ManifestReader.Builder>
config)
+ throws IOException {
+ // FILE_C has stats {id in [0, 99], data in [a, z]}, FILE_D has no stats
+ // config projects just the file location, but stats are automatically
projected for the filter
+ ManifestFile manifest =
+ writeManifest(FileFormat.PARQUET, UNPARTITIONED_TYPE,
ImmutableList.of(FILE_C, FILE_D));
+
+ V4ManifestReader.Builder builder =
+ V4ManifestReader.builder(manifest, IO, TABLE_SCHEMA,
UNPARTITIONED_SPECS)
+ .filter(Expressions.equal("id", 105)) // eliminates FILE_C
+ .metricsConfig(METRICS_CONFIG);
+
+ config.accept(builder);
+
+ List<TrackedFile> actualFiles = read(builder);
+
+
assertThat(actualFiles).extracting(TrackedFile::location).containsExactly(FILE_D.location());
+ }
+
+ @ParameterizedTest
+ @FieldSource("MANIFEST_FORMATS")
+ public void statsFilterCaseSensitivity(FileFormat format) throws IOException
{
Review Comment:
This tests both case sensitive and case insensitive. First it verifies that
a filter that requires case sensitivity fails and then it turns on case
sensitivity and verifies the filter works.
I think the confusion is that when case doesn't match, the result is
`ValidationException` because the filter cannot be bound to the schema, rather
than just ignoring 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: [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]
