[FEAT] Diff & Merge Table by Poseidon-fan · Pull Request #188 · oceanbase/pyseekdb

Poseidon-fan · 2026-03-20T03:51:57Z

Fixes #187

Summary by CodeRabbit

New Features
- Compare collections with diff() to show row-level differences.
- Merge collections with merge_into(), supporting conflict strategies: FAIL, THEIRS, OURS; invalid strategies are rejected.
Tests
- Added integration tests covering diff (identical/added/modified/deleted/empty/read-only) and merge (FAIL/THEIRS/OURS, no-conflict, invalid-strategy) behaviors.

test: rename _fork_enabled to _fork_table_enabled

coderabbitai · 2026-03-20T03:52:10Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d8a3ccf-b21d-4e2e-9a03-3448b262c3c4

📥 Commits

Reviewing files that changed from the base of the PR and between a07caed and 2e8402a.

📒 Files selected for processing (1)

src/pyseekdb/client/collection.py

📝 Walkthrough

Walkthrough

Adds server-gated DIFF and MERGE table operations: BaseClient internal support, Collection public APIs (.diff, .merge_into), integration tests for diff/merge behaviors, and a small test helper rename to probe fork-table capability.

Changes

Cohort / File(s)	Summary
BaseClient Diff/Merge Support `src/pyseekdb/client/client_base.py`	Added `_VALID_MERGE_STRATEGIES: ClassVar[set[str]]`, `_diff_merge_enabled()` capability check, `_collection_diff()` to run `DIFF TABLE ... AGAINST ...`, and `_collection_merge()` to run `MERGE TABLE ... INTO ... STRATEGY ...` with ownership and strategy validation.
Collection Public API `src/pyseekdb/client/collection.py`	Added `diff(self, other) -> list[dict[str, Any]]` and `merge_into(self, target, strategy: str = "FAIL") -> None` that delegate to BaseClient internal methods.
Integration Tests - Diff `tests/integration_tests/test_collection_diff.py`	New integration test suite validating DIFF TABLE behavior (identical/added/modified/deleted/empty/read-only) and the public `Collection.diff()` path; tests skip when server feature is disabled.
Integration Tests - Merge `tests/integration_tests/test_collection_merge.py`	New integration test suite validating MERGE TABLE behaviors for strategies FAIL/THEIRS/OURS, conflict handling/rollback, invalid-strategy error, and `Collection.merge_into()` usage; tests skip when server feature is disabled.
Integration Tests - Fork Refactor `tests/integration_tests/test_collection_fork.py`	Renamed test helper `_is_fork_enabled` -> `_is_fork_table_enabled` and updated probes to call `client._server._fork_table_enabled()`.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Collection
    participant BaseClient
    participant Server

    Client->>BaseClient: diff(incoming, current)
    activate BaseClient
    BaseClient->>BaseClient: _diff_merge_enabled()
    Note over BaseClient: check db_type == "seekdb" and version >= 1.2.0.0
    BaseClient->>BaseClient: _get_collection_table_name(incoming)\n_get_collection_table_name(current)
    BaseClient->>Server: DIFF TABLE <incoming_table> AGAINST <current_table>
    Server-->>BaseClient: diff rows
    deactivate BaseClient
    BaseClient-->>Client: list of diff entries

sequenceDiagram
    participant Client as Collection
    participant BaseClient
    participant Server

    Client->>BaseClient: merge_into(incoming, target, strategy)
    activate BaseClient
    BaseClient->>BaseClient: _diff_merge_enabled()
    Note over BaseClient: check db_type == "seekdb" and version >= 1.2.0.0
    BaseClient->>BaseClient: validate strategy in _VALID_MERGE_STRATEGIES
    BaseClient->>BaseClient: _get_collection_table_name(incoming)\n_get_collection_table_name(target)
    BaseClient->>Server: MERGE TABLE <incoming_table> INTO <target_table> STRATEGY <STRATEGY>
    Server-->>BaseClient: execution result / error
    deactivate BaseClient
    BaseClient-->>Client: None (raises on error)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

[TEST] rename _fork_enabled to _fork_table_enabled #186: Test helper rename to _is_fork_table_enabled / server probe updates — touches the same test utilities updated here.
Fork table #144: Changes around table-name resolution and version-gating patterns that the new diff/merge methods reuse.

Poem

