Conversation
Member
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 090c16ac1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
zvonand
force-pushed
the
feature/antalya-26.6/aggregate-function-states-in-parquet-iceberg
branch
2 times, most recently
from
September 3, 2026 10:46
d5099b8 to
ea636f0
Compare
Member
Author
This comment was marked as outdated.
This comment was marked as outdated.
zvonand
added a commit
that referenced
this pull request
Sep 24, 2026
Reuse `ISerialization` for aggregate state conversion, trim nonessential comments, and consolidate overlapping tests while preserving distinct format and Iceberg coverage. PR: #2301
Parquet and Iceberg have no aggregate-state type, so neither can describe an `AggregateFunction` or `SimpleAggregateFunction` column with its schema alone. This adds a ClickHouse-only annotation next to the data - the `clickhouse.column_types` key of a parquet file's footer, and a `clickhouse.type` key on an Iceberg schema field - naming the ClickHouse type so it can be rebuilt on read. An `AggregateFunction` state is stored as an opaque `BYTE_ARRAY` / `binary` holding the bytes the `-State` combinator produces; a `SimpleAggregateFunction(f, T)` is stored as an ordinary value of `T`. Other query engines ignore the key and see a plain binary or `T`-typed column. The recorded name always carries the state version, which pins the serialized layout: `getName` drops a zero version, so a versioned function pinned to version 0 would otherwise be rebuilt with the default version and its bytes misread. `getNameForAnnotation` keeps it. Two experimental settings gate the feature, both off by default: `allow_experimental_aggregate_function_states_in_parquet` for writing such a column and for reconstructing one during parquet schema inference, and `allow_experimental_aggregate_function_states_in_iceberg` for `CREATE TABLE` and for honouring a `clickhouse.type` key that names an `AggregateFunction`. Honouring the annotation lets the file or the table metadata, rather than the query, choose the deserializer the stored bytes are handed to, so a refused annotation is rejected rather than read as `String` - reading states as strings would be a wrong result, not an error. `SimpleAggregateFunction` needs no opt-in on read, holding ordinary values with no state deserializer involved. The annotation is checked against the type the schema derives on its own before it is honoured, so a stale or crafted one cannot re-type a column to anything of the same nesting shape. The parquet reader checks strictly, requiring a type its own writer maps to what the file holds; Iceberg checks the nesting structure, which is all its type system allows. The Iceberg gate is read from the context of the query that parses the schema rather than from one captured at `ATTACH`, so a reading query can opt in at all; `IcebergSchemaProcessor::addIcebergTableSchema` publishes a schema-id only once every field has parsed and drops what it wrote on failure, so a refused read can be retried with the setting on. The metadata prefetcher asks for the schema-id with `SchemaParsing::Skip`, since it has no query to carry the gate and must not publish a schema that later queries would be served. The parquet schema cache is keyed by both settings, so a schema inferred with a gate open is not served to a query that has it closed. `ALTER TABLE ... EXPORT PART` and `EXPORT PARTITION` from an `AggregatingMergeTree` table into an Iceberg table carry both gates: `EXPORT PARTITION` records them in its ZooKeeper manifest, so every replica executing the task applies the values the `ALTER` gave instead of its own profile, and the destination is resolved under those settings. A manifest without the fields, written before they existed, reads them as closed. Iceberg records no bounds for aggregate states, whose extremes cannot be compared, and parquet min/max statistics over serialized states are never used for pruning. Adds `04673_parquet_aggregate_function_state`, integration tests for the export paths and for round-tripping states through Spark, and unit tests for the annotation matching, the name parsing, the schema processor and the metadata generator. Documents the feature in `Parquet.md` and `iceberg.md`. PR: #2301
zvonand
force-pushed
the
feature/antalya-26.6/aggregate-function-states-in-parquet-iceberg
branch
from
September 24, 2026 11:23
de5caea to
322a47b
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2206
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Support aggregate function states in Parquet and Iceberg
CI/CD Options
Exclude tests:
Regression jobs to run: