Inconsistent formatting in errors for config options

[qrilka]

Describe the bug

I was trying to set parquet writer option to 2.0 in datafusion-cli and got the following confusing error message:

❯ copy src_table to 'table.parquet' (format parquet,writer_version '2.0');
Invalid or Unsupported Configuration: Unknown or unsupported parquet writer version '2.0' valid options are '1.0' and '2.0'

It appears that I could use 2.0 without quotes and with double quotes like "2.0" but not with single quotes as the error message reports the values.
Probably the error message should use double quotes instead.

To Reproduce

Use incorrect value for writer_version option in COPY TO using parquet format

Expected behavior

Error message should not be confusing

Additional context

No response

[qrilka]

If switching from single quotes to double quotes is accepted as a fix I could create a PR doing that.

[alamb]

Maybe there should simply be no quotes for values that are supposed to be numeric?

[alamb]

Perhaps @devinjdangelo has some thoughts

[devinjdangelo]

You can specify writer_version and in fact almost all options without quotes and I would favor that as the preferred way to specify them (see below). There are some special cases where datafusion's options parser relies on special characters that sqlparser-rs will complain about if it is not wrapped in single quotes. Notably compression options which contain '(' and ')' as well as column specific options using '::'. We could potentially make the UX in those cases nicer by modifying sqlparser-rs to accept those special syntax.

I think cleaning up the error messages and documentation to make all of this more clear is a good idea!

# Copy parquet with all supported statment overrides
query IT
COPY source_table 
TO 'test_files/scratch/copy/table_with_options' 
(format parquet,
single_file_output false,
compression snappy,
'compression::col1' 'zstd(5)',
'compression::col2' snappy,
max_row_group_size 12345,
data_pagesize_limit 1234,
write_batch_size 1234,
writer_version 2.0,
dictionary_page_size_limit 123,
created_by 'DF copy.slt',
column_index_truncate_length 123,
data_page_row_count_limit 1234,
bloom_filter_enabled true,
'bloom_filter_enabled::col1' false,
'bloom_filter_fpp::col2' 0.456,
'bloom_filter_ndv::col2' 456,
encoding plain,
'encoding::col1' DELTA_BINARY_PACKED,
'dictionary_enabled::col2' true,
dictionary_enabled false,
statistics_enabled page,
'statistics_enabled::col2' none,
max_statistics_size 123,
bloom_filter_fpp 0.001,
bloom_filter_ndv 100
)
----
2

[qrilka]

@devinjdangelo thanks, and what is the proposal then? Not use quotes when they are nor required and single quotes otherwise?

[devinjdangelo]

@devinjdangelo thanks, and what is the proposal then? Not use quotes when they are nor required and single quotes otherwise?

Yes, that is what I would propose.