-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Conversation
def split_table_and_column_names(table: str) -> Iterable[str]: | ||
|
||
partitioned = [x for x in table.partition(".") if x != ''] | ||
if partitioned[0].isnumeric() and partitioned[-1].isnumeric(): |
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.
What case is this handling? I'm having a hard time guessing what this logic accomplishes without having looked at the data.
previous_token = sql_tokens[0] | ||
for (token, next_token) in zip(sql_tokens[1:-1], sql_tokens[2:]): | ||
if token == "AS" and previous_token is not None: | ||
table_name = next_token[:-6] |
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.
This is removing the "aliasX" bit, right? Some comment about that would be nice here (as a comment or a docstring).
'=', 'LOCATION', '.', 'RESTAURANT_ID', 'AND', 'RESTAURANT', '.', 'NAME', '=', "'name0'", ';'] | ||
|
||
# Check we don't mangle decimal numbers: | ||
assert text2sql_utils.clean_unneeded_aliases(["2.5"]) == ["2.5"] |
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.
Ah, I see, that bit above makes sense now. A brief comment would be nice.
cleaned = text2sql_utils.clean_unneeded_aliases(sql) | ||
assert cleaned == ['SELECT', 'COUNT', '(', '*', ')', 'FROM', 'LOCATION', ',', 'RESTAURANT', 'WHERE', | ||
'LOCATION', '.', 'CITY_NAME', '=', "'city_name0'", 'AND', 'RESTAURANT', '.', 'ID', | ||
'=', 'LOCATION', '.', 'RESTAURANT_ID', 'AND', 'RESTAURANT', '.', 'NAME', '=', "'name0'", ';'] |
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's probably worth adding a test here with a case where you shouldn't remove the "AS", just to be sure we handle those correctly.
AS
statements to the scriptTABLE.COLUMN -> TABLE . COLUMN