Skip to content

Export CSV config revamp to support table exports#199

Open
davidwzhao wants to merge 26 commits intomainfrom
dz-csv-export-table-2
Open

Export CSV config revamp to support table exports#199
davidwzhao wants to merge 26 commits intomainfrom
dz-csv-export-table-2

Conversation

@davidwzhao
Copy link
Contributor

@davidwzhao davidwzhao commented Feb 17, 2026

Add export_csv_config_v2 with csv_source and table_def support

Adds grammar and printer support for the export_csv_config_v2 format, which uses the existing ExportCSVConfig proto fields csv_source and csv_config to provide a cleaner separation between data source specification and CSV formatting options.

Protobuf Structure

ExportCSVConfig now supports two patterns:

  • Legacy format: Uses data_columns (repeated ExportCSVColumn) with inline syntax_* and compression options
  • V2 format: Uses csv_source (ExportCSVSource) and csv_config (CSVConfig), where:
    • csv_source is a oneof containing either table_def or gnf_columns
    • csv_config is a CSVConfig object with csv_* prefixed options (header_row, delimiter, compression, etc.)

Changes

  • Grammar: Added export_csv_config_v2 parser/printer rules using csv_source and csv_config
  • Go printer: Added ExportCSVSource printer and updated ExportCSVConfig to emit v2 format when data_columns is empty
  • Codegen: Regenerated parsers and printers for all SDKs
  • Tests: Added test cases using both table_def and gnf_columns in the v2 format

Also renamed CSVConfig.partition_sizepartition_size_mb for clarity.

(thanks Claude for the description 😄)

Comment on lines 313 to 314
// Partitioning (for export)
int64 partition_size = 12;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was some desire to name this partition_size_mb? 🤔 Maybe I'm misremembering. I personally don't mind either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidwzhao davidwzhao marked this pull request as ready for review February 18, 2026 04:39
@davidwzhao davidwzhao requested a review from comnik February 18, 2026 04:39
@davidwzhao davidwzhao changed the title Export CSV Table Export CSV config revamp to support table exports Feb 20, 2026
@davidwzhao davidwzhao requested a review from mmcgr February 20, 2026 01:16
Copy link
Contributor

@mmcgr mmcgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Just one tiny nit.

decimal_separator: str = _extract_value_string(builtin.dict_get(config, "csv_decimal_separator"), ".")
encoding: str = _extract_value_string(builtin.dict_get(config, "csv_encoding"), "utf-8")
compression: str = _extract_value_string(builtin.dict_get(config, "csv_compression"), "auto")
partition_size: int = _extract_value_int64(builtin.dict_get(config, "csv_partition_size_mb"), 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
partition_size: int = _extract_value_int64(builtin.dict_get(config, "csv_partition_size_mb"), 0)
partition_size_mb: int = _extract_value_int64(builtin.dict_get(config, "csv_partition_size_mb"), 0)

and a matching update in the return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments