Detect invalid (unsupported) compression types when parsing #4637
Conversation
Signed-off-by: remzi <13716567376yh@gmail.com>
github-actions
bot
added
core
| /// Gzip-ed file | ||
| pub const GZIP: Self = Self { variant: GZIP }; | ||
|
|
||
| /// Bzip2-ed file | ||
| pub const BZIP2: Self = Self { variant: BZIP2 }; | ||
|
|
||
| /// Xz-ed file (liblzma) | ||
| pub const XZ: Self = Self { variant: XZ }; | ||
|
|
||
| /// Uncompressed file | ||
| pub const UNCOMPRESSED: Self = Self { | ||
| variant: UNCOMPRESSED, | ||
| }; |
There was a problem hiding this comment.
They are a little tricky, but can minimize the code change.
| @@ -66,7 +63,7 @@ pub struct CreateExternalTable { | |||
| /// Option to not error if table already exists | |||
| pub if_not_exists: bool, | |||
| /// File compression type (GZIP, BZIP2, XZ) | |||
| pub file_compression_type: String, | |||
| pub file_compression_type: CompressionTypeVariant, | |||
There was a problem hiding this comment.
Strengthening the type can let us find errors earlier.
There was a problem hiding this comment.
Looks good to me -- thank you @HaoYang670
The clippy CI check has failed on this PR, but I think that is because a new version of Rust was released -- if you merge from main it should be solved I think
| @@ -1487,7 +1488,7 @@ pub struct CreateExternalTable { | |||
| /// SQL used to create the table, if available | |||
| pub definition: Option<String>, | |||
| /// File compression type (GZIP, BZIP2, XZ) | |||
| pub file_compression_type: String, | |||
| pub file_compression_type: CompressionTypeVariant, | |||
| let sql = "CREATE EXTERNAL TABLE t STORED AS CSV COMPRESSION TYPE ZZZ LOCATION 'blahblah'"; | ||
| expect_parse_error( | ||
| sql, | ||
| "sql parser error: Unsupported file compression type ZZZ", |
|
I took the liberty of merging up from master to resolve another conflict and plan to merge this PR when CI passes |
|
Thanks @alamb ! |
|
Benchmark runs are scheduled for baseline = 067d044 and contender = c2f199a. c2f199a is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #4636.
Rationale for this change
Before the change:
After the change:
What changes are included in this PR?
Are these changes tested?
Yes
Are there any user-facing changes?