🐰 I hopped through rows both new and old,
I sniffed the diffs, I marked the bold.
THEIRS, OURS, or FAIL — I chose with care,
I stitched the tables, patching each pair.
Hooray, a merge! 🥕✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main feature being added: Diff & Merge Table functionality for collections.
Linked Issues check	✅ Passed	All coding requirements from issue `#187` are met: DIFF TABLE and MERGE TABLE wrappers are implemented on Collection object, providing git-like workflow operations.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing DIFF TABLE and MERGE TABLE wrappers; no unrelated modifications or out-of-scope features are present.
Docstring Coverage	✅ Passed	Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pyseekdb/client/client_base.py`:
- Around line 1577-1584: The diff/merge currently builds table names from
incoming/current but always runs SQL on self, allowing cross-client operations;
add a guard at the start of the diff/merge helpers (before calling
_get_collection_table_name or _execute) that verifies both incoming and current
belong to this client/context (e.g., compare their client attribute or
underlying connection/DB identifier to self) and raise a ValueError if they
differ; apply the same check to the analogous block around the 1600-1613 region
so operations cannot run against a different client/schema.
- Around line 1566-1589: _collection_diff returns the raw payload from _execute
which can be tuples on some backends, but Collection.diff is documented to
return list[dict[str, Any]]; update _collection_diff to normalize the rows into
dicts before returning (like other BaseClient paths do): after calling
self._execute(diff_sql) convert each row to a mapping using the collection
schema/column names from _get_collection_table_name/incoming/current (or
existing helper used elsewhere) so the method (_collection_diff) always returns
a list of dicts when _diff_merge_enabled() is true.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 124ce44a-b7b2-426b-9d05-f80c2d5b2a72

📥 Commits

Reviewing files that changed from the base of the PR and between 0ee6337 and c521d0d.

📒 Files selected for processing (5)

src/pyseekdb/client/client_base.py
src/pyseekdb/client/collection.py
tests/integration_tests/test_collection_diff.py
tests/integration_tests/test_collection_fork.py
tests/integration_tests/test_collection_merge.py

src/pyseekdb/client/client_base.py

src/pyseekdb/client/collection.py

hnwyllmm

Thanks for your contribution. I left some comments.
Please add some samples for database fork/diff/merge and test the functions both in server and embedded mode.

hnwyllmm · 2026-03-23T02:21:30Z

tests/integration_tests/test_collection_diff.py

+            with contextlib.suppress(Exception):
+                self._execute(client, f"DROP TABLE IF EXISTS `{name}`")
+
+    def test_diff_identical_tables(self, db_client):


Could you please test with collections but not common tables?

hnwyllmm · 2026-03-23T02:24:33Z

tests/integration_tests/test_collection_merge.py

+            with contextlib.suppress(Exception):
+                self._execute(client, f"DROP TABLE IF EXISTS `{name}`")
+
+    def test_merge_no_conflict(self, db_client):


Please test the cases with collections but not common tables.

hnwyllmm · 2026-03-23T02:57:22Z

The related functions are not available for collections, so let's hold this feature for a while.

Poseidon-fan and others added 6 commits March 18, 2026 16:31

feat: fork database

b5109a6

refactor: fix lint err

c731e77

Merge branch 'oceanbase:develop' into develop

617fcb4

test: rename _fork_enabled to _fork_table_enabled

60b7b52

Merge branch 'develop' of github.com:Poseidon-fan/pyseekdb into develop

ee1681f

test: rename _fork_enabled to _fork_table_enabled

feat: support diff & merge table for seekdb

c521d0d

coderabbitai bot reviewed Mar 20, 2026

View reviewed changes

src/pyseekdb/client/client_base.py Outdated Show resolved Hide resolved

src/pyseekdb/client/client_base.py Outdated Show resolved Hide resolved

src/pyseekdb/client/collection.py Show resolved Hide resolved

Poseidon-fan added 2 commits March 20, 2026 14:00

fix: normalize diff line exec & validate cross collection

a07caed

doc: limitation on diff merge

2e8402a

hnwyllmm requested changes Mar 23, 2026

View reviewed changes

hnwyllmm marked this pull request as draft March 23, 2026 02:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEAT] Diff & Merge Table#188

[FEAT] Diff & Merge Table#188
Poseidon-fan wants to merge 8 commits intooceanbase:developfrom
Poseidon-fan:develop

Poseidon-fan commented Mar 20, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 20, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hnwyllmm left a comment

Uh oh!

hnwyllmm Mar 23, 2026

Uh oh!

hnwyllmm Mar 23, 2026

Uh oh!

hnwyllmm commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Poseidon-fan commented Mar 20, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hnwyllmm left a comment

Choose a reason for hiding this comment

Uh oh!

hnwyllmm Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

hnwyllmm Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

hnwyllmm commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Poseidon-fan commented Mar 20, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 20, 2026 •

edited

Loading