minor: Improve error message for invalid column expression in SELECT statement#22486
Conversation
github-actions
Bot
added
sql
| // lookup with unqualified name "t1.c0" | ||
| let err = schema.index_of_column(&col).unwrap_err(); | ||
| let expected = "Schema error: No field named \"t1.c0\". \ | ||
| Column names are case sensitive. \ |
There was a problem hiding this comment.
Now this case sensitive hint only shows up, if the input expr has some case-insensitive match.
nuno-faria
left a comment
nuno-faria
left a comment
There was a problem hiding this comment.
Thanks @2010YOUY01, makes sense to me. I leave some suggestions below.
| "\nColumn names are case sensitive. You can use double quotes to refer to the {} column \ | ||
| or set the datafusion.sql_parser.enable_ident_normalization configuration.", | ||
| case_sensitive_match.quoted_flat_name() |
There was a problem hiding this comment.
I know this is not new, but should it say "... or disable the datafusion.sql_parser.enable_ident_normalization configuration."? I think "set" makes it sound like we need to enable the config.
| let field_flat_name = field.flat_name(); | ||
| let field_name_lower = field.name().to_lowercase(); | ||
| let field_flat_name_lower = field_flat_name.to_lowercase(); | ||
|
|
||
| valid_fields.iter().find(|valid_field| { | ||
| let valid_field_flat_name = valid_field.flat_name(); | ||
| let valid_field_name_lower = valid_field.name().to_lowercase(); | ||
| let valid_field_flat_name_lower = valid_field_flat_name.to_lowercase(); | ||
|
|
||
| let name_differs_only_by_case = field_name_lower == valid_field_name_lower | ||
| && field.name() != valid_field.name(); | ||
| let flat_name_differs_only_by_case = field_flat_name_lower | ||
| == valid_field_flat_name_lower | ||
| && field_flat_name != valid_field_flat_name; | ||
|
|
||
| name_differs_only_by_case || flat_name_differs_only_by_case |
There was a problem hiding this comment.
nit: For consistency, the "_flat_name" variables could be removed, or we could add variables for the .name() values.
| let distance = levenshtein(target, valid_name); | ||
| let max_len = target.chars().count().max(valid_name.chars().count()); | ||
| if max_len == 0 || distance * 2 > max_len { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Would it make sense to add a small comment here to explain the reasoning behind the formula?
|
Thank you for the review @nuno-faria ! All of them make sense, have updated in 934ffa9 |
2 participants
Which issue does this PR close?
Rationale for this change
User friendly 'did you mean' error message is missing for some cases, this PR fixes it.
Demo in
datafusion-cliWhat changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?