Skip to content

Update table-diff.md with max_diff_rows information#129

Merged
zaidshabbir25 merged 2 commits into
mainfrom
zaidshabbir25-patch-1
Jun 30, 2026
Merged

Update table-diff.md with max_diff_rows information#129
zaidshabbir25 merged 2 commits into
mainfrom
zaidshabbir25-patch-1

Conversation

@zaidshabbir25

Copy link
Copy Markdown
Contributor

Added a note about the max_diff_rows setting in table-diff documentation, explaining its global cap across node pairs and implications for reported differences.

Added a note about the max_diff_rows setting in table-diff documentation, explaining its global cap across node pairs and implications for reported differences.
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds 10 lines to docs/commands/diff/table-diff.md documenting that max_diff_rows is a global cap across all node pairs per run, that hitting it produces a partial-results warning, and that large divergences cause early enumeration halts.

Changes

table-diff max_diff_rows documentation

Layer / File(s) Summary
max_diff_rows cap and large divergence docs
docs/commands/diff/table-diff.md
Documents the global max_diff_rows cap across all node pairs, partial-results warning handling, and the large-divergence early-halt behavior.

🐇 A cap on rows, shared by all pairs in a run,
When limits are reached, the diff says "I'm done!"
Treat warnings as partial, then fix and retry,
Until "TABLES MATCH" greets your wondering eye.
The bunny hops on, with docs clear and bright! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding max_diff_rows information to table-diff documentation.
Description check ✅ Passed The description is directly related to the documentation update and matches the changes in the pull request.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch zaidshabbir25-patch-1

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

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/commands/diff/table-diff.md (1)

90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Replace intensifier "very large" with stronger wording.

The style checker flags "very large" as a weak intensifier. Consider "large" or a specific quantitative description if appropriate.

🤖 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 `@docs/commands/diff/table-diff.md` at line 90, Replace the weak intensifier in
the sentence starting with “When the number of differences between nodes...” in
the table diff documentation with stronger, clearer wording such as “large” or a
more specific quantitative description; keep the rest of the sentence unchanged
and ensure the revised phrasing matches the doc style used in this section.
🤖 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 `@docs/commands/diff/table-diff.md`:
- Around line 83-87: The `table-diff` docs currently describe `max_diff_rows` as
a global limit across all node pairs, but the code paths in `table-diff` are
pair-scoped. Update the wording in the `table-diff` documentation to say the cap
is enforced per node pair (for example, n1/n2 and n1/n3 each track their own
limit) and that each pair can independently stop at `max_diff_rows`. Keep the
warning text aligned with this behavior so readers understand the limit is not
shared across pairs.

---

Nitpick comments:
In `@docs/commands/diff/table-diff.md`:
- Line 90: Replace the weak intensifier in the sentence starting with “When the
number of differences between nodes...” in the table diff documentation with
stronger, clearer wording such as “large” or a more specific quantitative
description; keep the rest of the sentence unchanged and ensure the revised
phrasing matches the doc style used in this section.
🪄 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: 3d3ebbd4-2e0b-4c4b-a3d3-9dc539378ff7

📥 Commits

Reviewing files that changed from the base of the PR and between 5fb45b6 and d7c6855.

📒 Files selected for processing (1)
  • docs/commands/diff/table-diff.md

Comment thread docs/commands/diff/table-diff.md Outdated
Comment on lines +83 to +87
5. The `max_diff_rows` setting caps the number of differing rows that `table-diff` collects in a single run.
- The cap applies globally across **all node pairs combined**, not to each pair individually. On a three-node cluster that diffs n1/n2 and n1/n3 at the same time, both pairs share the single cap. As a result, each pair can report fewer
differences than the table actually contains.
- The `table-diff stopped after reaching max_diff_rows` warning indicates that the run hit the cap. Treat the reported result as partial whenever the warning appears. Re-run `table-diff` after each repair pass, and continue until the command reports `TABLES MATCH`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚖️ Poor tradeoff

Documentation incorrectly states max_diff_rows is a global cap across all node pairs.

The implementation and tests show max_diff_rows is enforced per node pair, not globally across all pairs. The pairDiffRows map in internal/consistency/diff/table_diff.go stores separate atomic counters keyed by pair, and shouldStopPair checks the limit independently for each pair. The integration test TestTableDiffMaxDiffRowsPerPair explicitly validates this: with max_diff_rows=14 and two pairs each differing by 10 rows, both pairs fully report their diffs and DiffRowLimitReached remains false—behavior that would be impossible under a global cap.

Please correct the documentation to state that the cap applies per node pair (e.g., n1/n2, n1/n3), not globally across all pairs combined.

🤖 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 `@docs/commands/diff/table-diff.md` around lines 83 - 87, The `table-diff` docs
currently describe `max_diff_rows` as a global limit across all node pairs, but
the code paths in `table-diff` are pair-scoped. Update the wording in the
`table-diff` documentation to say the cap is enforced per node pair (for
example, n1/n2 and n1/n3 each track their own limit) and that each pair can
independently stop at `max_diff_rows`. Keep the warning text aligned with this
behavior so readers understand the limit is not shared across pairs.

Correct formatting and indentation.
@zaidshabbir25 zaidshabbir25 merged commit 9f66f99 into main Jun 30, 2026
3 checks passed
@zaidshabbir25 zaidshabbir25 deleted the zaidshabbir25-patch-1 branch June 30, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants