aminghadersohi commented on code in PR #37411:
URL: https://github.com/apache/superset/pull/37411#discussion_r2770979175
##########
superset/sql/execution/executor.py:
##########
@@ -684,6 +696,34 @@ def _check_disallowed_functions(self, script: SQLScript)
-> set[str] | None:
return found if found else None
+ def _check_disallowed_tables(self, script: SQLScript) -> set[str] | None:
+ """
+ Check for disallowed SQL tables/views.
+
+ :param script: Parsed SQL script
+ :returns: Set of disallowed tables found, or None if none found
+ """
+ disallowed_config = app.config.get("DISALLOWED_SQL_TABLES", {})
+ engine_name = self.database.db_engine_spec.engine
+
+ # Get disallowed tables for this engine
+ engine_disallowed = disallowed_config.get(engine_name, set())
+ if not engine_disallowed:
+ return None
+
+ # Use AST-based table detection
+ if script.check_tables_present(engine_disallowed):
+ # Return the specific tables that were found
+ found = set()
+ for statement in script.statements:
+ present = {table.table.lower() for table in statement.tables}
+ for table in engine_disallowed:
+ if table.lower() in present:
+ found.add(table)
+ return found or set(engine_disallowed)
+
+ return None
Review Comment:
Addressed — replaced the double traversal with a single-pass implementation
that iterates `script.statements` once and collects disallowed tables directly.
This eliminates both the performance overhead and the theoretical mismatch
between `check_tables_present` and the secondary loop.
--
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]
