Skip to content

GH-37065: [C++][Parquet] use std::optional in parquet statistics#49403

Open
Alvaro-Kothe wants to merge 11 commits intoapache:mainfrom
Alvaro-Kothe:refactor/optional-statistics
Open

GH-37065: [C++][Parquet] use std::optional in parquet statistics#49403
Alvaro-Kothe wants to merge 11 commits intoapache:mainfrom
Alvaro-Kothe:refactor/optional-statistics

Conversation

@Alvaro-Kothe
Copy link
Contributor

@Alvaro-Kothe Alvaro-Kothe commented Feb 26, 2026

Rationale for this change

Simplify parquet statistics handling by using std::optional instead of has_xxx boolean + xxx attribute.

What changes are included in this PR?

  • Remove has_xxx public attributes for parquet statistics in favor of using std::optional
  • Deprecate some methods to construct the statistics (don't know if it was necessary)
  • Add new methods to handle std::optional

Are these changes tested?

Partially. The new changes are tested by the existing C++ test-suite. The deprecated methods aren't being tested.

Are there any user-facing changes?

Yes.

This PR includes breaking changes to public APIs.

This PR:

  • removes and modifies attributes in EncodedStatistics class;
  • deprecate methods that use signatures containing arguments with xxx + has_xxx.

@Alvaro-Kothe
Copy link
Contributor Author

Most of the CI errors are due to compiler warnings in c_glib due to the deprecations in statistics.h. Before diving deeper into fixing them, I'd like to ask for guidance on three points:

  1. Should I proceed with the deprecation of these methods, should I remove them directly, or is there a better way to handle this migration?
  2. I removed several public attributes and changed the type to some of them. Should I re-add these as deprecated members to maintain backward compatibility, or is the migration to std::optional sufficient?
  3. The Statistics class in parquest_types.h also uses a similar mechanism of boolean flags to keep track of what is defined. Should I also migrate it to use std::optional?

cc: @pitrou @felipecrv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant