refactor: Add format-agnostic StatisticsBuilder to dwio/common by mbasmanova · Pull Request #16693 · facebookincubator/velox

mbasmanova · 2026-03-10T01:18:26Z

Summary:
The DWRF writer's StatisticsBuilder contains merge/addValues logic that is
format-agnostic, but lives in the DWRF writer library, creating an undesirable
cross-format dependency for consumers like Axiom and Parquet.

This change extracts the format-agnostic parts into new dwio::common classes:

StatisticsBuilder base class with merge(), reset(), create(), createTree()
Typed builders: Boolean, Integer, Double, String, Binary
build() method that produces read-only ColumnStatistics snapshots

The DWRF builders now extend the common base, adding only toProto() and
proto-based build() for DWRF file format serialization.

Axiom's StatisticsBuilderImpl now wraps dwio::common::StatisticsBuilder,
removing the DWRF writer dependency.

Differential Revision: D95885404

netlify · 2026-03-10T01:18:31Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`443612b`
🔍 Latest deploy log	https://app.netlify.com/projects/meta-velox/deploys/69b023feb7bb2e0008247a17

meta-codesync · 2026-03-10T01:18:33Z

@mbasmanova has exported this pull request. If you are a Meta employee, you can view the originating Diff in D95885404.

PingLiuPing

I had a look at parquet writer statistic, and it has its own completely self-contained statistics system that is quite different from dwio::common::StatisticsBuilder.
For example:
Update API: one value at a time VS bulk Arrow array
Comparison: direct VS customised typed comparator
Output: ColumnStatistics snapshot VS EncodedStatistics (min/max serialized as byte strings)
Parquet also has page level statistics.

mbasmanova · 2026-03-10T11:43:07Z

@PingLiuPing Thank you for looking. Is it the case that unlike DWRF, Parquet stores stats only per row group. It doesn't aggregate these and doesn't store file-level stats. What's your suggestion?

PingLiuPing · 2026-03-10T11:39:00Z

velox/dwio/common/StatisticsBuilder.cpp

