feat: implement -p flag for print statistics

[David Levy (dlevy-msft-sql)]

Problem

The Go implementation of sqlcmd does not support the legacy -p option for printing batch performance statistics.

Solution

Add -p and -p1 support. Statistics are printed after each batch execution in the legacy human-readable or colon-separated format. This PR reports batch totals only. Per-statement timing requested in #621 remains separate work.

Changes

File	Change
cmd/sqlcmd/sqlcmd.go	Parse and validate -p values 0 and 1, then pass the setting to Sqlcmd.
pkg/sqlcmd/commands.go	Accumulate execution time and print statistics after batch execution.
pkg/sqlcmd/sqlcmd.go	Measure query execution and format batch statistics.
pkg/sqlcmd/sqlcmd_test.go	Cover standard, colon-separated, disabled, sub-millisecond, and repeated-batch output.
README.md	Document the new option and output.

Usage

sqlcmd -S server -Q "SELECT 1" -p
sqlcmd -S server -Q "SELECT 1" -p1

Output formats

Standard format (-p or -p0):

Network packet size (bytes): 4096
1 xact(s):
Clock Time (ms.): total         5  avg   5.00 (200.00 xacts per sec.)

Colon-separated format (-p1):

4096:1:5:5.00:200.00

Testing

• go test -work ./pkg/sqlcmd -run '^TestPrintStatisticsUnit$' -count=1
• Repository-wide compile-only test passed after merging main.

Related issues

Related to #621. This PR implements batch-level -p statistics, not per-statement timing.

[Copilot]

Pull request overview

This PR adds support for a -p flag that prints per-batch performance statistics, improving compatibility with legacy ODBC sqlcmd and documenting the feature and its formats.

Changes:

• Extend the Sqlcmd engine with a PrintStatistics option and a printStatistics helper that measures batch execution time and prints statistics in standard or colon-separated format.
• Wire the new -p/--print-statistics flag through the CLI argument parsing pipeline, including normalization and propagation into the Sqlcmd instance.
• Add unit tests for the statistics output formats and disabled behavior, and document usage and sample output in the README.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
pkg/sqlcmd/sqlcmd.go	Adds PrintStatistics support, measures batch duration in runQuery, and implements printStatistics for both human-readable and colon-separated output.
pkg/sqlcmd/sqlcmd_test.go	Adds tests that verify standard-format output, colon-separated output, and the disabled case when PrintStatistics is nil.
cmd/sqlcmd/sqlcmd.go	Introduces the -p/--print-statistics CLI flag, integrates it into argument parsing, normalization, and run, and passes the option into the Sqlcmd core.
README.md	Documents the new -p behavior and shows an example of the standard statistics output format.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

🟡 Changes recommended

The statistics output format and documentation have compatibility/consistency issues (notably the xact[s] label and avg-time calculation) and the PR’s stated linkage to #621 does not match the implemented batch-level behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

pkg/sqlcmd/sqlcmd_test.go:767

• This test’s negative assertion also uses the incorrect "xact[s]" label; it should match whatever the production label is ("xact(s)" if aligning with legacy sqlcmd).

	// Should not contain statistics output
	assert.NotContains(t, output, "Network packet size", "Should not contain packet size when disabled")
	assert.NotContains(t, output, "xact[s]:", "Should not contain xacts label when disabled")
}

pkg/sqlcmd/sqlcmd_test.go:785

• Unit test expects "xact[s]" but the intended label is "xact(s)" for compatibility with legacy output and docs.

		assert.Contains(t, out, "Network packet size (bytes): 4096")
		assert.Contains(t, out, "3 xact[s]:")
		assert.Contains(t, out, "Clock Time (ms.): total       150")
		assert.Contains(t, out, "xacts per sec.")

pkg/sqlcmd/sqlcmd_test.go:833

• Unit test expects "xact[s]" but the intended label is "xact(s)" for compatibility with legacy output and docs.

		out := buf.String()
		assert.Contains(t, out, "10 xact[s]:")
		assert.Contains(t, out, "total      1000")

• Files reviewed: 6/6 changed files
• Comments generated: 6
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Statistics output currently hardcodes \n instead of using SqlcmdEol, and CLI tests don’t cover the claimed -p1 combined shorthand form.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

cmd/sqlcmd/sqlcmd_test.go:107

• Command-line conversion tests cover -p and -p 1, but not the -p1 shorthand form that the PR claims to support. Adding an explicit -p1 case helps ensure convertOsArgs / flag parsing continues to accept the combined form (similar to the existing -V10 test).

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

runQuery’s QueryContext error path is inconsistent with existing SQL error handling (and can leave batch-finalization behavior inconsistent), and the new localized CLI string lacks regenerated translation artifacts.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

pkg/sqlcmd/sqlcmd.go:475

• runQuery returns the raw QueryContext error without applying the same SQL-error handling rules used for ReturnMessage/rows.Err() (e.g., ExitOnError / ErrorSeverityLevel), and it also leaves the batch open until the end of the function. Handling this error path explicitly avoids inconsistent behavior and ensures EndBatch() is called before returning.

	rows, qe := s.db.QueryContext(ctx, query, retmsg)
	if qe != nil {
		s.Format.AddError(qe)
	}
	var err error

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The feature is correctly gated behind an explicit -p flag, integrates cleanly into existing batch execution flow, and includes targeted unit tests plus documentation updates.

Review details

Files not reviewed (1)

• internal/translations/catalog.go: Generated file

• Files reviewed: 17/18 changed files
• Comments generated: 0 new
• Review effort level: Lite