feat: support parsing for parquet writer option #4938
Conversation
Signed-off-by: fan <[email protected]>
Signed-off-by: fan <[email protected]>
🫣i will add some tests later. |
Signed-off-by: fan <[email protected]>
| match "plain_xxx".parse::<Encoding>() { | ||
| Ok(e) => { | ||
| panic!("Should not be able to parse {:?}", e); | ||
| } | ||
| Err(e) => { | ||
| assert_eq!(e.to_string(), "Parquet error: unknown encoding: plain_xxx"); | ||
| } | ||
| } |
There was a problem hiding this comment.
| match "plain_xxx".parse::<Encoding>() { | |
| Ok(e) => { | |
| panic!("Should not be able to parse {:?}", e); | |
| } | |
| Err(e) => { | |
| assert_eq!(e.to_string(), "Parquet error: unknown encoding: plain_xxx"); | |
| } | |
| } | |
| let err = "plain_xxx".parse::<Encoding>().unwrap_err(); | |
| assert_eq!(e.to_string(), "Parquet error: unknown encoding: plain_xxx"); |
parquet/src/basic.rs
Outdated
| assert_eq!(compress, Compression::LZ4); | ||
|
|
||
| // test unknown compression | ||
| match "unknown".parse::<Compression>() { |
There was a problem hiding this comment.
These could also make use of unwrap_err()
parquet/src/basic.rs
Outdated
| type Err = ParquetError; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| match s.to_owned().to_uppercase().as_str() { |
There was a problem hiding this comment.
I wonder if we should be case sensitive and leave it to the caller to force to upper case if that is what they want?
It seems strange that we would support parsing things like PlAiN
There was a problem hiding this comment.
It seems strange that we would support parsing things like
PlAiN
For users, whose usage is generally unobservable, I also find it odd to support this kind of parsing like PlAiN, but it seems that both cases should be supported?
There was a problem hiding this comment.
I think we should just support case sensitive parsing and the user can opt in to case insensitive parsing by converting the input to uppercase should they want this behaviour
There was a problem hiding this comment.
I think we should just support case sensitive parsing and the user can opt in to case insensitive parsing by converting the input to uppercase should they want this behaviour
done. i think it's a good suggestion.
Signed-off-by: fan <[email protected]>
Signed-off-by: fan <[email protected]>
fansehep
force-pushed
the
parquet_encoding
branch
from
3b9e331 to
1cc87ec
Compare
|
Thank you |
Which issue does this PR close?
part #4693
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?