+namespace {
+
+template <typename T>
+void addWithOverflowCheck(std::optional<T>& to, T value, uint64_t count) {


Seems addWithOverflowCheck, mergeWithOverflowCheck, mergeCount, mergeMin, mergeMax, isValidLength are copy pasted identically in both dwio/common/StatisticsBuilder.cpp and dwio/dwrf/writer/StatisticsBuilder.cpp.

PingLiuPing · 2026-03-10T12:02:30Z

velox/dwio/common/StatisticsBuilder.cpp

+  return result;
+}
+
+void IntegerStatisticsBuilder::addValues(int64_t value, uint64_t count) {


addValues, merge, reset, and init methods in DWRF typed builder (Boolean, Integer, Double, String, Binary) are copy paste of the corresponding dwio::common builders.

PingLiuPing · 2026-03-10T13:32:36Z

Is it the case that unlike DWRF, Parquet stores stats only per row group?

Parquet also stores page-level statistics. See:

velox/velox/dwio/parquet/thrift/parquet.thrift

Lines 210 to 213 in 4bea9ba

    
            * Statistics per row group and per page 
        
            * All fields are optional. 
        
            */ 
        
           struct Statistics {

and
https://parquet.apache.org/docs/file-format/metadata/#page-header

It doesn’t aggregate these and doesn’t store file-level stats.

Yes, Parquet aggregates statistics at the page level and row-group level, but not at the file level.
In #16062, I implemented file-level statistics for Parquet according to Iceberg requirements.

I think this PR is still valuable. It decouples statistics computation from format-specific serialization and introduces format-agnostic statistics into dwio::common, so it now only depends on velox_dwio_common. This also allows the statistics computation logic to be shared across formats.

For Parquet, since it is not straightforward to reuse dwio::common::StatisticsBuilder due to the differences, I think it might makes sense to keep the existing Parquet writer statistics logic.

…ookincubator#16693) Summary: The DWRF writer's StatisticsBuilder contains merge/addValues logic that is format-agnostic, but lives in the DWRF writer library, creating an undesirable cross-format dependency for consumers like Axiom and Parquet. This change extracts the format-agnostic parts into new dwio::common classes: - StatisticsBuilder base class with merge(), reset(), create(), createTree() - Typed builders: Boolean, Integer, Double, String, Binary - build() method that produces read-only ColumnStatistics snapshots The DWRF builders now extend the common base, adding only toProto() and proto-based build() for DWRF file format serialization. Axiom's StatisticsBuilderImpl now wraps dwio::common::StatisticsBuilder, removing the DWRF writer dependency. Differential Revision: D95885404

Summary: X-link: facebookincubator/velox#16693 The DWRF writer's StatisticsBuilder contains merge/addValues logic that is format-agnostic, but lives in the DWRF writer library, creating an undesirable cross-format dependency for consumers like Axiom and Parquet. This change extracts the format-agnostic parts into new dwio::common classes: - StatisticsBuilder base class with merge(), reset(), create(), createTree() - Typed builders: Boolean, Integer, Double, String, Binary - build() method that produces read-only ColumnStatistics snapshots The DWRF builders now extend the common base, adding only toProto() and proto-based build() for DWRF file format serialization. Axiom's StatisticsBuilderImpl now wraps dwio::common::StatisticsBuilder, removing the DWRF writer dependency. Differential Revision: D95885404

…ookincubator#16693) Summary: The DWRF writer's StatisticsBuilder contains merge/addValues logic that is format-agnostic, but lives in the DWRF writer library, creating an undesirable cross-format dependency for consumers like Axiom and Parquet. This change extracts the format-agnostic parts into new dwio::common classes: - StatisticsBuilder base class with merge(), reset(), create(), createTree() - Typed builders: Boolean, Integer, Double, String, Binary - build() method that produces read-only ColumnStatistics snapshots The DWRF builders now extend the common base, adding only toProto() and proto-based build() for DWRF file format serialization. Axiom's StatisticsBuilderImpl now wraps dwio::common::StatisticsBuilder, removing the DWRF writer dependency. Differential Revision: D95885404

mbasmanova · 2026-03-10T14:32:05Z

@PingLiuPing I fixed code duplication. A follow-up PR #16700 shows how the newly extracted class is used. Another follow-up Axiom PR facebookincubator/axiom#1031 shows e2e usage.

PingLiuPing

Thanks.

PingLiuPing · 2026-03-10T15:44:15Z

velox/dwio/common/StatisticsBuilder.cpp

+      break;
+    }
+    default:
+      VELOX_FAIL("Not supported type: {}", kind);


nit: dwrf/writer/StatisticsBuilder.cpp uses DWIO_RAISE

PingLiuPing · 2026-03-10T15:45:54Z

velox/dwio/common/StatisticsBuilder.cpp

+  }
+}
+
+bool isValidLength(const std::optional<uint64_t>& length) {


nit: this one still duplicated.

Yes, but it is trivial. Probably not worth the trouble of sharing.

Summary: X-link: facebookincubator/velox#16693 The DWRF writer's StatisticsBuilder contains merge/addValues logic that is format-agnostic, but lives in the DWRF writer library, creating an undesirable cross-format dependency for consumers like Axiom and Parquet. This change extracts the format-agnostic parts into new dwio::common classes: - StatisticsBuilder base class with merge(), reset(), create(), createTree() - Typed builders: Boolean, Integer, Double, String, Binary - build() method that produces read-only ColumnStatistics snapshots The DWRF builders now extend the common base, adding only toProto() and proto-based build() for DWRF file format serialization. Axiom's StatisticsBuilderImpl now wraps dwio::common::StatisticsBuilder, removing the DWRF writer dependency. Reviewed By: Yuhta Differential Revision: D95885404 fbshipit-source-id: 28da10bb54506b3f305021758f74485291596b59

meta-codesync · 2026-03-10T17:37:50Z

This pull request has been merged in 5699270.

mbasmanova requested a review from majetideepak as a code owner March 10, 2026 01:18

meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 10, 2026

meta-codesync bot added fb-exported meta-exported labels Mar 10, 2026

mbasmanova requested review from PingLiuPing, Yuhta and xiaoxmeng March 10, 2026 01:20

mbasmanova force-pushed the export-D95885404 branch from 55cecef to f6afc29 Compare March 10, 2026 01:23

PingLiuPing reviewed Mar 10, 2026

View reviewed changes

mbasmanova force-pushed the export-D95885404 branch from f6afc29 to cddfbc7 Compare March 10, 2026 13:35

mbasmanova force-pushed the export-D95885404 branch from cddfbc7 to 7ef6446 Compare March 10, 2026 13:43

mbasmanova force-pushed the export-D95885404 branch from 7ef6446 to 443612b Compare March 10, 2026 14:00

Yuhta approved these changes Mar 10, 2026

View reviewed changes

PingLiuPing mentioned this pull request Mar 10, 2026

feat: Implement ParquetReader::columnStatistics() #16700

Closed

PingLiuPing approved these changes Mar 10, 2026

View reviewed changes

meta-codesync bot closed this in 5699270 Mar 10, 2026

facebook-github-bot added the Merged label Mar 10, 2026

Conversation

mbasmanova commented Mar 10, 2026

Uh oh!

netlify bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for meta-velox canceled.

Uh oh!

meta-codesync bot commented Mar 10, 2026

Uh oh!

PingLiuPing left a comment

Choose a reason for hiding this comment

Uh oh!

mbasmanova commented Mar 10, 2026

Uh oh!

PingLiuPing Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

PingLiuPing Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

PingLiuPing commented Mar 10, 2026

Uh oh!

mbasmanova commented Mar 10, 2026

Uh oh!

PingLiuPing left a comment

Choose a reason for hiding this comment

Uh oh!

PingLiuPing Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

PingLiuPing Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

mbasmanova Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

meta-codesync bot commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

netlify bot commented Mar 10, 2026 •

edited

Loading