villebro commented on code in PR #21535:
URL: https://github.com/apache/superset/pull/21535#discussion_r1188528666
##########
superset/sql_parse.py:
##########
@@ -187,6 +187,14 @@ def __eq__(self, __o: object) -> bool:
class ParsedQuery:
+ class TableTuple(NamedTuple):
+ table: Table
+ level: int
+
+ class AliasTuple(NamedTuple):
+ name: str
+ level: int
Review Comment:
Nit: while I'm having a hard time coming up with a better name for these,
I'm not a big fan of Hungarian notation. An alternative could be
`TableWithLevel` and `AliasWithLevel` to clarify the contents rather than the
type.
##########
superset/sql_parse.py:
##########
@@ -203,13 +211,32 @@ def __init__(self, sql_statement: str, strip_comments:
bool = False):
@property
def tables(self) -> Set[Table]:
+ def deepest_alias_level(alias_name: str) -> int:
+ alias_entries = [a for a in alias_name_tuples if str(a.name) ==
alias_name]
+ if len(alias_entries) > 0:
+ return max(alias_entries, key=lambda a: a.level).level
+ return -1
+
if not self._tables:
for statement in self._parsed:
- self._extract_from_token(statement)
-
- self._tables = {
- table for table in self._tables if str(table) not in
self._alias_names
- }
+ result = self._extract_from_token(statement)
+ if result:
+ (table_tuples, alias_name_tuples) = result
Review Comment:
Continuing my naming rant.. 😄 Here we coud probably just call then `tables`
and `aliases`, and then later extract the `table_names`/`table_levels` from
them etc.
##########
tests/unit_tests/sql_parse_tests.py:
##########
@@ -525,6 +558,16 @@ def test_extract_tables_reusing_aliases() -> None:
with q1 as ( select key from q2 where key = '5'),
q2 as ( select key from src where key = '5')
select * from (select key from q1) a
+"""
+ )
+ == {Table("q2"), Table("src")}
+ )
Review Comment:
are we really sure that the `q2` reference in the `q1` CTE will in fact
reference the table `q2` in the database? I did the following test on my devenv
which has examples data and the superset metadata in the same database, and
```sql
with q1 as ( select distinct gender from birth_names),
birth_names as ( select * from ab_role)
select * from (select gender from q1) a
```
produces the following error:
> sqlite error: no such column: gender
So to me it appears as if only `src` is in fact referenced here, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
