[WIP] Iceberg deletion vector support - #2119
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bd1ca373d
ℹ️ 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".
| const bool effective_need_only_count = need_only_count; | ||
| #endif | ||
|
|
||
| const bool can_use_count_cache = effective_need_only_count && headers_requested |
There was a problem hiding this comment.
Negate the
_headers guard before using the count cache
When a count-style query requests _headers and a cached row count exists, this predicate enables the ConstChunkGenerator shortcut, so no data GET occurs and the result can contain metadata-probe/HEAD headers instead of the requested GET headers. Conversely, ordinary count queries without _headers can no longer use the cache. Require !headers_requested here so the shortcut is skipped rather than falling back to different headers.
AGENTS.md reference: AGENTS.md:L153-L153
Useful? React with 👍 / 👎.
| @@ -0,0 +1,46 @@ | |||
| --- | |||
There was a problem hiding this comment.
Add mandatory sidebar fields to the new Puffin docs
Both this new page and docs/en/interfaces/formats/Puffin/PuffinMetadata.md omit the mandatory sidebar_label and sidebar_position frontmatter fields, leaving the new documentation pages noncompliant with the repository's required metadata. Add both fields to each page's frontmatter.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
Restore `!headers_requested` after a merge-conflict resolution inverted the condition, which disabled the cache for ordinary counts and allowed wrong headers on cache hits. Co-authored-by: Cursor <cursoragent@cursor.com>
Include `sidebar_label`, `sidebar_position`, and input/output flags so the new pages match AGENTS.md and peer format docs. Co-authored-by: Cursor <cursoragent@cursor.com>
Without the guard, Fast test builds that disable Avro fail to compile because IcebergPathFromMetadata is only visible when Avro is enabled. Co-authored-by: Cursor <cursoragent@cursor.com>
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Iceberg deletion vector support
Documentation entry for user-facing changes
Iceberg deletion vector support.
Based on uncommited Puffin file reader ClickHouse#109990.
CI/CD Options
Exclude tests:
Regression jobs to run: