Add column name suggestions to presto validator #1330
Conversation
| class PrestoOptimizingValidator(BaseQueryValidator): | ||
| def languages(self): | ||
| return ["presto", "trino"] | ||
| class ColumnNameSuggester(BasePrestoSQLGlotDecorator): |
There was a problem hiding this comment.
I guess column name suggester is not specific to presto, other engines, liek sparksql can also benefit from it?
There was a problem hiding this comment.
Refactored so that we can use it with other query engines
|
|
||
| return validation_suggestions | ||
| def _get_column_name_from_position( |
There was a problem hiding this comment.
why not just get the column name from the error message "Column .* cannot be resolved"?
There was a problem hiding this comment.
Updated to use that regex
| ) -> List[QueryValidationResult]: | ||
| return self._get_explain_validator().validate(query, uid, engine_id) | ||
| def _search_columns_for_suggestion(self, columns: List[str], suggestion: str): | ||
| """Return the case-sensitive column name by searching the table's columns for the suggestion text""" |
There was a problem hiding this comment.
isn't the highlighted column name always one of the columns? wondering if this function is necessary.
There was a problem hiding this comment.
The highlighted column is always lowercase, this makes sure we get the case-sensitive version of the column name
| return PrestoExplainValidator("") | ||
|
|
||
| def _get_decorated_validator(self) -> BaseQueryValidator: | ||
| return UnionAllValidator( |
There was a problem hiding this comment.
what's the reason of changing from a list of validators to a chain of validators?
There was a problem hiding this comment.
Using the decorator pattern so that we can add suggestions on top of the validation messages
querybook/server/lib/query_analysis/validation/validators/metadata_suggesters.py
Outdated
Show resolved
Hide resolved
* Add table & column name suggestions to presto validator
Add column name suggestions to presto optimizing validator: