rusackas commented on code in PR #42598:
URL: https://github.com/apache/superset/pull/42598#discussion_r3707431317


##########
superset/sql/parse.py:
##########
@@ -1996,6 +1996,22 @@ def extract_tables_from_statement(
     }
 
 
+def count_referenced_tables(statement: str, dialect: Dialects | str | None) -> 
int:
+    """
+    Count the distinct tables referenced by a raw SQL string.
+
+    Falls back to a conservative count of 1 (i.e. "not multi-table") if the
+    statement can't be parsed, since callers gating multi-table-only behavior
+    on this count should default to treating an unparseable statement as a
+    single table.
+    """
+    try:
+        parsed = sqlglot.parse_one(statement, dialect=dialect)
+        return len(extract_tables_from_statement(parsed, dialect))

Review Comment:
   Good catch, fixed! Added the `_check_script_length` call before 
`sqlglot.parse_one` in `count_referenced_tables`, matching every other call 
site in the module. Pushed a regression test pinning the fallback to 1 when the 
statement is over the cap.



##########
superset/extensions/metadb.py:
##########
@@ -70,6 +73,25 @@
 from superset import db, feature_flag_manager, security_manager
 from superset.sql.parse import Table
 
+# Counts references to `superset://` virtual tables in the statement being
+# executed against the engine. Those tables are always addressed as
+# double-quoted `database[[.catalog].schema].table` identifiers (see the
+# dialect docstring below), since the literal dot(s) would otherwise be
+# parsed as a schema/catalog separator, so this also catches multi-table
+# statements that don't use the `JOIN` keyword, e.g. an implicit comma join
+# like `FROM "database1.table1", "database2.table2" WHERE ...`. Shillelagh
+# calls `SupersetShillelaghAdapter.get_data` once per underlying table,
+# independently of any other table referenced by the same statement, so it
+# has no way on its own to tell whether it's being asked for a standalone
+# table or for one side of a multi-table query.
+# `SupersetAPSWDialect.do_execute*` populates `_executing_multi_table_query`
+# for the duration of a statement so that `get_data` can tell the two cases
+# apart (see `get_data` for why this matters).
+_TABLE_REF_RE = re.compile(r'"[^"]*\.[^"]*"')

Review Comment:
   This one's moot now. The regex is gone, `count_referenced_tables` parses 
with sqlglot instead, so a "fake.table" string sitting inside a comment just 
isn't a table reference to the parser anymore.



-- 
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]

Reply via email to