Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support backwards compatible / un prefixed config options (compression rather than parquet.compression) #9575

Closed
Tracked by #9682
alamb opened this issue Mar 12, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@alamb
Copy link
Contributor

alamb commented Mar 12, 2024

Is your feature request related to a problem or challenge?

Before #9382, DataFusion write configuration was inconsistent and different than the main configuration system and #9382 rationalized it ❤️

However, it also broke compatibility with old configuration and introduced some redundancy that can be annoying, as @devinjdangelo noted #9382 (review)

For example, to specify the compression type when writing parquet, after #9382 one needs to repeat 'parquet' to namespace the configuration options (format parquet and 'parquet.compression')

DataFusion CLI v36.0.0
❯ COPY (values (1)) TO 'test_files/scratch/copy/table/' (format parquet, 'compression' 'zstd(10)');

Invalid or Unsupported Configuration: could not find config namespace for key "compression"
❯ COPY (values (1)) TO 'test_files/scratch/copy/table/' (format parquet, 'parquet.compression' 'zstd(10)');
+-------+
| count |
+-------+
| 1     |
+-------+
1 row in set. Query took 0.024 seconds.

Describe the solution you'd like

I would like to introduce some backwards compatibility so when I write

COPY source_table TO 'test_files/scratch/copy/table/' (format parquet, 'compression' 'zstd(10)');

It means the same as

COPY source_table TO 'test_files/scratch/copy/table/' (format parquet, 'parquet.compression' 'zstd(10)');

Describe alternatives you've considered

I suggest we add some sort of special case code that tried to parse the option as normal, and if that failed, try with the different formats / prefixes

For example, if compression wasn't a valid option,

Additional context

No response

@alamb alamb added enhancement New feature or request good first issue Good for newcomers labels Mar 12, 2024
@alamb
Copy link
Contributor Author

alamb commented Mar 12, 2024

I think this would be a good first issue as it should be relatively straightforward and is a user visible behavior change.

The high level would be

  1. Add a sqllogictest that reproduced the problem
  2. Add the relevant special case code

@alamb alamb changed the title Support un prefixed config options (compression rather than parquet.compression) Support backwards compatible / un prefixed config options (compression rather than parquet.compression) Mar 12, 2024
@tinfoil-knight
Copy link
Contributor

take

@alamb
Copy link
Contributor Author

alamb commented Mar 20, 2024

A subsequent PR renamed the prefix from parquet to format -- #9716 tracks removing the need to add format

Thanks for the analysis @tinfoil-knight #9594 (comment)

@alamb alamb closed this as completed Mar 20, 2024
@alamb alamb reopened this Mar 20, 2024
@alamb alamb closed this as not planned Won't fix, can't repro, duplicate, stale Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants