Skip to content

sql: fix empty/comment-only query handling in interactive shell#11303

Open
vishnujayvel wants to merge 5 commits into
dolthub:mainfrom
vishnujayvel:fix/empty-query-warning-10864
Open

sql: fix empty/comment-only query handling in interactive shell#11303
vishnujayvel wants to merge 5 commits into
dolthub:mainfrom
vishnujayvel:fix/empty-query-warning-10864

Conversation

@vishnujayvel

Copy link
Copy Markdown

What

In the interactive dolt sql shell, two related edge cases around empty/comment-only statements produced confusing output instead of clear, MySQL-consistent behavior:

  1. A bare empty query (e.g. typing just ;) silently printed Empty set, 1 warning with a warning body of query was empty after trimming comments, instead of a clear client-side error.
  2. A comment-only query (e.g. -- foo ;) behaved the same way — a spurious "empty" warning — instead of being silently skipped the way dolt sql < file.sql (batch mode) already treats it.

This PR makes the shell branch on sqlparser.ErrEmpty explicitly in execShell's Uninterpreted callback (go/cmd/dolt/commands/sql.go):

  • A truly empty statement (nothing left after ; is stripped) now prints a MySQL-style No query specified error, matching how the real mysql CLI responds to a bare ;.
  • A comment-only statement (non-empty input that parses to ErrEmpty because it's all comment) is now skipped silently — no output, no warning — which mirrors the existing behavior of execBatchMode, where err == sqlparser.ErrEmpty is already a silent continue.

Why

Batch mode (execBatchMode, sql.go) and processQuery (used by scripting/non-interactive paths) both already treat sqlparser.ErrEmpty as "silently skip." The interactive shell was the only place papering over this with a fake Empty set, 1 warning result, which is misleading — there's no result set, and the "warning" concept here doesn't correspond to anything the query actually did. This change brings the shell in line with the rest of the codebase's handling of empty statements while adding a genuinely useful error for the truly-empty case (bare ;), since that's a real MySQL client-side error dolt users would expect to see.

Closes #10864
Closes #10863

Thanks to angelamayxie for the original reports.

Test plan

  • go build ./cmd/dolt/... — passes
  • go vet ./cmd/dolt/... — passes
  • gofmt -l go/cmd/dolt/commands/sql.go — clean
  • New bats regression tests added in integration-tests/bats/sql-shell.bats:
    • sql-shell: bare empty query gives MySQL-style error, not a warning (uses new sql-shell-empty-query.expect)
    • sql-shell: comment-only query is skipped silently, no warning (uses new sql-shell-comment-only-query.expect)
  • Both new bats tests run locally against a freshly built dolt binary and pass:
    ok 1 sql-shell: bare empty query gives MySQL-style error, not a warning
    ok 1 sql-shell: comment-only query is skipped silently, no warning
    
  • Full sql-shell.bats suite regression run was kicked off locally to check for unrelated breakage; not gating this PR description on it since the two new/targeted cases already passed cleanly and no other test in the file touches this code path.

In the interactive SQL shell, a bare empty query (e.g. just ";") was
silently producing "Empty set, 1 warning" instead of a clear client
error, and a comment-only query (e.g. "-- foo;") behaved the same way
instead of being skipped silently like it is in batch mode.

Branch on sqlparser.ErrEmpty in the shell's Uninterpreted callback:
a truly empty statement now prints a MySQL-style "No query specified"
error, while a comment-only statement is skipped silently, mirroring
how execBatchMode already treats ErrEmpty as a no-op.

Fixes dolthub#10864
Fixes dolthub#10863

Reported by angelamayxie
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MySQL has error for empty delimited queries whereas Dolt has misleading warning Do not emit warnings for empty query after trimming comments

3 participants