leaves12138 commented on code in PR #938:
URL: https://github.com/apache/paimon-rust/pull/938#discussion_r4093074109
##########
crates/paimon/src/table/kv_file_writer.rs:
##########
@@ -433,16 +437,35 @@ impl KeyValueFileWriter {
self.file_io.mkdirs(&format!("{bucket_dir}/")).await?;
let file_path = format!("{bucket_dir}/{file_name}");
let output = self.file_io.new_output(&file_path)?;
- let writer = create_format_writer(
- &output,
- physical_schema.clone(),
- write.file_compression,
- self.config.file_compression_zstd_level,
- None,
- None,
- None,
- )
- .await?;
+ // The physical KV file also contains sequence and row-kind columns.
Give
+ // Parquet only the logical value fields so metadata stats and their
dense
+ // column mapping follow Java's value schema (and its stats options).
+ // Keep the existing unshredded KV layout for this writer.
+ let writer: Box<dyn FormatFileWriter> = if
write.file_format.eq_ignore_ascii_case("parquet")
+ {
+ Box::new(
+ ParquetFormatWriter::new(
+ &output,
+ physical_schema.clone(),
+ write.file_compression,
+ self.config.file_compression_zstd_level,
+ Some(&self.config.value_fields),
Review Comment:
[P1] Do not publish nested leaf null counts as parent value-column stats
Passing all logical value fields to this extractor exposes an unsafe mapping
in `build_row_group_column_indices`: a single Parquet leaf such as
`payload.child` is matched to its root `payload`, and its leaf null count is
then serialized as the parent's null count.
I reproduced this through the public writer with a primary-key Parquet table
`(id INT PRIMARY KEY, payload ROW<child INT>)`,
`deletion-vectors.enabled=true`, and two rows whose payloads are non-null
structs containing a null child. With `metadata.stats-mode=full`, this PR
writes `value_stats.null_counts = [Some(0), Some(2)]`, although the correct
parent null count is zero (or it should be unknown). An unfiltered
`new_scan().with_scan_all_files().plan()` keeps the single file, but adding
`payload IS NOT NULL` yields zero splits. The same regression test passes on
base `e16b01c50daf78e58a6555d248d677da122c6e9d`; disabling stats also preserves
the file.
The extractor predates this PR, but this new call makes PK files persist
these invalid value stats instead of having no value stats. Please leave
nested/repeated root-column stats unknown unless their parent-level statistics
are actually available; a child leaf's null count is not a parent's null count.
Add a regression covering non-null structs with null children, and guard
repeated-column stats similarly.
--
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]
