Experiment: Sqlcmd - #402
Draft
Shiwani Gupta (shiwanigupta0809) wants to merge 16 commits into
Draft
Conversation
Shiwani Gupta (shiwanigupta0809)
changed the title
Contributor
Author
|
Copilot resolve the merge conflicts in this pull request |
Copilot started work on behalf of
Shiwani Gupta (shiwanigupta0809)
August 27, 2026 17:25
View session
Contributor
Resolved the merge conflicts with |
added 13 commits
August 29, 2026 16:48
A batch that fails part-way still has result sets to render, but the client stopped iterating at the first ERROR token, so everything after the failing statement was lost. Add an opt-in defer_batch_errors mode that collects errors and keeps reading, tracking COLMETADATA as it goes so the rows of later result sets can still be parsed. Also expose the row count from each DONE token in arrival order. A statement under SET NOCOUNT ON sends no count and is recorded as None, so a caller can tell "affected zero rows" apart from "reported no count" — the distinction sqlcmd needs to place its "(N rows affected)" lines correctly. Both are additive and off by default, so existing callers are unaffected.
A single binary that stands in for either existing client, built on mssql-tds rather than the ODBC driver, so it carries no native install footprint and builds the same way on Windows and Linux. ODBC sqlcmd behaviour is the default, so existing scripts keep working unchanged. --compat go (or SQLCMDCOMPAT=go) selects go-sqlcmd's rendering and messages where the two disagree, and go-sqlcmd's subcommand CLI -- config, query, and the container lifecycle -- is available directly. Correctness is established by differential testing against the real binaries rather than hand-written expectations, so the suites cannot drift from what the references actually do: ODBC sqlcmd 116 cases on Windows, 111 on Linux go-sqlcmd legacy 62 cases on both go-sqlcmd subcommands 49 cases on both, 53 with the container lifecycle SQLCMDCOLORSCHEME 35 cases, captured through a PTY Where an option is not supported it is refused with a message rather than accepted and ignored, so a caller is never silently given behaviour it did not ask for. sqlcmd.md records the plan, the measured divergences between the two references, and the reasoning behind each decision.
ClientContext::login_server_name separates where to connect from what name to present at login. A connection through a tunnel, proxy or port-forward dials localhost while the login must still name the real server, so server-side routing and any name-based policy see the intended target. None keeps the previous behaviour of writing the dialled address. LOGIN7 stores ServerName as an offset/length pair separate from its payload, so a length taken from the dialled address with bytes taken from the override would corrupt the packet while looking correct from the client side. Both now read one accessor. The tests read the name back off the wire from the mock server rather than asserting on client state, and cover an override both longer and shorter than the address dialled; reintroducing the mismatch fails three of them. Also add the five federated methods the enum was missing -- AzCli, AzureDeveloperCli, AzurePipelines, Environment and ClientAssertion -- and map them to the same workflow byte as the other non-password credentials, since each resolves to a bearer token out of band. ActiveDirectoryMSI now maps alongside ActiveDirectoryManagedIdentity instead of falling through to the error arm, which it should have done all along as a documented alias.
The Entra methods parsed but never acquired a token: no factory was registered on the client context, so a connection that negotiated FedAuth reached the handshake with nothing to send. Every federated method now has one, covering the full set both references name -- default, password, service principal, managed identity, device code, workload identity, Azure CLI, Azure Developer CLI, Azure Pipelines, environment and client assertion -- plus the Application/MSI aliases. Most map onto an azure_identity credential; password and device code talk to the token endpoint directly because the Rust SDK has no equivalent. A unit test asserts every method registers a factory, since the failure is otherwise invisible until a real Entra server refuses the login. --server-name is now accepted rather than refused, resting on the LOGIN7 override added to mssql-tds. Three gaps found by comparing against the real binaries rather than reading the plan: -f with an unusable code page fell back to UTF-8, writing bytes the caller did not ask for. It is now refused, with the reference's own wording. SQLCMDINI reported nothing when the script could not be opened, where both references name the variable and its value. More subtly, -X suppressed environment seeding entirely -- but ODBC still seeds under -X and still runs the startup script, and only go-sqlcmd suppresses. Measured with :listvar against both. `open ads` is implemented instead of refused. The reference only handles Windows: its macOS build writes no password (Azure Data Studio reads UTF-16 from the Keychain, the Go library writes UTF-8) and its Linux build panics. This launches on all three, and hands the password over only where that can be done correctly -- elsewhere Azure Data Studio prompts, which beats storing something it cannot decode. Interactive input now goes through rustyline, which was already a dependency but unused, so the prompt has history and line editing. Piped input keeps the plain reader: an editor on a non-terminal would strip the bytes the batch parser needs.
A server message with state 127 ends the session whatever its severity, and
outranks both -b and -V. The exit code is the message's own number, so
RAISERROR(14599, 16, 127) exits 14599 and an ad-hoc RAISERROR('boom', 16, 127)
exits 50000. The rest of the batch is discarded.
Unix exit statuses are 8 bits and the two references disagree about that:
go-sqlcmd hands the full number to exit() and lets the OS truncate -- 50000
becomes 80, 14599 becomes 7 -- while msodbcsql clamps to a plain failure. On
Windows both return the number itself. All four combinations were measured
against the real binaries rather than inferred; the differential suites now
cover both compat modes.
Also back out the rustyline editor added in the previous commit. Captured
through a PTY, its line redraw erased output the results stream had already
written: the `a` column heading came back blank where both references show it.
The editor owns the line it is redrawing while the results stream is written
independently, and reconciling the two means routing every write through the
editor's external printer -- more than the history and line editing are worth
here. The prompt is plain text again, and now verified: `1> 2> 3> a` matches
go-sqlcmd byte for byte through the same PTY capture. The dependency is dropped
rather than left unused.
Two rows of the parity table were platform-specific and I had not checked whether that was a test gap or a feature gap. Both were test gaps, and running them found a real bug. The container lifecycle works on Linux and now runs there: 53 cases, matching Windows exactly. Doing so caught a regression from the `open ads` commit -- the "Open in Azure Data Studio" hint was added to `create` unconditionally, but the reference offers it only on Windows and macOS, where it implements the command. This port runs `open ads` on Linux too, but the hint list is compared against the reference, so it follows the same rule. Colour could not be compared end to end on Windows because that needs a pseudo-terminal and the harness has no equivalent of `script(1)` there. The parts that need no PTY now run on both platforms: the suppression gate, which is what a script capturing our output depends on, and `:list color`. Redirected output was separately confirmed byte-identical to go-sqlcmd on Windows. `:list color` needs a reachable server despite answering locally, because sqlcmd connects before it reads any input -- the first version of that test assumed otherwise and failed on Linux, where `-E` wants a Kerberos ticket. PARITY.md records what each platform actually covers and why, rather than leaving the difference unexplained.
The parity table claimed all three tools accepted -R and ignored it. That was wrong, and only running the binary showed it: ODBC implements it, on Linux as well as Windows. A script using -R would have got different output from this build -- a genuine drop-in gap, and one that had been written down as a non-issue. The reference goes through the platform's own locale services, so this does too rather than carrying a locale database: GetCurrencyFormatEx and friends on Windows, localeconv and strftime elsewhere. Money, decimal/numeric and every date/time type are formatted; int, bigint, float and real are left alone, matching the reference. Ten differential cases cover it on both platforms, including the types it must not touch. The Linux behaviour needed measuring rather than guessing. POSIX reports frac_digits as CHAR_MAX in the C locale, meaning "unspecified", and the reference renders that as no decimal places at all -- which is why 1234.56 comes back as 1235 there rather than unchanged. Three defects in the reference are deliberately not reproduced, because each puts something in front of a user that cannot be meant: datetime2 renders an unsubstituted printf specifier, `1:45:06.%07lu PM` time fails with "Internal error at LocalizeTimestampData" (Windows) negative money fails with "Internal error at ReadAndHandleColumnData" (Linux) go-sqlcmd accepts -R and ignores it, so the flag stays inert under --compat go.
Re-measuring the divergence table against all four binaries turned up eight defects a green differential suite had not caught: - SQLCMDCOMPAT was documented but never read; only --compat reached the parser. - -Q/-q bypassed the batch pipeline, so $(var) went to the server unexpanded and -e echoed nothing. Both worked from -i files. - -q and -Q ran both queries instead of -Q winning. - -m hid PRINT, which neither reference does, and -m -1 dropped the Msg header. - SQLCMDUSER was empty under -E; both references report the OS account. - SQLCMDDBNAME was filled from the landed database; both leave it empty. The go-only feature tests hard-coded a Windows integrated-auth connection, so seven of them could never run on Linux. They now take the same connection prefix as the differential suites. PARITY.md is corrected: four rows of the divergence table were recorded backwards, and the table now says which reference each mode actually matched.
The 57 long options invited the obvious question — what are the four Go does not have? — and the doc could not answer it. Every count in section 1 is now backed by a set diff rather than a total: - Short options: Go is 46, not 45. The doc counted -? for ODBC but not for Go. - 52 short = ODBC's 47 plus five it accepts but omits from its usage text (-D, -n, -O, -T) plus Go's -J. Almost no new surface. - 57 long = Go's 53 plus --dsn and --print-statistics, which are long spellings for ODBC options Go never implemented, plus --compat and --format. - "Superset" is now stated as a measured fact: diffing the three sets shows nothing either reference accepts is missing here. The output-format row claimed CSV and JSON. Neither was ever designed — the design doc has always said vert/vertical/ascii/horiz/horizontal, and all five work. The claim was invented in this document. What is a real defect is that an unrecognised name falls through silently instead of being refused, which breaks the standing rule that no option is accepted then ignored; that is recorded as a gap rather than fixed here. Also records two gaps found while answering the same question: the banner hardcodes NT on every platform where ODBC prints the platform name, and the binary is not self-contained the way go-sqlcmd is — it needs system OpenSSL 3.
extract-styles.ps1 wrote to $env:TEMP and wrap-styles.ps1 read it back, with an absolute path to one machine's working copy. Neither ran anywhere else. generate-schemes.ps1 does both steps, locates chroma through the Go module cache or an explicit -ChromaPath, and writes to a path derived from its own location. Also adds SUMMARY.md, a status overview for reviewers.
main replaced execute(sql, timeout, cancel) with ExecuteOptions and renamed move_to_next to advance_to_rows. The rebase carried the old call sites across without conflict, since the files themselves did not clash, so the breakage was semantic rather than textual.
These two tests were dropped during an earlier merge with main rather than adapted, taking their coverage of set_defer_batch_errors with them. Restored and moved onto on_rows/next_row/advance_to_rows.
Shiwani Gupta (shiwanigupta0809)
force-pushed
the
dev/shiwanigupta/rust-sqlcmd
branch
from
August 29, 2026 11:24
0e12bd7 to
ff2577f
Compare
added 3 commits
August 29, 2026 17:38
The tool moves from main.rs into lib.rs, so the same implementation can be a standalone binary or an archive the native ODBC sqlcmd links. `run` returns the exit code rather than exiting, because when the caller is the native binary it owns the process. main.rs becomes the four lines that call it. ffi.rs is the whole boundary: `sqlcmd_modern_claims` answers whether this side owns the command line and `sqlcmd_modern_main` runs it. No Rust type crosses and no allocation changes hands, so the two halves need agree on nothing but argv. What is claimed is deliberately narrow: the go-sqlcmd subcommand verbs, and the long options with no short form in the ODBC option string. Those are exactly the options go-sqlcmd added, so nothing written against ODBC sqlcmd can be using them, and a command line the ODBC tool could have parsed is never taken. The crate gains rlib, which the binary and tests link as usual, and staticlib, which the native build links so both CLIs end up in one executable.
The native tool's entry point is main(char**) on Unix but wmain(WCHAR**) on Windows, so a narrow-only boundary cannot be called from the Windows build. Mirror the pair as sqlcmd_modern_claims_w and sqlcmd_modern_main_w, taking UTF-16 and narrowing to the same Vec<String> the existing pair produces. Every decision after that is shared, and a test asserts the two agree on the same command lines, so the halves cannot diverge by platform. Both are cfg(windows): nothing changes for Unix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issues
Checklist
cargo bfmtpassescargo bclippypassescargo btestpasses