Fix(lsp): Extend support for table references with columns #4763
Conversation
themisvaltinos
changed the title
sqlmesh/lsp/reference.py
Outdated
| The Range covering the table reference in the column | ||
| """ | ||
|
|
||
| table_parts = column.parts[:-1] if len(column.parts) > 1 else [column.parts[0]] |
There was a problem hiding this comment.
Not sure about the else branch here. If there's only one part in a Column instance, isn't that the column name and not a table?
There was a problem hiding this comment.
actually yes there's no need for it altogether since if we're here it would have more than one part so table_parts = column.parts[:-1] will be enough
sqlmesh/lsp/reference.py
Outdated
| table_parts = [part.name for part in column.parts[:-1]] | ||
| table_ref = ".".join(table_parts) | ||
| normalized_reference_name = normalize_model_name( | ||
| table_ref, | ||
| default_catalog=default_catalog, | ||
| dialect=dialect, | ||
| ) |
There was a problem hiding this comment.
This section seems a little sketchy: part.name gets rid of the quotes, so normalize_model_name will normalize even case-sensitive (quoted) identifiers. Has this been tested / taken into account?
There was a problem hiding this comment.
yes the reference_name it compares it too has gone through the same normalisation in get_model_definitions_for_a_path this is why, but I will test further with more edge cases to ensure
There was a problem hiding this comment.
revised it to use .sql(0 instead which is used in get_model_definitions_for_a_path as well and added a test for it : test_quoted_uppercase_table_and_column_references
themisvaltinos
force-pushed
the
themis/refere
branch
from
6ab35c4 to
12953f5
Compare
themisvaltinos
force-pushed
the
themis/refere
branch
from
12953f5 to
fd45c56
Compare
This adds support to detect table references in column expressions for models and ctes (e.g. sushi.orders.customer_id). (needed for: #4718)