Add -p[1] performance statistics flag #625
Conversation
Implements the -p[1] flag for printing performance statistics after each result set: - -p or -p0: Human-readable format with network packet size, transaction count, and timing - -p1: Colon-separated format for spreadsheet/script processing Statistics include: - Network packet size (bytes) - Transaction count - Clock time: total, average, and transactions per second Fixes compatibility with ODBC sqlcmd -p flag.
There was a problem hiding this comment.
Pull request overview
This PR implements the -p[1] flag for printing performance statistics after each batch execution, matching ODBC sqlcmd behavior. The feature tracks cumulative statistics across the session and supports two output formats: human-readable (default) and colon-separated (for scripting).
Changes:
- Added performance statistics tracking with cumulative session metrics
- Implemented
-p[1]command-line flag with validation for values 0 and 1 - Added comprehensive test coverage for flag parsing and validation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/sqlcmd/sqlcmd.go | Added PrintStatistics field, SessionStats struct, and printStatistics method with dual output formats; integrated timing into runQuery |
| cmd/sqlcmd/sqlcmd.go | Added -p flag registration with default value handling, validation, and integration with sqlcmd instance |
| cmd/sqlcmd/sqlcmd_test.go | Added test cases for valid flag usage (-p, -p1, --print-statistics 0) and invalid usage (-p2) |
| README.md | Added documentation with usage examples showing the -p[1] flag and sample output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _ = rootCmd.Flags().BoolP("enable-quoted-identifiers", "I", true, localizer.Sprintf("Provided for backward compatibility. Quoted identifiers are always enabled")) | ||
| _ = rootCmd.Flags().BoolP("client-regional-setting", "R", false, localizer.Sprintf("Provided for backward compatibility. Client regional settings are not used")) | ||
| _ = rootCmd.Flags().IntP(removeControlCharacters, "k", 0, localizer.Sprintf("%s Remove control characters from output. Pass 1 to substitute a space per character, 2 for a space per consecutive characters", "-k [1|2]")) | ||
| _ = rootCmd.Flags().IntP(printStatistics, "p", -1, localizer.Sprintf("%s Print performance statistics for every result set. Pass 1 to output in colon-separated format", "-p[1]")) |
There was a problem hiding this comment.
The help text says "Print performance statistics for every result set" but the implementation actually prints statistics after each batch (GO command), not after each result set within a batch. Consider updating the help text to say "Print performance statistics after each batch" to accurately reflect the implementation.
| _, _ = out.Write([]byte(localizer.Sprintf("Clock Time (ms.): total:%d:avg:%.2f:(%.4f xacts per sec.)%s", | ||
| int(s.stats.TotalTimeMs), avgMs, xactsPerSec, SqlcmdEol))) | ||
| } else { | ||
| // Human-readable format | ||
| _, _ = out.Write([]byte(SqlcmdEol)) | ||
| _, _ = out.Write([]byte(localizer.Sprintf("Network packet size (bytes): %d%s", packetSize, SqlcmdEol))) | ||
| _, _ = out.Write([]byte(localizer.Sprintf("%d xact(s):%s", s.stats.TotalTransactions, SqlcmdEol))) | ||
| _, _ = out.Write([]byte(localizer.Sprintf("Clock Time (ms.): total %d avg %.2f (%.4f xacts per sec.)%s", | ||
| int(s.stats.TotalTimeMs), avgMs, xactsPerSec, SqlcmdEol))) |
There was a problem hiding this comment.
The README example shows xacts per sec with 2 decimal places (66.67) but the code formats it with 4 decimal places using %.4f. For consistency with the documentation example, consider using %.2f instead of %.4f for the xactsPerSec value in both format modes.
| program_name sqlcmd | ||
| ``` | ||
|
|
||
| - The `-p[1]` flag prints performance statistics after each result set. Use `-p` for human-readable format or `-p1` for colon-separated format suitable for scripts. |
There was a problem hiding this comment.
The documentation states the flag "prints performance statistics after each result set" but the implementation prints statistics after each batch (GO command), not after each individual result set within a batch. Consider updating the documentation to say "after each batch" to accurately reflect the implementation behavior.
| _, _ = out.Write([]byte(localizer.Sprintf("Clock Time (ms.): total:%d:avg:%.2f:(%.4f xacts per sec.)%s", | ||
| int(s.stats.TotalTimeMs), avgMs, xactsPerSec, SqlcmdEol))) | ||
| } else { | ||
| // Human-readable format | ||
| _, _ = out.Write([]byte(SqlcmdEol)) | ||
| _, _ = out.Write([]byte(localizer.Sprintf("Network packet size (bytes): %d%s", packetSize, SqlcmdEol))) | ||
| _, _ = out.Write([]byte(localizer.Sprintf("%d xact(s):%s", s.stats.TotalTransactions, SqlcmdEol))) | ||
| _, _ = out.Write([]byte(localizer.Sprintf("Clock Time (ms.): total %d avg %.2f (%.4f xacts per sec.)%s", | ||
| int(s.stats.TotalTimeMs), avgMs, xactsPerSec, SqlcmdEol))) |
There was a problem hiding this comment.
The PR description and README examples show the average time as an integer (e.g., "avg:15" or "avg 15"), but the code formats it with %.2f which will display decimal places (e.g., "15.00"). If matching ODBC sqlcmd output exactly is important, consider using %d formatting when the average is a whole number, or using %.0f to show no decimal places when they're not significant. Otherwise, %.2f is acceptable for consistency.
|
Duplicate of #631 which implements -p print statistics flag. |
1 participant
Summary
Implements the -p[1]\ flag for printing performance statistics after each result set, matching ODBC sqlcmd behavior.
Changes
Usage
Output Examples
Human-readable (-p\ or -p0):
\
Network packet size (bytes): 4096
1 xact(s):
Clock Time (ms.): total 15 avg 15 (66.67 xacts per sec.)
\\
Colon-separated (-p1):
\
Network packet size (bytes):4096
1 xact(s):
Clock Time (ms.): total:15:avg:15:(66.67 xacts per sec.)
\\
Testing
Fixes compatibility with ODBC sqlcmd -p flag.