feat: add row delta update - #15
Closed
manuzhang wants to merge 16 commits into
Closed
Conversation
![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: d1915a92e8
ℹ️ 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".
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5bbb4939
ℹ️ 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".
manuzhang
force-pushed
the
codex/row-delta
branch
from
June 10, 2026 07:28
0f158ee to
c569300
Compare
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c569300452
ℹ️ 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".
manuzhang
force-pushed
the
codex/row-delta
branch
from
June 10, 2026 09:54
c569300 to
6bc036e
Compare
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
manuzhang
force-pushed
the
codex/row-delta
branch
2 times, most recently
from
June 17, 2026 07:02
0dea7d5 to
27a496b
Compare
## Summary - Add a DeleteFiles snapshot update API and wire it through table and transaction update flows. - Add DeleteFiles implementation/build integration. - Add coverage for path matching, case-insensitive row filters, empty delete commits, and strict-projection partial-match rejection. ## Validation - `cmake --build build --target table_update_test` - `./build/src/iceberg/test/table_update_test '--gtest_filter=DeleteFilesTest.*'` Co-authored-by: Codex <codex@openai.com>
manuzhang
force-pushed
the
codex/row-delta
branch
from
June 22, 2026 04:45
27a496b to
277c566
Compare
Part 2 of the logging stack (builds on apache#722). Adds the logging API and a swappable default logger — the foundation the backends and macros plug into. **What's here** - `Logger`: the pluggable sink interface (`ShouldLog` / `Log` / `SetLevel` / `Flush` / `Initialize`). `ShouldLog()` is the single source of truth for runtime filtering. - `LogMessage` owns its formatted text so a sink can safely keep a record; reserves an attributes field for future structured logging. - Process-global default logger: `GetDefaultLogger` / `SetDefaultLogger` / `SetDefaultLevel`, with a lock-free thread-local fast path so logging stays cheap. - `Initialize` applies the `level` property, so config-driven levels actually work. `CurrentLogger()` is safe to call even from a `thread_local` destructor during thread shutdown. - `logger.h` stays backend-agnostic (never includes the build config header), so consumers see one stable API regardless of backend. **Examples** — using the API directly (the `LOG_*` macros that wrap it arrive in apache#725): ```cpp // A custom sink, installed as the process default. class MySink : public Logger { public: bool ShouldLog(LogLevel level) const override { return level >= level_; } void Log(LogMessage&& m) noexcept override { write_line(m.message); } void SetLevel(LogLevel level) override { level_ = level; } LogLevel level() const override { return level_; } private: std::atomic<LogLevel> level_{LogLevel::kInfo}; }; SetDefaultLogger(std::make_shared<MySink>()); // install process-wide SetDefaultLevel(LogLevel::kDebug); // adjust the threshold auto logger = GetDefaultLogger(); // borrow the current default if (logger->ShouldLog(LogLevel::kInfo)) { logger->Log(LogMessage{.level = LogLevel::kInfo, .message = "scan ready"}); } // Or configure from catalog-style properties (applies the "level" key): auto sink = std::make_shared<MySink>(); auto status = sink->Initialize({{std::string(kLevelProperty), "warn"}}); // -> kWarn ``` The same example is documented inline in `logger.h`. **Tests** — `logger_test`: default-logger API, level-from-property, invalid level rejected, concurrent swap/read, and logging during thread teardown. Built and run with clang/libc++ (spdlog ON and OFF). This pull request and its description were written by Isaac.
## What Cache the installed vcpkg packages on the Windows builds in `test` and `sql_catalog_test`, and skip the install step when the cache is present. ## Why Every Windows run reinstalls the same packages (zlib, nlohmann-json, nanoarrow, roaring, plus cpr / sqlite3) from scratch, costing a couple of minutes each time. Caching them removes that on every run after the first. This matches what `aws_test` already does. ## Validation A warm run reused the cached packages and skipped the install. The cache only rebuilds when the package list changes. Co-authored-by: Abanoub Doss <abanoub.doss@gmail.com>
## What
Add a concurrency block to the five workflows that don't have one —
`cpp-linter`, `pre-commit`, `license_check`, `zizmor`, and `codeql`:
```yaml
concurrency:
group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{ github.workflow }}
cancel-in-progress: true
```
## Why
The heavier workflows already cancel outdated runs, but these five don't
- so pushing again to a PR leaves the old runs going and tying up
runners. Grouping on `head_ref || sha` cancels superseded PR runs while
leaving `main` and scheduled runs untouched.
Co-authored-by: Abanoub Doss <abanoub.doss@gmail.com>
manuzhang
force-pushed
the
codex/row-delta
branch
from
June 23, 2026 03:43
277c566 to
bfaf21b
Compare
There was a problem hiding this comment.
Pull request overview
Adds new table update APIs for row-level deltas and data-file deletion, wiring them into the Table/Transaction surface area and build systems, with focused unit tests to validate commit semantics and conflict detection behavior.
Changes:
- Introduce
RowDelta(aMergingSnapshotUpdatesubclass) for adding rows, adding delete files, removing data/delete files, and validating conflicts. - Introduce
DeleteFilesfor deleting data files by path, by file object, or by row filter. - Expose
NewDeleteFiles()/NewRowDelta()viaTableandTransaction, and add corresponding tests + build registrations.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/iceberg/update/row_delta.h | Declares the new RowDelta update API and validation configuration methods. |
| src/iceberg/update/row_delta.cc | Implements RowDelta commit operation selection and validation logic. |
| src/iceberg/update/meson.build | Installs new update headers (delete_files.h, row_delta.h). |
| src/iceberg/update/merging_snapshot_update.h | Exposes DV validation helper to derived updates. |
| src/iceberg/update/delete_files.h | Declares the new DeleteFiles update API for deleting data files. |
| src/iceberg/update/delete_files.cc | Implements DeleteFiles staging and validation behavior. |
| src/iceberg/type_fwd.h | Adds forward declarations for DeleteFiles and RowDelta. |
| src/iceberg/transaction.h | Adds Transaction::NewDeleteFiles() and Transaction::NewRowDelta() APIs. |
| src/iceberg/transaction.cc | Wires new transaction update factories and includes new headers. |
| src/iceberg/test/table_test.cc | Extends static-table “not supported” coverage for new update APIs (partially). |
| src/iceberg/test/row_delta_test.cc | Adds tests for RowDelta operation summaries and validation behaviors. |
| src/iceberg/test/delete_files_test.cc | Adds tests for DeleteFiles delete modes and validation behavior. |
| src/iceberg/test/CMakeLists.txt | Adds new test sources to the table_update_test target. |
| src/iceberg/table.h | Adds Table::NewDeleteFiles() / Table::NewRowDelta() declarations; adds StaticTable::NewRowDelta() override. |
| src/iceberg/table.cc | Implements Table::NewDeleteFiles() / Table::NewRowDelta(); implements StaticTable::NewRowDelta(). |
| src/iceberg/meson.build | Adds new update .cc files to the core Meson build. |
| src/iceberg/CMakeLists.txt | Adds new update .cc files to the core CMake build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+344
to
+346
| Result<std::shared_ptr<RowDelta>> StaticTable::NewRowDelta() { | ||
| return NotSupported("Cannot create a row delta for a static table"); | ||
| } |
Comment on lines
+71
to
+72
| /// \brief Fail if any requested data/delete-file removal is missing. | ||
| RowDelta& ValidateDeletedFiles(); |
While reviewing apache#614, I noticed that `PlanErrorHandler::Accept` does not match the behavior of the Java implementation. A detailed comparison with Java's `ErrorHandlers` revealed several gaps between iceberg-cpp and Iceberg Java. Some of these are oversights in the original implementation, while others correspond to improvements that were made later in Iceberg Java, including: * apache/iceberg#13143 * apache/iceberg#14927 * apache/iceberg#15051 * apache/iceberg#16059 This PR closes those gaps and brings the error handling behavior in iceberg-cpp closer to the Java implementation.
## What Turn on compiler caching (sccache) for the Linux and macOS builds in `test`, `aws_test`, `sanitizer_test`, and `sql_catalog_test`, and switch the Windows `test` build to the same setup. `main` builds once and saves the cache; pull requests reuse it without writing back. ## Why Right now only the Windows builds reuse compiled output — every Linux and macOS build recompiles the whole bundled Arrow/Parquet/Avro/Boost stack from scratch, even though it never changes between PRs. Building it once and reusing it removes most of that repeated work. Saving the cache as a single file (instead of one upload per compiled file) also avoids the upload rate limit that causes "cache write error" spam. ## Validation On a warm pull-request run, every build reused the cache: 99.6–99.9% of files came from cache, zero write errors. The heavy builds drop from ~10–27 min to ~1.5–5 min. --------- Co-authored-by: Abanoub Doss <abanoub.doss@gmail.com>
manuzhang
force-pushed
the
codex/row-delta
branch
from
June 23, 2026 04:12
bfaf21b to
f5e70ea
Compare
Introduce the Iceberg v3 types (variant, geometry, geography), including their schema/JSON serialization and type-system integration (visitors, schema projection, etc.). Reading and writing data of these types is not implemented yet: conversion to/from Arrow, Avro, and Parquet returns an error, as do identity transform binding and scalar validation for them.
This fix is aligned with apache/iceberg#16521
Implements the RowDelta update builder, table and transaction factory methods, and focused tests for row-level add/delete flows. Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
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
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.
Summary
Tests