8000 Made dolt diff --stat -r sql an error by timsehn · Pull Request #9247 · dolthub/dolt · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Made dolt diff --stat -r sql an error #9247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 27, 2025
Merged

Made dolt diff --stat -r sql an error #9247

merged 2 commits into from
May 27, 2025

Conversation

timsehn
Copy link
Contributor
@timsehn timsehn commented May 23, 2025

Fixes #7817

From Claude:

⏺ Perfect! The changes are working correctly. Let me summarize what I've
  implemented:

  Summary

  I successfully implemented the fix for GitHub issue #7817. The issue was
  that dolt diff --stat -r sql was throwing a generic "unsupported" error,
  but it should return a more specific error message explaining that SQL
  format diffs are only for schema or data changes.

  Changes Made:

  1. Updated the error message in
  /Users/timsehn/dolthub/git/dolt/go/cmd/dolt/commands/diff_output.go:
    - Changed from: "diff stats are not supported for sql output"
    - Changed to: "SQL format diffs only rendered for schema or data 
  changes"
  2. Updated the test in
  /Users/timsehn/dolthub/git/dolt/integration-tests/bats/sql-diff.bats:
    - Updated the expected error message in the test case to match the new
  error message

  Reasoning:

  The --stat flag shows statistics about diffs (like number of rows
  changed), which cannot be meaningfully represented as SQL statements. SQL
   format should only be used for actual schema or data changes that can be
   expressed as executable SQL commands. The new error message clearly
  communicates this limitation to users.

  The implementation correctly returns an error when someone tries to use
  dolt diff --stat -r sql, and the tests confirm that:
  1. The specific error case now returns the improved error message
  2. Normal SQL diff functionality continues to work as expected

> /cost 
  ⎿ Total cost:            $3.08
    Total duration (API):  11m 9.9s
    Total duration (wall): 25m 15.4s
    Total code changes:    2 lines added, 3 lines removed
    Token usage by model:
        claude-3-5-haiku:  79.9k input, 1.9k output, 0 cache read, 0 cache 
    write
           claude-sonnet:  23.2k input, 15.3k output, 4.1m cache read, 
    396.2k cache write

@coffeegoddd
Copy link
Contributor

@timsehn DOLT

comparing_percentages
100.000000 to 100.000000
version result total
0d2a40e ok 5937457
version total_tests
0d2a40e 5937457
correctness_percentage
100.0

@coffeegoddd
Copy link
Contributor

@timsehn DOLT

comparing_percentages
100.000000 to 100.000000
version result total
d37b21b ok 5937457
version total_tests
d37b21b 5937457
correctness_percentage
100.0

@timsehn timsehn requested a review from jycor May 27, 2025 16:43
jycor
jycor approved these changes May 27, 2025
View reviewed changes
Copy link
Contributor
@jycor jycor left a comment

Choose a reason for hiding this comment

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

LGTM

@timsehn timsehn merged commit e4da223 into main May 27, 2025
21 checks passed
@tbantle22 tbantle22 deleted the tim/fix-diff-stat-r-rql branch May 28, 2025 21:35
@BrewTestBot BrewTestBot mentioned this pull request May 28, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jycor jycor jycor approved these changes

Assignees
No one assigned
Labels
correctness_approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dolt diff --stat -r sql is unimplemented
3 participants
@timsehn @coffeegoddd @jycor
0