JingsongLi commented on PR #10173: URL: https://github.com/apache/paimon/pull/10173#issuecomment-5831616783
Reviewed both commits through 2bf6010. The Python writer change has end-to-end value, and the standard-file manifest tests cover `none`, `counts`, `truncate(N)`, and `full`. Before production use, please address two write paths that still violate the requested table-level mode: **[P2] Row-id update files advertise value stats without writing them.** `_RowIdUpdateFileWriter.write_batches` still selects `fields` only when `metadata_stats_enabled()` is true (`full`). With `counts` or `truncate(N)`, it therefore builds `SimpleStats.empty_stats()`, while the changed `DataWriter._create_data_file_meta` sets `value_stats_cols=None` because `_value_stats_on` is true. I reproduced a row-id update under `counts`: its new file had `(value_stats_cols=None, len(null_counts)=0)` instead of the declared columns’ null counts. The update read still succeeded locally, but the manifest metadata contradicts the selected mode and gives downstream stats readers no counts. Convert this path too, and test a row-id update file under `counts` and `truncate(N)`. **[P2] Native writing bypasses the mode.** `create_native_write` excludes primary-key tables only when `metadata_stats_enabled()` is `full`; `counts`/`truncate(N)` remain eligible. The existing code notes that Rust omits PK value stats, so those modes still record none on that route. For append tables the PR itself notes Rust records full stats even under `counts`/`truncate(N)`, and the new E2E class is marked `python_write` to avoid this path. Please either apply the mode in Rust or select the Python writer whenever native output cannot honor it; add a native-route regression check. Validation: `metadata_stats_mode_test.py` 12 passed; adjacent data-evolution and pushdown tests 26 passed (23 subtests); `git diff --check` passed; CI run 36096816455 is green. The manifest shape is unchanged, but I did not find a cross-engine Java/Spark/Flink read of these new Python-written stats in this PR; that remains the release smoke check. -- 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]
