rdblue commented on a change in pull request #3273:
URL: https://github.com/apache/iceberg/pull/3273#discussion_r729956517
##########
File path: core/src/main/java/org/apache/iceberg/DataFiles.java
##########
@@ -285,7 +285,12 @@ public DataFile build() {
}
Preconditions.checkArgument(format != null, "File format is required");
Preconditions.checkArgument(fileSizeInBytes >= 0, "File size is
required");
- Preconditions.checkArgument(recordCount >= 0, "Record count is
required");
+ Preconditions.checkArgument(recordCount != null, "Record count is
required");
+ // MetricsEvaluator skips using other metrics, if record count is -1
+ Preconditions.checkArgument(recordCount >= 0 ||
+ (recordCount == -1 && valueCounts == null && columnSizes == null
&& nanValueCounts == null &&
+ lowerBounds == null && upperBounds == null),
+ "Metrics cannot be set if record count is -1.");
Review comment:
Why is this change necessary? I don't think that we should allow writing
a file without a record count.
##########
File path: core/src/main/java/org/apache/iceberg/DataFiles.java
##########
@@ -285,7 +285,12 @@ public DataFile build() {
}
Preconditions.checkArgument(format != null, "File format is required");
Preconditions.checkArgument(fileSizeInBytes >= 0, "File size is
required");
- Preconditions.checkArgument(recordCount >= 0, "Record count is
required");
+ Preconditions.checkArgument(recordCount != null, "Record count is
required");
+ // MetricsEvaluator skips using other metrics, if record count is -1
+ Preconditions.checkArgument(recordCount >= 0 ||
+ (recordCount == -1 && valueCounts == null && columnSizes == null
&& nanValueCounts == null &&
+ lowerBounds == null && upperBounds == null),
+ "Metrics cannot be set if record count is -1.");
Review comment:
Got it. What about getting the correct count in the import code? We
already open up Parquet files and read the footer. It wouldn't be too bad to
skip through Avro files, like we do to find the count for a specific starting
offset:
https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/avro/AvroIO.java#L149
--
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]
