Add CSV output format for SQL query results#4728
Conversation
Add a --format csv flag to the query command for exporting results as CSV. Uses Go's encoding/csv for proper escaping and quoting. Column headers are included as the first row. Co-authored-by: Isaac
…ests Co-authored-by: Isaac
shreyas-goenka
left a comment
There was a problem hiding this comment.
Note: This review was posted by Claude (AI assistant). Shreyas will do a separate, more thorough review pass.
Priority: MEDIUM — Dual-flag design concern
MEDIUM: --format vs --output dual-flag confusion
The PR introduces a --format flag for SQL statement execution that is separate from the existing --output flag used everywhere else in the CLI. This creates two different ways to control output format, which may confuse users. Consider whether --output csv could be extended to handle this case instead, or at minimum document the distinction clearly.
Other Observations
- CSV output implementation is clean and correct
- Good handling of SQL result pagination
- Proper escaping of CSV values with special characters
- Missing test for interaction between
--formatand--outputflags when both are set
The main thing to discuss is whether this warrants a new flag or should extend the existing --output flag.
|
Re: We intentionally use a separate Adding The PR already includes a mutual-exclusion guard that rejects using both flags together with a clear error message. |
renaudhartert-db
left a comment
There was a problem hiding this comment.
Re:
--formatvs--outputflag concern:We intentionally use a separate
--formatflag here rather than extending--output. The global--outputflag is aPersistentFlagon the root command with a hard-codedSet()validator that only acceptsjsonandtext. There is no mechanism in Cobra/pflag to extend a parent's persistent flag with additional values on a per-command basis.Adding
csvto the global--outputwould require changes tolibs/flags/output.go, thecmdiorender pipeline, and every command would need to handle or rejectcsv. That is a large, invasive change for a feature that only applies to the SQL query command.The PR already includes a mutual-exclusion guard that rejects using both flags together with a clear error message.
Should they be mutually exclusive though? For example, what if I'd like the table to be in csv and my error message in json? Naively, I would expect --format (or maybe --table-format?) to override the value of --output for the table.
We should probably address this eventually but right now we don't have any error formatting depending on output. |
Shadow the root command's persistent --output flag with a local flag on the query command that accepts text, json, and csv. This removes the separate --format flag and the mutual-exclusion guard between --format and --output. The local flag handles DATABRICKS_OUTPUT_FORMAT env var fallback and registers shell completions for all three values. Co-authored-by: Isaac
Use flags.OutputText/OutputJSON constants and a local outputCSV constant instead of bare string literals. Define envOutputFormat constant matching the root command's env var name. Co-authored-by: Isaac
Normalize --output flag value to lowercase before validation, matching the root command's flags.Output.Set() behavior. This ensures --output JSON works the same as on every other command. Silently ignore invalid DATABRICKS_OUTPUT_FORMAT env var values instead of hard-failing, matching the root command's behavior. Add tests for case insensitivity, env var override, invalid env var ignored, and explicit flag overriding env var. Co-authored-by: Isaac
Co-authored-by: Isaac
Why
The SQL query command supports JSON and table output but not CSV. CSV is the most common format for data export and piping into tools like Excel, pandas, and database imports.
Changes
Before:
databricks sql queryonly supports JSON and table output formats via the global--outputflag (text/json).Now: The query command shadows the global
--outputflag with a local version that also acceptscsv, writing results as RFC 4180 CSV with column headers as the first row.The local
--outputflag:text,json, andcsv(the global flag only acceptstextandjson)DATABRICKS_OUTPUT_FORMATenv var (invalid values silently ignored, matching root behavior)libs/flags/output.go,cmd/root/io.go,libs/cmdio/)Uses Go's
encoding/csvpackage for proper escaping and quoting.Test plan
--output(e.g.--output JSON)make checkspasses