feat: add CSV output format support with multiple CLI options (#393)

This pull request significantly enhances spanner-mycli by introducing robust CSV output capabilities for query results, providing users with a structured, machine-readable output option that adheres to RFC 4180.

Key changes:
- Added comprehensive CSV output format support with automatic escaping of special characters
- Introduced --csv flag for direct CSV output and generic --format flag for flexible format selection
- Implemented case-insensitive format parsing via parseDisplayMode function for consistency
- Enhanced error handling for edge cases (empty columns) with proper logging
- Fixed potential panic in Vertical display mode when rows have more columns than headers
- Used defer statement for CSV writer flush to prevent data loss on errors
- Added extensive test coverage for CSV output including special characters and edge cases
- Updated documentation in README.md and system_variables.md

This implementation follows Go best practices for error handling and resource management, ensuring reliable CSV output even when writing to files via the --tee option.

Fixes #392

Author: apstndb

README.md

@@ -43,7 +43,7 @@ There are differences between spanner-mycli and spanner-cli that include not onl
   * Support `--skip-column-names` flag to suppress column headers in output (useful for scripting)
   * Support `--host` and `--port` flags as first-class options
   * Support `--deployment-endpoint` as an alias for `--endpoint`
-  * Support `--html` and `--xml` output format options with proper escaping (security-enhanced compared to reference implementation)
+  * Support `--html`, `--xml`, and `--csv` output format options with proper escaping (security-enhanced compared to reference implementation)
 * Generalized concepts to extend without a lot of original syntax
   * Generalized system variables concept inspired by Spanner JDBC properties
     * `SET <name> = <value>` statement
@@ -113,6 +113,8 @@ spanner:
   -t, --table                                             Display output in table format for batch mode.
       --html                                              Display output in HTML format.
       --xml                                               Display output in XML format.
+      --csv                                               Display output in CSV format.
+      --format=[table|tab|vertical|html|xml|csv]         Output format (alternative to individual format flags)
   -v, --verbose                                           Display verbose output.
       --credential=                                       Use the specific credential file
       --prompt=                                           Set the prompt to the specified format (default: spanner%t> )
@@ -888,8 +890,9 @@ They have almost same semantics with [Spanner JDBC properties](https://cloud.goo
 > - `TAB` - Tab-separated values (default for batch mode)
 > - `HTML` - HTML table format (compatible with Google Cloud Spanner CLI)
 > - `XML` - XML format (compatible with Google Cloud Spanner CLI)
+> - `CSV` - Comma-separated values (RFC 4180 compliant with automatic escaping)
 >
-> You can change the output format at runtime using `SET CLI_FORMAT = 'HTML';` or use command-line flags `--table`, `--html`, or `--xml`.
+> You can change the output format at runtime using `SET CLI_FORMAT = 'CSV';` or use command-line flags `--table`, `--html`, `--xml`, or `--csv`.
 
 ### Batch statements
 

cli_output.go

@@ -3,6 +3,7 @@ package main
 import (
 	"cmp"
 	_ "embed"
+	"encoding/csv"
 	"encoding/xml"
 	"fmt"
 	"html"
@@ -41,8 +42,35 @@ const (
 	DisplayModeTab
 	DisplayModeHTML
 	DisplayModeXML
+	DisplayModeCSV
 )
 
+// parseDisplayMode converts a string format name to DisplayMode.
+// It accepts both uppercase and lowercase format names.
+// Returns an error if the format name is invalid.
+func parseDisplayMode(format string) (DisplayMode, error) {
+	switch strings.ToUpper(format) {
+	case "TABLE":
+		return DisplayModeTable, nil
+	case "TABLE_COMMENT":
+		return DisplayModeTableComment, nil
+	case "TABLE_DETAIL_COMMENT":
+		return DisplayModeTableDetailComment, nil
+	case "VERTICAL":
+		return DisplayModeVertical, nil
+	case "TAB":
+		return DisplayModeTab, nil
+	case "HTML":
+		return DisplayModeHTML, nil
+	case "XML":
+		return DisplayModeXML, nil
+	case "CSV":
+		return DisplayModeCSV, nil
+	default:
+		return DisplayModeTable, fmt.Errorf("invalid format: %v", format)
+	}
+}
+
 // renderTableHeader renders TableHeader. It is nil safe.
 func renderTableHeader(header TableHeader, verbose bool) []string {
 	if header == nil {
@@ -65,6 +93,17 @@ func printTableData(sysVars *systemVariables, screenWidth int, out io.Writer, re
 
 	columnNames := renderTableHeader(result.TableHeader, false)
 
+	// Early return if no columns - Spanner requires at least one column in SELECT,
+	// so this only happens for edge cases where no output is expected
+	if len(columnNames) == 0 {
+		// Log logic error where we have rows but no columns
+		if len(result.Rows) > 0 {
+			slog.Error("printTableData called with empty column headers but non-empty rows - this indicates a logic error",
+				"rowCount", len(result.Rows))
+		}
+		return
+	}
+
 	switch mode {
 	case DisplayModeTable, DisplayModeTableComment, DisplayModeTableDetailComment:
 		rw := runewidthex.NewCondition()
@@ -140,8 +179,14 @@ func printTableData(sysVars *systemVariables, screenWidth int, out io.Writer, re
 		for i, row := range result.Rows {
 			fmt.Fprintf(out, "*************************** %d. row ***************************\n", i+1)
 			for j, column := range row { // j represents the index of the column in the row
-
-				fmt.Fprintf(out, format, columnNames[j], column)
+				var columnName string
+				if j < len(columnNames) {
+					columnName = columnNames[j]
+				} else {
+					// Use a default column name if row has more columns than headers
+					columnName = fmt.Sprintf("Column_%d", j+1)
+				}
+				fmt.Fprintf(out, format, columnName, column)
 			}
 		}
 	case DisplayModeTab:
@@ -177,6 +222,12 @@ func printTableData(sysVars *systemVariables, screenWidth int, out io.Writer, re
 		if err := printXMLResultSet(out, columnNames, result.Rows, sysVars.SkipColumnNames); err != nil {
 			slog.Error("printXMLResultSet() failed", "err", err)
 		}
+	case DisplayModeCSV:
+		// Output data in CSV format using encoding/csv for RFC 4180 compliance.
+		// This provides automatic escaping of special characters (commas, quotes, newlines).
+		if err := printCSVTable(out, columnNames, result.Rows, sysVars.SkipColumnNames); err != nil {
+			slog.Error("printCSVTable() failed", "err", err)
+		}
 	}
 }
 
@@ -540,7 +591,7 @@ func formatTypedHeaderColumn(field *sppb.StructType_Field) string {
 // Note: This function streams output row-by-row for memory efficiency.
 func printHTMLTable(out io.Writer, columnNames []string, rows []Row, skipColumnNames bool) error {
 	if len(columnNames) == 0 {
-		return nil
+		return fmt.Errorf("no columns to output")
 	}
 
 	if _, err := fmt.Fprint(out, "<TABLE BORDER='1'>"); err != nil {
@@ -619,7 +670,7 @@ type xmlResultSet struct {
 // with the TABLE format over memory efficiency.
 func printXMLResultSet(out io.Writer, columnNames []string, rows []Row, skipColumnNames bool) error {
 	if len(columnNames) == 0 {
-		return nil
+		return fmt.Errorf("no columns to output")
 	}
 
 	// Build the result set structure
@@ -660,3 +711,41 @@ func printXMLResultSet(out io.Writer, columnNames []string, rows []Row, skipColu
 	_, err := fmt.Fprintln(out) // Add final newline
 	return err
 }
+
+// printCSVTable outputs query results in CSV format.
+// The format follows RFC 4180 with automatic escaping via encoding/csv.
+// Column headers are included unless skipColumnNames is true.
+//
+// Note: Spanner requires at least one column in SELECT queries, so columnNames
+// should never be empty for valid query results. The empty check is defensive
+// programming for edge cases like client-side statements or error conditions.
+func printCSVTable(out io.Writer, columnNames []string, rows []Row, skipColumnNames bool) error {
+	if len(columnNames) == 0 {
+		return fmt.Errorf("no columns to output")
+	}
+
+	csvWriter := csv.NewWriter(out)
+	// Defer Flush to ensure the buffer is written to the output, even on error.
+	defer csvWriter.Flush()
+
+	if !skipColumnNames {
+		if err := csvWriter.Write(columnNames); err != nil {
+			return err
+		}
+	}
+
+	for _, row := range rows {
+		if err := csvWriter.Write(row); err != nil {
+			return err
+		}
+	}
+
+	// The deferred Flush() will write any remaining buffered data.
+	// We must check for an error from the writer, which can happen
+	// during a Write or the final Flush.
+	if err := csvWriter.Error(); err != nil {
+		return fmt.Errorf("csv writer error: %w", err)
+	}
+
+	return nil
+}

cli_output_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"strings"
 	"testing"
+
+	"github.com/MakeNowJust/heredoc/v2"
 )
 
 func TestPrintTableDataHTML(t *testing.T) {
@@ -216,8 +218,10 @@ func TestCLIFormatSystemVariable(t *testing.T) {
 		{"set TAB", "TAB", DisplayModeTab, false},
 		{"set HTML", "HTML", DisplayModeHTML, false},
 		{"set XML", "XML", DisplayModeXML, false},
+		{"set CSV", "CSV", DisplayModeCSV, false},
 		{"set html lowercase", "html", DisplayModeHTML, false},
 		{"set xml lowercase", "xml", DisplayModeXML, false},
+		{"set csv lowercase", "csv", DisplayModeCSV, false},
 		{"set invalid", "INVALID", DisplayModeTable, true},
 	}
 
@@ -252,6 +256,7 @@ func TestCLIFormatSystemVariableGetter(t *testing.T) {
 		{DisplayModeTab, "TAB"},
 		{DisplayModeHTML, "HTML"},
 		{DisplayModeXML, "XML"},
+		{DisplayModeCSV, "CSV"},
 		{DisplayMode(999), "TABLE"}, // Invalid mode should return default
 	}
 
@@ -344,8 +349,11 @@ func TestHTMLAndXMLHelpers(t *testing.T) {
 	t.Run("printHTMLTable with empty input", func(t *testing.T) {
 		var buf bytes.Buffer
 		err := printHTMLTable(&buf, []string{}, []Row{}, false)
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
+		if err == nil {
+			t.Error("expected error for empty columns, got nil")
+		}
+		if err != nil && err.Error() != "no columns to output" {
+			t.Errorf("unexpected error message: %v", err)
 		}
 		if buf.String() != "" {
 			t.Errorf("expected empty output, got: %q", buf.String())
@@ -355,8 +363,25 @@ func TestHTMLAndXMLHelpers(t *testing.T) {
 	t.Run("printXMLResultSet with empty input", func(t *testing.T) {
 		var buf bytes.Buffer
 		err := printXMLResultSet(&buf, []string{}, []Row{}, false)
-		if err != nil {
-			t.Errorf("unexpected error: %v", err)
+		if err == nil {
+			t.Error("expected error for empty columns, got nil")
+		}
+		if err != nil && err.Error() != "no columns to output" {
+			t.Errorf("unexpected error message: %v", err)
+		}
+		if buf.String() != "" {
+			t.Errorf("expected empty output, got: %q", buf.String())
+		}
+	})
+
+	t.Run("printCSVTable with empty input", func(t *testing.T) {
+		var buf bytes.Buffer
+		err := printCSVTable(&buf, []string{}, []Row{}, false)
+		if err == nil {
+			t.Error("expected error for empty columns, got nil")
+		}
+		if err != nil && err.Error() != "no columns to output" {
+			t.Errorf("unexpected error message: %v", err)
 		}
 		if buf.String() != "" {
 			t.Errorf("expected empty output, got: %q", buf.String())
@@ -390,3 +415,104 @@ func TestHTMLAndXMLHelpers(t *testing.T) {
 		}
 	})
 }
+
+func TestPrintTableDataCSV(t *testing.T) {
+	tests := []struct {
+		name         string
+		result       *Result
+		skipColNames bool
+		wantOutput   string
+	}{
+		{
+			name: "simple CSV output",
+			result: &Result{
+				TableHeader: toTableHeader("num", "str", "bool"),
+				Rows: []Row{
+					{"1", "test", "true"},
+					{"2", "data", "false"},
+				},
+			},
+			wantOutput: heredoc.Doc(`
+				num,str,bool
+				1,test,true
+				2,data,false
+			`),
+		},
+		{
+			name: "CSV with special characters",
+			result: &Result{
+				TableHeader: toTableHeader("name", "description", "value"),
+				Rows: []Row{
+					{"John, Jr.", "Says \"Hello\"", "100"},
+					{"Jane\nDoe", "Has,comma", "$50"},
+					{"Bob", "Normal text", "75"},
+				},
+			},
+			wantOutput: heredoc.Doc(`
+				name,description,value
+				"John, Jr.","Says ""Hello""",100
+				"Jane
+				Doe","Has,comma",$50
+				Bob,Normal text,75
+			`),
+		},
+		{
+			name: "CSV with skip column names",
+			result: &Result{
+				TableHeader: toTableHeader("col1", "col2"),
+				Rows: []Row{
+					{"val1", "val2"},
+					{"val3", "val4"},
+				},
+			},
+			skipColNames: true,
+			wantOutput: heredoc.Doc(`
+				val1,val2
+				val3,val4
+			`),
+		},
+		{
+			name: "empty result",
+			result: &Result{
+				TableHeader: nil,
+				Rows:        []Row{},
+			},
+			wantOutput: "",
+		},
+		{
+			name: "CSV with quotes and newlines",
+			result: &Result{
+				TableHeader: toTableHeader("text"),
+				Rows: []Row{
+					{"Line 1\nLine 2"},
+					{"\"Quoted\""},
+					{"Normal"},
+				},
+			},
+			wantOutput: heredoc.Doc(`
+				text
+				"Line 1
+				Line 2"
+				"""Quoted"""
+				Normal
+			`),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			sysVars := &systemVariables{
+				CLIFormat:       DisplayModeCSV,
+				SkipColumnNames: tt.skipColNames,
+			}
+
+			printTableData(sysVars, 0, &buf, tt.result)
+
+			got := buf.String()
+			if got != tt.wantOutput {
+				t.Errorf("printTableData() = %q, want %q", got, tt.wantOutput)
+			}
+		})
+	}
+}

docs/system_variables.md

@@ -19,7 +19,7 @@ TODO
   SELECT * FROM users;  -- Output without column headers
   ```
 - **Notes**:
-  - Affects table format (`CLI_FORMAT='TABLE'`) and tab format (`CLI_FORMAT='TAB'`)
+  - Affects table format (`CLI_FORMAT='TABLE'`), tab format (`CLI_FORMAT='TAB'`), CSV format (`CLI_FORMAT='CSV'`), HTML format (`CLI_FORMAT='HTML'`), and XML format (`CLI_FORMAT='XML'`)
   - Headers are always preserved in vertical format (`CLI_FORMAT='VERTICAL'`) as they are integral to the format
   - Can be set via `--skip-column-names` command-line flag
   - Useful for scripting and data processing where only raw data is needed
@@ -53,6 +53,7 @@ TODO
   - `TAB` - Tab-separated values (default for batch mode)
   - `HTML` - HTML table format
   - `XML` - XML format
+  - `CSV` - Comma-separated values (RFC 4180 compliant)
 - **Usage**: 
   ```sql
   SET CLI_FORMAT = 'VERTICAL';
@@ -64,15 +65,20 @@ TODO
   SET CLI_FORMAT = 'XML';
   SELECT * FROM users;  -- Output as XML
 
+  SET CLI_FORMAT = 'CSV';
+  SELECT * FROM users;  -- Output as CSV (comma-separated values)
+  
   SET CLI_FORMAT = 'TABLE_DETAIL_COMMENT';
   SELECT * FROM users;  -- Output as table with execution stats, all wrapped in /* */ comments
   ```
 - **Notes**:
   - Can be set via `--html` flag (sets to HTML format)
   - Can be set via `--xml` flag (sets to XML format)
+  - Can be set via `--csv` flag (sets to CSV format)
   - Can be set via `--table` flag (sets to TABLE format in batch mode)
   - HTML and XML formats are compatible with Google Cloud Spanner CLI
-  - All special characters are properly escaped in HTML and XML formats for security
+  - All special characters are properly escaped in HTML, XML, and CSV formats for security
+  - CSV format follows RFC 4180 standard with automatic escaping of commas, quotes, and newlines
   - The format affects how query results are displayed, not how they are executed
   - `TABLE_DETAIL_COMMENT` is particularly useful with `CLI_ECHO_INPUT=TRUE` and `CLI_MARKDOWN_CODEBLOCK=TRUE` for documentation