Export CSV config revamp to support table exports#199
Export CSV config revamp to support table exports#199davidwzhao wants to merge 34 commits intomainfrom
Conversation
| // Partitioning (for export) | ||
| int64 partition_size = 12; |
There was a problem hiding this comment.
I think there was some desire to name this partition_size_mb? 🤔 Maybe I'm misremembering. I personally don't mind either way.
There was a problem hiding this comment.
I think that would be https://relationalai.atlassian.net/browse/RAI-47099
davidwzhao
changed the title
mmcgr
left a comment
mmcgr
left a comment
There was a problem hiding this comment.
Thanks!
Just one tiny nit.
| %validator_ignore_completeness DecimalValue | ||
| %validator_ignore_completeness BeTreeLocator | ||
| %validator_ignore_completeness BeTreeConfig | ||
| %validator_ignore_completeness ExportCSVColumns |
There was a problem hiding this comment.
What is this about? 🤔 Is this from not declaring "optional" that we were discussing with @nystrom ?
There was a problem hiding this comment.
In the grammar i directly construct the ExportCSVSource from columns, rather than having an intermediate - see below
export_csv_source
: "(" "gnf_columns" export_csv_column* ")"
construct: $$ = transactions.ExportCSVSource(gnf_columns=transactions.ExportCSVColumns(columns=$3))
deconstruct if builtin.has_proto_field($$, 'gnf_columns'):
$3: Sequence[transactions.ExportCSVColumn] = $$.gnf_columns.columns
That means that there's no grammar rule that actually produces an ExportCSVColumns object on its own. And no, i think this is independent of the optional thing
| ExportCSVSource csv_source = 10; | ||
| CSVConfig csv_config = 11; | ||
|
|
||
| optional int64 partition_size = 3; |
There was a problem hiding this comment.
| optional int64 partition_size = 3; | |
| // Below are all deprecated in favour of the `csv_config` above. | |
| optional int64 partition_size = 3; |
comnik
left a comment
comnik
left a comment
There was a problem hiding this comment.
Minor question, but looks good!
3 participants
Add export_csv_config_v2 with csv_source and table_def support
Adds grammar and printer support for the
export_csv_config_v2format, which uses the existingExportCSVConfigproto fieldscsv_sourceandcsv_configto provide a cleaner separation between data source specification and CSV formatting options.Protobuf Structure
ExportCSVConfignow supports two patterns:data_columns(repeated ExportCSVColumn) with inline syntax_* and compression optionscsv_source(ExportCSVSource) andcsv_config(CSVConfig), where:csv_sourceis a oneof containing eithertable_deforgnf_columnscsv_configis a CSVConfig object with csv_* prefixed options (header_row, delimiter, compression, etc.)Changes
export_csv_config_v2parser/printer rules using csv_source and csv_configtable_defandgnf_columnsin the v2 formatAlso renamed
CSVConfig.partition_size→partition_size_mbfor clarity.(thanks Claude for the description 😄)