sundapeng opened a new pull request, #9379:
URL: https://github.com/apache/paimon/pull/9379

   ### Purpose
   
   `Partition` declares the four statistics as primitive `long` in its 
`@JsonCreator`. Jackson fills a primitive with `0` when the property is absent 
or null, so a `listPartitions` response that omits them decodes to 
`recordCount=0` and `PartitionStatistics.isKnown` returns true. The class 
javadoc says the opposite:
   
   > a negative value (`UNKNOWN`) means nobody ever reported that field, and 
`0` means an exact zero. The two are not interchangeable: a consumer that 
treats unknown as zero plans against an empty partition that may hold a billion 
rows.
   
   Omitting the fields is legal. `docs/static/rest-catalog-open-api.yaml` has 
no `required` list on `Partition`, so a catalog that stores no statistics can 
answer with the spec alone. `RESTCatalogServer` already stores `UNKNOWN` for a 
partition that was only registered, so the server side follows the contract and 
only the client decode does not.
   
   Since #9351 this reaches Spark. `CatalogSplitEnumerator.rowCount` sums the 
record counts and gives up at the first unknown one. Zeros are known, so it 
returns `OptionalLong.of(0)`, `PaimonStatistics.numRows` becomes 0, and 
`sizeInBytes` becomes 0 too because the file-size fallback only runs when 
`numRows` is absent. Spark then plans against a 0 byte relation. 
`canBroadcastBySize` passes at any threshold, so a large format table can be 
chosen as the broadcast build side. Before #9351 the same scan fell back to the 
real split sizes, so this is a regression for catalogs that report nothing.
   
   The fix is a static `@JsonCreator` factory on `Partition` that takes the 
four statistics as `Long` and maps null to `UNKNOWN`. The public constructor 
keeps its signature, so the change is source and binary compatible.
   
   Two things stay as they are:
   
   - `totalBuckets` still defaults to `0`. Writers older than that field omit 
it, and `PartitionStatisticsTest.testLegacyPartitionStatisticsDeserialization` 
pins that behavior.
   - `PartitionStatistics` itself is not changed. On its own it is the commit 
delta in `CommitTableRequest` and `CreatePartitionsRequest`, where a negative 
value is a decrement and an absent field is not an unmeasured one.
   
   Paimon always writes all four fields when it serializes a partition, so JSON 
produced by Paimon round trips exactly as before. Spelling this out in the 
OpenAPI spec can be a follow-up.
   
   ### Tests
   
   - `PartitionTest`: absent statistics, null statistics, an explicit zero that 
must stay exact, and a round trip (7 tests)
   - `RESTApiJsonTest.listPartitionsResponseWithoutStatisticsParseTest`
   - 
`CatalogManagedPartitionScanTest.testPlanRowCountStaysUnknownWhenCatalogReportsNoStatistics`
 parses the partitions from statistics free JSON and asserts the plan row count 
stays empty
   - `mvn -pl paimon-api test`, 151 tests
   - `mvn -pl paimon-core 
-Dtest='org.apache.paimon.rest.**.*Test,org.apache.paimon.table.format.*Test,org.apache.paimon.manifest.*Test,org.apache.paimon.table.system.PartitionsTableTest,org.apache.paimon.utils.PartitionStatisticsReporterTest'`,
 688 tests
   - `CatalogManagedPartitionAnalyzeTest` on Spark 3.5, 21 tests
   - Spotless and checkstyle for paimon-api and paimon-core
   


-- 
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]

Reply via email to