-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
adding config to control Varchar behavior #11090
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me -- thank you @Lordworms
I had a suggestion for a different parameter name, but I don't think that is required
datafusion/common/src/config.rs
Outdated
/// type with a specified length. This can be useful for enforcing certain schema | ||
/// constraints or maintaining compatibility with systems that do not support | ||
/// length-specified `Varchar` types. | ||
pub error_on_varchar_with_length: bool, default = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be easier to understand to avoid the double negative here
So perhaps the config flag could be support_varchar_with_length
and default to true
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, I'll fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @Lordworms
I tweaked the config documentation a little and added a new test.
This looks great
* adding config to control Varchar behavior * fix failed tests * fix config_md * format md * optimize code * format md * format md * adding config * Tweak documentation * Update sqllogictest * tweaks strings --------- Co-authored-by: Andrew Lamb <[email protected]>
* adding config to control Varchar behavior * fix failed tests * fix config_md * format md * optimize code * format md * format md * adding config * Tweak documentation * Update sqllogictest * tweaks strings --------- Co-authored-by: Andrew Lamb <[email protected]>
Which issue does this PR close?
Closes #10743
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?