stevenzwu commented on code in PR #18310:
URL: https://github.com/apache/iceberg/pull/18310#discussion_r4208058072
##########
core/src/main/java/org/apache/iceberg/FieldStats.java:
##########
@@ -61,11 +61,35 @@ public interface FieldStats<T> {
/** The total NaN value count, defined only when {@link
#hasNanValueCount()}. */
long nanValueCount();
+ /** Whether total bytes are tracked for this field. */
+ default boolean hasTotalBytes() {
+ return false;
+ }
+
+ /**
+ * The total uncompressed size in memory in bytes of non-null values, for
variable-length types
+ * (string, binary, variant, geometry, geography).
+ */
+ default long totalBytes() {
+ return 0L;
+ }
+
/**
* The avg value size in memory (uncompressed) in bytes for variable-length
types (string, binary,
* variant, geometry, geography)
+ *
+ * @deprecated since 1.13.0, will be removed in 1.14.0; use {@link
#totalBytes()} instead.
*/
- Integer avgValueSizeInBytes();
+ @Deprecated
Review Comment:
we may not need to deprecate this API because it is just a cheap derived
value computation. we can provide it as a convenient method if this is useful
information to caller.
I guess one down side of keeping it in the signature. if we maintain
consistency as other numeric types, this should return a primitive `int` and we
need another `hasAvgValueSizeInBytes` API.
##########
api/src/main/java/org/apache/iceberg/Metrics.java:
##########
@@ -244,14 +244,50 @@ public Map<Integer, ByteBuffer> upperBounds() {
return upperBounds;
}
+ /**
+ * Get the total uncompressed size in memory in bytes of non-null values,
for all fields where it
+ * was collected.
+ *
+ * @return a Map of fieldId to total uncompressed size in bytes
+ */
+ public Map<Integer, Long> totalBytes() {
+ return totalBytes;
+ }
+
/**
* Get the average value size in memory (uncompressed) in bytes over
non-null values, for all
* fields where it was collected.
*
* @return a Map of fieldId to average value size in bytes
+ * @deprecated since 1.13.0, will be removed in 2.0.0; use {@link
#totalBytes()} instead.
*/
+ @Deprecated
public Map<Integer, Integer> avgValueSizes() {
- return avgValueSizes;
+ return avgValueSizes(totalBytes, valueCounts, nullValueCounts);
Review Comment:
this is the API that we can probably drop without maintaining compatibility
because it requires relatively expensive computation.
--
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]
