From 73f75b0e715729a6d7e8685a3ac5d2e8f5c6a499 Mon Sep 17 00:00:00 2001 From: Sun Dapeng Date: Sun, 9 Aug 2026 01:47:34 +0800 Subject: [PATCH] [core] Spell out what a negative partition statistic means MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PartitionStatistics said only that its fields "may be negative, indicating that some data has been removed". That covers one of the two planes the class is read on, and consumers have been getting the other one wrong. On the delta plane — what a commit changed — a negative value is a decrement the server adds to what it holds. That is the existing meaning and nothing here changes it. On the observation plane — what listPartitions returns for a partition as it stands — a negative value means nobody ever reported that field, and 0 means an exact zero. Conflating them is not cosmetic: a consumer that reads unknown as zero plans against an empty partition that may hold a billion rows, and one that does arithmetic on it gets a number that is wrong rather than missing. So the plane is named in the javadoc, unknown gets a name (UNKNOWN, with isKnown() to test it rather than each caller comparing against -1), and unknown is documented as per field: a reporter that only knows the file count leaves the record count unknown and fills the rest. The fields stay primitive. Boxing them to express unknown as null would be a breaking change to a @Public class, and the encoding above needs no new type. FileSystemSplitEnumerator now says PartitionStatistics.UNKNOWN where it said -1. Discovering partitions by listing directories measures nothing about what is inside them, which is what unknown already meant there; this is the same value under its own name. --- .../paimon/partition/PartitionStatistics.java | 42 ++++++++++++++++++- .../partition/PartitionStatisticsTest.java | 32 ++++++++++++++ .../format/FileSystemSplitEnumerator.java | 12 +++++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/paimon-api/src/main/java/org/apache/paimon/partition/PartitionStatistics.java b/paimon-api/src/main/java/org/apache/paimon/partition/PartitionStatistics.java index ab87f02ed6a1..6717bbc258ea 100644 --- a/paimon-api/src/main/java/org/apache/paimon/partition/PartitionStatistics.java +++ b/paimon-api/src/main/java/org/apache/paimon/partition/PartitionStatistics.java @@ -30,8 +30,23 @@ import java.util.Objects; /** - * Statistics of a partition, fields inside may be negative, indicating that some data has been - * removed. + * Statistics of a partition. + * + *

The numeric fields are read on two planes, and a negative value means a different thing on + * each. Which plane an instance belongs to follows from where it came from, never from the value: + * + *

+ * + *

Unknown is per field, not per partition: a reporter that only knows the file count leaves the + * record count {@link #UNKNOWN} and fills the rest. Use {@link #isKnown(long)} rather than + * comparing against {@code -1}; any negative value on the observation plane is unknown. */ @JsonIgnoreProperties(ignoreUnknown = true) @Public @@ -39,6 +54,15 @@ public class PartitionStatistics implements Serializable { private static final long serialVersionUID = 1L; + /** + * Canonical encoding of "this field was never reported" on the observation plane. Any negative + * value carries the same meaning; this is the one to write. + */ + public static final long UNKNOWN = -1L; + + /** Format tables have no buckets, so their bucket count is always unknown. */ + public static final int UNKNOWN_TOTAL_BUCKETS = -1; + public static final String FIELD_SPEC = "spec"; public static final String FIELD_RECORD_COUNT = "recordCount"; public static final String FIELD_FILE_SIZE_IN_BYTES = "fileSizeInBytes"; @@ -82,6 +106,20 @@ public PartitionStatistics( this.totalBuckets = totalBuckets; } + /** Statistics of a partition nobody ever reported on: every field {@link #UNKNOWN}. */ + public static PartitionStatistics unknown(Map spec) { + return new PartitionStatistics( + spec, UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN_TOTAL_BUCKETS); + } + + /** + * Whether an observation-plane field carries a real measurement. Never apply this to a + * delta-plane value, where a negative number is a decrement rather than a missing measurement. + */ + public static boolean isKnown(long value) { + return value >= 0; + } + @JsonGetter(FIELD_SPEC) public Map spec() { return spec; diff --git a/paimon-api/src/test/java/org/apache/paimon/partition/PartitionStatisticsTest.java b/paimon-api/src/test/java/org/apache/paimon/partition/PartitionStatisticsTest.java index d9fd8e8bb162..ec7f12e933a6 100644 --- a/paimon-api/src/test/java/org/apache/paimon/partition/PartitionStatisticsTest.java +++ b/paimon-api/src/test/java/org/apache/paimon/partition/PartitionStatisticsTest.java @@ -22,6 +22,9 @@ import org.junit.jupiter.api.Test; +import java.util.Collections; +import java.util.Map; + import static org.assertj.core.api.Assertions.assertThat; /** Test for {@link PartitionStatistics}. */ @@ -41,4 +44,33 @@ void testLegacyPartitionStatisticsDeserialization() { assertThat(stats.lastFileCreationTime()).isEqualTo(123456789L); assertThat(stats.totalBuckets()).isEqualTo(0); } + + @Test + void testZeroIsAKnownMeasurement() { + // The boundary the whole observation-plane contract rests on: an empty partition was + // measured, and a consumer that reads its zero as "nobody looked" plans against the wrong + // table. + assertThat(PartitionStatistics.isKnown(0L)).isTrue(); + assertThat(PartitionStatistics.isKnown(1L)).isTrue(); + assertThat(PartitionStatistics.isKnown(Long.MAX_VALUE)).isTrue(); + + assertThat(PartitionStatistics.isKnown(PartitionStatistics.UNKNOWN)).isFalse(); + // Unknown is any negative value, not only the canonical -1. + assertThat(PartitionStatistics.isKnown(-2L)).isFalse(); + assertThat(PartitionStatistics.isKnown(Long.MIN_VALUE)).isFalse(); + } + + @Test + void testUnknownLeavesEveryFieldUnknown() { + Map spec = Collections.singletonMap("pt", "1"); + + PartitionStatistics stats = PartitionStatistics.unknown(spec); + + assertThat(stats.spec()).isEqualTo(spec); + assertThat(PartitionStatistics.isKnown(stats.recordCount())).isFalse(); + assertThat(PartitionStatistics.isKnown(stats.fileSizeInBytes())).isFalse(); + assertThat(PartitionStatistics.isKnown(stats.fileCount())).isFalse(); + assertThat(PartitionStatistics.isKnown(stats.lastFileCreationTime())).isFalse(); + assertThat(PartitionStatistics.isKnown(stats.totalBuckets())).isFalse(); + } } diff --git a/paimon-core/src/main/java/org/apache/paimon/table/format/FileSystemSplitEnumerator.java b/paimon-core/src/main/java/org/apache/paimon/table/format/FileSystemSplitEnumerator.java index 7373e35d4d4c..7d4bccfd15f8 100644 --- a/paimon-core/src/main/java/org/apache/paimon/table/format/FileSystemSplitEnumerator.java +++ b/paimon-core/src/main/java/org/apache/paimon/table/format/FileSystemSplitEnumerator.java @@ -25,6 +25,7 @@ import org.apache.paimon.manifest.PartitionEntry; import org.apache.paimon.partition.PartitionPredicate; import org.apache.paimon.partition.PartitionPredicate.MultiplePartitionPredicate; +import org.apache.paimon.partition.PartitionStatistics; import org.apache.paimon.predicate.Predicate; import org.apache.paimon.table.FormatTable; import org.apache.paimon.table.source.Split; @@ -125,7 +126,16 @@ List listPartitionEntries() { List partitionEntries = new ArrayList<>(); for (Pair, Path> partition2Path : partition2Paths) { BinaryRow row = toPartitionRow(partition2Path.getKey()); - partitionEntries.add(new PartitionEntry(row, -1L, -1L, -1L, -1L, -1)); + // Discovering partitions from directories measures nothing about what is inside them, + // so every statistic is unknown rather than zero. + partitionEntries.add( + new PartitionEntry( + row, + PartitionStatistics.UNKNOWN, + PartitionStatistics.UNKNOWN, + PartitionStatistics.UNKNOWN, + PartitionStatistics.UNKNOWN, + PartitionStatistics.UNKNOWN_TOTAL_BUCKETS)); } return partitionEntries; }