Skip to content

feat: write value stats for append data files#477

Open
QuakeWang wants to merge 7 commits into
apache:mainfrom
QuakeWang:value-stats
Open

feat: write value stats for append data files#477
QuakeWang wants to merge 7 commits into
apache:mainfrom
QuakeWang:value-stats

Conversation

@QuakeWang

Copy link
Copy Markdown
Member

Purpose

Rust append writes currently emit empty _VALUE_STATS, so manifest-level stats pruning cannot skip data files written by Rust even when min/max/null-count information is available.

Brief change log

  • Accumulate min/max/null-count value stats across batches in DataFileWriter.
  • Persist stats in existing DataFileMeta.value_stats and dense value_stats_cols.
  • Skip unsupported variable-length value stats and add nanosecond timestamp extraction support.
  • Add tests for value stats writing, timestamp(9), skipped variable-length columns, and scan pruning.

Tests

  • cargo fmt --all -- --check
  • cargo clippy --all-targets --workspace --features fulltext,vortex,mosaic -- -D warnings
  • cargo test -p paimon --all-targets --features fulltext,vortex,mosaic

API and Format

Documentation

QuakeWang added 2 commits July 7, 2026 21:18
Append data files previously wrote empty value_stats, so manifest-level stats pruning could not skip files produced by Rust append writes.

Accumulate min/max/null counts for supported fixed-width value columns across batches, persist dense value_stats_cols, and handle nanosecond timestamp stats.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Java Side, the performance of the accumulator is poor, and the only truly useful link is the one that extracts stats from Parquet. Perhaps we should optimize its implementation instead of the accumulator.

QuakeWang added 2 commits July 8, 2026 20:45
Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
@QuakeWang

Copy link
Copy Markdown
Member Author

In Java Side, the performance of the accumulator is poor, and the only truly useful link is the one that extracts stats from Parquet. Perhaps we should optimize its implementation instead of the accumulator.

@JingsongLi Thanks, addressed.

I changed the implementation to avoid per-row stats accumulation. Append value stats are now extracted from Parquet writer metadata on close, aligned with the Java ParquetSimpleStatsExtractor path.

Added/updated tests for append value stats, stats pruning, and timestamp stats.

parquet_metadata: Option<&parquet::file::metadata::ParquetMetaData>,
) -> DataFileMeta {
let (value_stats, value_stats_cols) =
crate::arrow::format::parquet::extract_value_stats(parquet_metadata, write_fields);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just return stats from parquet? It is not good to expose ParquetMetaData in core.

.collect()
}

pub(crate) fn extract_value_stats(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to support default stats mode: truncate(16), otherwise the stats are too big.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
@QuakeWang

Copy link
Copy Markdown
Member Author

@JingsongLi Addressed, thanks. Conflicts are resolved, ParquetMetaData is no longer exposed to core, and string value stats now use default truncate(16) with test coverage.

@JingsongLi

Copy link
Copy Markdown
Contributor

Thanks for the update. I think there are still a couple of Java compatibility gaps here.

  1. The manifest stats mode is currently hard-coded to the default truncate(16) behavior. In Java, stats collectors are created from metadata.stats-mode, fields.<name>.stats-mode, and metadata.stats-keep-first-n-columns. With this implementation, a table configured with metadata.stats-mode = none/counts/full or a field-level override would still get truncate(16)-style stats in Rust-written manifests. Could we thread the table stats options into this extraction path and apply the same collector semantics as Java?

  2. supports_manifest_value_stats currently skips unsupported min/max types completely. Java does not do that: for unsupported Parquet min/max types such as BINARY/VARBINARY, ParquetSimpleStatsExtractor still returns SimpleColStats(null, null, nullCount), and dense stats storage keeps the column when the null count is known. The current Rust behavior drops those columns from value_stats_cols, so their null counts are lost. Also, Java supports TIME_WITHOUT_TIME_ZONE through IntStatistics, while Rust's conversion code has Time support but the support list excludes DataType::Time. Could we keep null counts for unsupported min/max columns and include Time to match Java?

QuakeWang added 2 commits July 9, 2026 19:03
Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants