Ace 205 mtree table diff misses differences same pk different data by zaidshabbir25 · Pull Request #150 · pgEdge/ace

zaidshabbir25 · 2026-07-17T09:18:54Z

mtree build on an empty table fails with a confusing internal error ("could not determine a reference node") instead of a clear "table has no rows" message.

Also update release notes

coderabbitai · 2026-07-17T09:19:45Z

📝 Walkthrough

Walkthrough

Changes

Merkle boundary hashing correction

Layer / File(s)	Summary
Exclusive leaf boundary semantics `db/queries/queries.go`, `docs/CHANGELOG.md`	Upper-bound predicates now use exclusive `<` semantics, with documentation covering boundary assignment and rebuilding affected trees.
Build-time conflict regression coverage `tests/integration/mtree_update_conflict_test.go`	Integration cases verify that both plain table-diff and Merkle diff detect one divergent row across single-row and boundary layouts.

Poem

I’m a rabbit counting leaves in a row,
With < keeping each boundary aglow.
No XOR tricks make conflicts disappear,
One changed carrot now shows bright and clear.
Build fresh trees, then hop without fear!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly names the mtree table-diff bug where same-PK rows with different data can be missed.
Description check	✅ Passed	The description is related to the PR, describing the empty-table mtree build error and release note update.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ACE-205-mtree-table-diff-misses-differences-same-pk-different-data

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.}

codacy-production · 2026-07-17T09:21:21Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results

Duplication 0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

coderabbitai

🧹 Nitpick comments (1)

internal/consistency/mtree/merkle.go (1)
1532-1544: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider removing redundant pool cleanup.

The explicit pool-closing loops here are redundant because there is already a deferred cleanup function at line 1477 that safely closes all pools upon return. You can simplify these error paths by returning the errors directly.
♻️ Proposed refactor
 	if successfulEstimates == 0 {
-		for _, p := range pools {
-			p.Close()
-		}
 		return fmt.Errorf("could not determine a reference node; failed to get row estimates from all nodes")
 	}
 
 	if maxRows == 0 {
-		for _, p := range pools {
-			p.Close()
-		}
 		return fmt.Errorf("table %s has 0 rows on all nodes; add data before building a Merkle tree", m.QualifiedTableName)
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/consistency/mtree/merkle.go` around lines 1532 - 1544, Remove the
redundant pool-closing loops from the successfulEstimates == 0 and maxRows == 0
error branches in the surrounding Merkle tree-building function. Return the
existing errors directly and rely on the deferred cleanup established near the
function’s setup.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/consistency/mtree/merkle.go`:
- Around line 1532-1544: Remove the redundant pool-closing loops from the
successfulEstimates == 0 and maxRows == 0 error branches in the surrounding
Merkle tree-building function. Return the existing errors directly and rely on
the deferred cleanup established near the function’s setup.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 331919c5-687b-4029-93a0-fa72bfc2f234

📥 Commits

Reviewing files that changed from the base of the PR and between 9e658ad and ee21deb.

📒 Files selected for processing (5)

db/queries/queries.go
docs/CHANGELOG.md
internal/consistency/mtree/merkle.go
tests/integration/mtree_empty_table_test.go
tests/integration/mtree_update_conflict_test.go

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/mtree_update_conflict_test.go`:
- Around line 74-76: Update the local seed function to call t.Helper() and defer
tx.Rollback(ctx) immediately after Begin succeeds, ensuring the transaction is
aborted and its connection returned even when a require.NoError assertion exits
the helper.
- Around line 39-55: Update runMtreeUpdateConflictCase to accept an explicit
divergentID argument, pass 3 from the MultiRowBoundary subtest and the
appropriate existing ID from SingleRow, and remove the helper’s ids[len(ids)-1]
selection. Add t.Helper() at the start of runMtreeUpdateConflictCase so
assertion failures point to the calling t.Run block.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 403eb9a4-3ddf-461d-9e99-f472d2224a84

📥 Commits

Reviewing files that changed from the base of the PR and between ee21deb and 8bce136.

📒 Files selected for processing (3)

db/queries/queries.go
docs/CHANGELOG.md
tests/integration/mtree_update_conflict_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

db/queries/queries.go

mason-sharp

Also, address the Coderabbit feedback

…es (ACE-205) mtree table-diff could report divergent nodes as identical: the same primary key holding different non-key data on two nodes produced matching Merkle-tree root hashes, so the diff printed "Merkle trees are identical" while table-diff correctly found the difference. This is the core UPDATE-UPDATE conflict that active-active clusters produce, silently reported as consistent. Root cause: leaf hashing used a closed ("<=") upper bound on block ranges, but a block's range_end is the EXCLUSIVE start of the next block -- the build offsets query pairs bounds with LEAD (each range_end is the following block's range_start) and splitBlocks sets range_end to a split point that GetBulkSplitPoints returns as the first row of the next chunk. The closed bound double-counted every boundary row into two adjacent leaves. When a table's rows collapsed into overlapping leaves that hashed identically (most visibly a single-row table, where the row lands in both [pk,pk] and [pk,inf)), the XOR-based parent hash cancelled the duplicate siblings to zero on every node, so divergent data yielded matching roots. Leaf hashing now uses an exclusive ("<") upper bound, matching the boundaries the build and split paths already produce, so each row belongs to exactly one leaf. Merkle trees built before this fix must be rebuilt (mtree build) to pick up the corrected layout. Adds an integration regression test (single-row and multi-row-boundary cases) asserting mtree diff agrees with table-diff. Verified against the full mtree integration suite plus db/queries and consistency unit tests.

…ble build Document pick_freshest latest-commit-wins (new capability) and the clearer mtree build empty-table error.

zaidshabbir25 requested a review from mason-sharp July 17, 2026 09:18

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

zaidshabbir25 force-pushed the ACE-205-mtree-table-diff-misses-differences-same-pk-different-data branch from ee21deb to 8bce136 Compare July 17, 2026 13:01

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread tests/integration/mtree_update_conflict_test.go Outdated

Comment thread tests/integration/mtree_update_conflict_test.go

mason-sharp requested changes Jul 20, 2026

View reviewed changes

Comment thread db/queries/queries.go Outdated

Comment thread docs/CHANGELOG.md Outdated

Comment thread internal/consistency/mtree/merkle.go

Comment thread tests/integration/mtree_empty_table_test.go

mason-sharp force-pushed the ACE-205-mtree-table-diff-misses-differences-same-pk-different-data branch from 8bce136 to 5c53823 Compare July 21, 2026 16:17

docs(changelog): add v2.1.1 entries for repair commit_ts and empty-ta…

f226236

…ble build Document pick_freshest latest-commit-wins (new capability) and the clearer mtree build empty-table error.

mason-sharp approved these changes Jul 21, 2026

View reviewed changes

mason-sharp merged commit ea996dc into main Jul 21, 2026
2 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ace 205 mtree table diff misses differences same pk different data#150

Ace 205 mtree table diff misses differences same pk different data#150
mason-sharp merged 2 commits into
mainfrom
ACE-205-mtree-table-diff-misses-differences-same-pk-different-data

zaidshabbir25 commented Jul 17, 2026 •

edited by mason-sharp

Loading

Uh oh!

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

codacy-production Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

mason-sharp left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zaidshabbir25 commented Jul 17, 2026 • edited by mason-sharp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codacy-production Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mason-sharp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zaidshabbir25 commented Jul 17, 2026 •

edited by mason-sharp

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

codacy-production Bot commented Jul 17, 2026 •

edited

Loading