gaborkaszab commented on code in PR #17000:
URL: https://github.com/apache/iceberg/pull/17000#discussion_r3506176570
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -49,10 +49,9 @@ class TestTrackedFileStruct {
private static final int FILE_SIZE_IN_BYTES_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.FILE_SIZE_IN_BYTES);
private static final int SPEC_ID_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.SPEC_ID);
- // partition and content_stats are rebuilt with the supplied struct types
inside
- // schemaWithContentStats, so their ordinals are looked up by field ID.
+ // partition is rebuilt with the supplied struct types inside
schemaWithContentStats, so its
+ // ordinal is looked up by field ID.
private static final int PARTITION_ORDINAL =
ordinalOf(TrackedFile.PARTITION_ID);
- private static final int CONTENT_STATS_ORDINAL =
ordinalOf(TrackedFile.CONTENT_STATS_ID);
Review Comment:
Found an overkill to open a new PR to remove an unused variable from the
tests and rewrite/remove the relevant comment.
Reverted these now, opened a separate PR:
https://github.com/apache/iceberg/pull/17031
##########
core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java:
##########
@@ -100,7 +100,7 @@ public int specId() {
@Override
public StructLike partition() {
- return file().partition();
+ return file().partition() != null ? file().partition() :
BaseFile.EMPTY_PARTITION_DATA;
Review Comment:
I introduced PartitionData.EMPTY for this
##########
core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java:
##########
@@ -49,10 +49,9 @@ class TestTrackedFileStruct {
private static final int FILE_SIZE_IN_BYTES_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.FILE_SIZE_IN_BYTES);
private static final int SPEC_ID_ORDINAL =
SCHEMA_FIELDS.indexOf(TrackedFile.SPEC_ID);
- // partition and content_stats are rebuilt with the supplied struct types
inside
- // schemaWithContentStats, so their ordinals are looked up by field ID.
+ // partition is rebuilt with the supplied struct types inside
schemaWithContentStats, so its
+ // ordinal is looked up by field ID.
Review Comment:
seem my comment below
--
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]
