Copilot commented on code in PR #39389:
URL: https://github.com/apache/superset/pull/39389#discussion_r3190525766
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
+
+ @classmethod
+ def _parse_statement(
+ cls,
+ statement: str,
+ engine: str,
+ ) -> str:
+ if engine != "firebird":
+ raise SupersetParseError(
+ statement,
+ engine,
+ message=f"Invalid engine: {engine}",
+ )
+
+ statements = _FIREBIRD_SEMI_SPLIT_RE.split(statement)
+ statements = [s.strip() for s in statements if s.strip()]
+ if len(statements) != 1:
+ raise SupersetParseError(
+ statement,
+ engine,
+ message="FirebirdStatement should have exactly one statement",
+ )
+ return statements[0]
+
+ @classmethod
+ def _extract_tables_from_statement(
+ cls,
+ parsed: str,
+ engine: str,
+ ) -> set[Table]:
+ """
+ Extract table references from a Firebird SQL statement.
+
+ Since we cannot reliably parse Firebird SQL without a proper parser,
+ we return an empty set and log a warning. This means that data-access
+ roles will not be enforced by Superset for Firebird databases.
+ """
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table extraction "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return set()
+
+ def format(self, comments: bool = True) -> str:
+ """Return the SQL statement as-is (no AST reformatting)."""
+ return self._parsed.strip()
+
+ def get_settings(self) -> dict[str, str | bool]:
+ """Return statement-level settings. Firebird has none."""
+ return {}
+
+ def is_select(self) -> bool:
+ """Check whether the statement is a SELECT-style query."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... SELECT is a select; WITH ... INSERT/DELETE/etc. is not.
+ return not bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word == "SELECT"
+
+ def is_mutating(self) -> bool:
+ """Check whether the statement performs a mutating operation."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... INSERT/DELETE/UPDATE is mutating.
+ return bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word in _FIREBIRD_MUTATING_KEYWORDS
+
+ def optimize(self) -> FirebirdStatement:
+ """Return self — no AST-level optimisation is possible."""
+ return FirebirdStatement(ast=self._parsed, engine=self.engine)
+
+ def check_functions_present(self, functions: set[str]) -> bool:
+ """Check whether any of the given function names appear in the SQL."""
+ upper = self._parsed.upper()
+ return any(f.upper() in upper for f in functions)
+
+ def check_tables_present(self, tables: set[str]) -> bool:
+ """Check if any of the given tables are present in the statement."""
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table checking "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return False
+
+ def get_limit_value(self) -> int | None:
+ """
+ Extract the ``FIRST <n>`` limit from the statement.
+
+ >>> FirebirdStatement("SELECT FIRST 10 * FROM t").get_limit_value()
+ 10
+ >>> FirebirdStatement("SELECT * FROM t").get_limit_value() is None
+ True
+ """
+ match = _FIREBIRD_FIRST_RE.search(self._parsed)
+ return int(match.group(1)) if match else None
+
+ def set_limit_value(
+ self,
+ limit: int,
+ method: LimitMethod = LimitMethod.FORCE_LIMIT,
+ ) -> None:
+ """
+ Set (or replace) the ``FIRST <n>`` clause.
+
+ >>> stmt = FirebirdStatement("SELECT FIRST 1000 * FROM t")
+ >>> stmt.set_limit_value(10)
+ >>> stmt.format()
+ 'SELECT FIRST 10 * FROM t'
+
+ >>> stmt = FirebirdStatement("SELECT * FROM t")
+ >>> stmt.set_limit_value(10)
+ >>> stmt.format()
+ 'SELECT FIRST 10 * FROM t'
+ """
+ if method == LimitMethod.FETCH_MANY:
+ return
+ if method not in (LimitMethod.FORCE_LIMIT, LimitMethod.WRAP_SQL):
+ raise SupersetParseError(
+ self._parsed,
+ self.engine,
+ message=f"Unsupported limit method for Firebird: {method}",
+ )
+
+ match = _FIREBIRD_FIRST_RE.search(self._parsed)
+ if match:
+ self._parsed = (
+ self._parsed[: match.start()]
+ + f"FIRST {limit}"
+ + self._parsed[match.end() :]
+ )
+ else:
+ self._parsed = re.sub(
+ r"(?i)^(\s*SELECT)\b",
+ rf"\1 FIRST {limit}",
+ self._parsed,
+ count=1,
+ )
+
+ def parse_predicate(self, predicate: str) -> str:
+ """Return the predicate unchanged (no AST rewriting for Firebird)."""
+ return predicate
+
+ def apply_rls(
+ self,
+ catalog: str | None,
+ schema: str | None,
+ predicates: dict[Table, list[str]],
+ method: RLSMethod,
+ ) -> None:
+ """Apply RLS rules. Not supported for Firebird (no AST)."""
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — RLS rewriting "
+ "disabled; row-level security will not be enforced by Superset."
Review Comment:
This implementation explicitly disables table extraction and RLS rewriting,
which the docstrings/logs state will cause data-access roles and row-level
security to not be enforced for Firebird. That’s a fail-open security posture
(users may be able to query data without the usual security constraints).
Consider failing closed instead (e.g., raise an error when table extraction /
RLS enforcement is required, or gate this behavior behind a feature flag/config
that defaults to off). Also consider reducing repeated warning logs (these
paths may be hit frequently in SQL Lab), but the primary concern is ensuring
security controls are not bypassed.
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
+
+ @classmethod
+ def _parse_statement(
+ cls,
+ statement: str,
+ engine: str,
+ ) -> str:
+ if engine != "firebird":
+ raise SupersetParseError(
+ statement,
+ engine,
+ message=f"Invalid engine: {engine}",
+ )
+
+ statements = _FIREBIRD_SEMI_SPLIT_RE.split(statement)
+ statements = [s.strip() for s in statements if s.strip()]
+ if len(statements) != 1:
+ raise SupersetParseError(
+ statement,
+ engine,
+ message="FirebirdStatement should have exactly one statement",
+ )
+ return statements[0]
+
+ @classmethod
+ def _extract_tables_from_statement(
+ cls,
+ parsed: str,
+ engine: str,
+ ) -> set[Table]:
+ """
+ Extract table references from a Firebird SQL statement.
+
+ Since we cannot reliably parse Firebird SQL without a proper parser,
+ we return an empty set and log a warning. This means that data-access
+ roles will not be enforced by Superset for Firebird databases.
+ """
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table extraction "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return set()
Review Comment:
This implementation explicitly disables table extraction and RLS rewriting,
which the docstrings/logs state will cause data-access roles and row-level
security to not be enforced for Firebird. That’s a fail-open security posture
(users may be able to query data without the usual security constraints).
Consider failing closed instead (e.g., raise an error when table extraction /
RLS enforcement is required, or gate this behavior behind a feature flag/config
that defaults to off). Also consider reducing repeated warning logs (these
paths may be hit frequently in SQL Lab), but the primary concern is ensuring
security controls are not bypassed.
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
Review Comment:
This regex doesn’t correctly handle escaped single quotes inside Firebird
string literals (Firebird uses doubled quotes: `''`). For example, `SELECT
'a'';b'; SELECT 1` can be split incorrectly because the quote-counting
heuristic treats the escaped quote as a terminator. Recommend switching to a
small state-machine splitter that scans characters and understands `''` escapes
(and ideally also comment forms), rather than relying on this regex.
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
+
+ @classmethod
+ def _parse_statement(
+ cls,
+ statement: str,
+ engine: str,
+ ) -> str:
+ if engine != "firebird":
+ raise SupersetParseError(
+ statement,
+ engine,
+ message=f"Invalid engine: {engine}",
+ )
+
+ statements = _FIREBIRD_SEMI_SPLIT_RE.split(statement)
+ statements = [s.strip() for s in statements if s.strip()]
+ if len(statements) != 1:
+ raise SupersetParseError(
+ statement,
+ engine,
+ message="FirebirdStatement should have exactly one statement",
+ )
+ return statements[0]
+
+ @classmethod
+ def _extract_tables_from_statement(
+ cls,
+ parsed: str,
+ engine: str,
+ ) -> set[Table]:
+ """
+ Extract table references from a Firebird SQL statement.
+
+ Since we cannot reliably parse Firebird SQL without a proper parser,
+ we return an empty set and log a warning. This means that data-access
+ roles will not be enforced by Superset for Firebird databases.
+ """
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table extraction "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return set()
+
+ def format(self, comments: bool = True) -> str:
+ """Return the SQL statement as-is (no AST reformatting)."""
+ return self._parsed.strip()
+
+ def get_settings(self) -> dict[str, str | bool]:
+ """Return statement-level settings. Firebird has none."""
+ return {}
+
+ def is_select(self) -> bool:
+ """Check whether the statement is a SELECT-style query."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... SELECT is a select; WITH ... INSERT/DELETE/etc. is not.
+ return not bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
Review Comment:
Both `is_select` and `is_mutating` build a regex pattern dynamically on
every call (`'|'.join(...)`). This is avoidable overhead and also relies on set
iteration order. Recommend precompiling a single `_FIREBIRD_MUTATING_RE =
re.compile(...)` once (using a stable ordering like `sorted(...)`) and reusing
it in both methods.
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
+
+ @classmethod
+ def _parse_statement(
+ cls,
+ statement: str,
+ engine: str,
+ ) -> str:
+ if engine != "firebird":
+ raise SupersetParseError(
+ statement,
+ engine,
+ message=f"Invalid engine: {engine}",
+ )
+
+ statements = _FIREBIRD_SEMI_SPLIT_RE.split(statement)
+ statements = [s.strip() for s in statements if s.strip()]
+ if len(statements) != 1:
+ raise SupersetParseError(
+ statement,
+ engine,
+ message="FirebirdStatement should have exactly one statement",
+ )
+ return statements[0]
+
+ @classmethod
+ def _extract_tables_from_statement(
+ cls,
+ parsed: str,
+ engine: str,
+ ) -> set[Table]:
+ """
+ Extract table references from a Firebird SQL statement.
+
+ Since we cannot reliably parse Firebird SQL without a proper parser,
+ we return an empty set and log a warning. This means that data-access
+ roles will not be enforced by Superset for Firebird databases.
+ """
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table extraction "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return set()
+
+ def format(self, comments: bool = True) -> str:
+ """Return the SQL statement as-is (no AST reformatting)."""
+ return self._parsed.strip()
+
+ def get_settings(self) -> dict[str, str | bool]:
+ """Return statement-level settings. Firebird has none."""
+ return {}
+
+ def is_select(self) -> bool:
+ """Check whether the statement is a SELECT-style query."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... SELECT is a select; WITH ... INSERT/DELETE/etc. is not.
+ return not bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word == "SELECT"
+
+ def is_mutating(self) -> bool:
+ """Check whether the statement performs a mutating operation."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... INSERT/DELETE/UPDATE is mutating.
+ return bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word in _FIREBIRD_MUTATING_KEYWORDS
+
+ def optimize(self) -> FirebirdStatement:
+ """Return self — no AST-level optimisation is possible."""
+ return FirebirdStatement(ast=self._parsed, engine=self.engine)
+
+ def check_functions_present(self, functions: set[str]) -> bool:
+ """Check whether any of the given function names appear in the SQL."""
+ upper = self._parsed.upper()
+ return any(f.upper() in upper for f in functions)
+
+ def check_tables_present(self, tables: set[str]) -> bool:
+ """Check if any of the given tables are present in the statement."""
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table checking "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return False
+
+ def get_limit_value(self) -> int | None:
+ """
+ Extract the ``FIRST <n>`` limit from the statement.
+
+ >>> FirebirdStatement("SELECT FIRST 10 * FROM t").get_limit_value()
+ 10
+ >>> FirebirdStatement("SELECT * FROM t").get_limit_value() is None
+ True
+ """
+ match = _FIREBIRD_FIRST_RE.search(self._parsed)
+ return int(match.group(1)) if match else None
+
+ def set_limit_value(
+ self,
+ limit: int,
+ method: LimitMethod = LimitMethod.FORCE_LIMIT,
+ ) -> None:
+ """
+ Set (or replace) the ``FIRST <n>`` clause.
+
+ >>> stmt = FirebirdStatement("SELECT FIRST 1000 * FROM t")
+ >>> stmt.set_limit_value(10)
+ >>> stmt.format()
+ 'SELECT FIRST 10 * FROM t'
+
+ >>> stmt = FirebirdStatement("SELECT * FROM t")
+ >>> stmt.set_limit_value(10)
+ >>> stmt.format()
+ 'SELECT FIRST 10 * FROM t'
+ """
+ if method == LimitMethod.FETCH_MANY:
+ return
+ if method not in (LimitMethod.FORCE_LIMIT, LimitMethod.WRAP_SQL):
+ raise SupersetParseError(
+ self._parsed,
+ self.engine,
+ message=f"Unsupported limit method for Firebird: {method}",
+ )
+
+ match = _FIREBIRD_FIRST_RE.search(self._parsed)
+ if match:
+ self._parsed = (
+ self._parsed[: match.start()]
+ + f"FIRST {limit}"
+ + self._parsed[match.end() :]
+ )
+ else:
+ self._parsed = re.sub(
+ r"(?i)^(\s*SELECT)\b",
+ rf"\1 FIRST {limit}",
+ self._parsed,
+ count=1,
+ )
+
Review Comment:
`set_limit_value` only injects `FIRST` when the SQL begins with `SELECT`.
For common SELECT queries starting with `WITH ... SELECT ...`, this will not
add a limit at all (leaving queries unbounded in contexts where Superset
expects to enforce row limits). If Firebird support is meant for SQL Lab
exploration, consider adding handling for `WITH` statements (or explicitly
erroring for unsupported shapes) so LIMIT enforcement is not silently skipped.
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
Review Comment:
For inputs that contain only semicolons (e.g. ';' or ';;'), `script.strip()`
is non-empty, so this falls through to `return [cls(stripped, ...)]`, but
`_parse_statement` will then reject it as having 0 statements (after
stripping). This makes semicolon-only scripts raise unexpectedly. Recommend
returning `[]` when the split produces no non-empty parts (regardless of
whether `script.strip()` is non-empty), so 'empty' scripts like ';;' behave
like other empty/whitespace-only scripts.
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
+
+ @classmethod
+ def _parse_statement(
+ cls,
+ statement: str,
+ engine: str,
+ ) -> str:
+ if engine != "firebird":
+ raise SupersetParseError(
+ statement,
+ engine,
+ message=f"Invalid engine: {engine}",
+ )
+
+ statements = _FIREBIRD_SEMI_SPLIT_RE.split(statement)
+ statements = [s.strip() for s in statements if s.strip()]
+ if len(statements) != 1:
+ raise SupersetParseError(
+ statement,
+ engine,
+ message="FirebirdStatement should have exactly one statement",
+ )
+ return statements[0]
+
+ @classmethod
+ def _extract_tables_from_statement(
+ cls,
+ parsed: str,
+ engine: str,
+ ) -> set[Table]:
+ """
+ Extract table references from a Firebird SQL statement.
+
+ Since we cannot reliably parse Firebird SQL without a proper parser,
+ we return an empty set and log a warning. This means that data-access
+ roles will not be enforced by Superset for Firebird databases.
+ """
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table extraction "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return set()
+
+ def format(self, comments: bool = True) -> str:
+ """Return the SQL statement as-is (no AST reformatting)."""
+ return self._parsed.strip()
+
+ def get_settings(self) -> dict[str, str | bool]:
+ """Return statement-level settings. Firebird has none."""
+ return {}
+
+ def is_select(self) -> bool:
+ """Check whether the statement is a SELECT-style query."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... SELECT is a select; WITH ... INSERT/DELETE/etc. is not.
+ return not bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word == "SELECT"
+
+ def is_mutating(self) -> bool:
+ """Check whether the statement performs a mutating operation."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... INSERT/DELETE/UPDATE is mutating.
+ return bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word in _FIREBIRD_MUTATING_KEYWORDS
+
+ def optimize(self) -> FirebirdStatement:
+ """Return self — no AST-level optimisation is possible."""
+ return FirebirdStatement(ast=self._parsed, engine=self.engine)
+
+ def check_functions_present(self, functions: set[str]) -> bool:
+ """Check whether any of the given function names appear in the SQL."""
+ upper = self._parsed.upper()
+ return any(f.upper() in upper for f in functions)
Review Comment:
This implementation can produce false positives because it checks for
substrings anywhere (e.g. function `VERSION` matches `MY_VERSIONED_COL`, or
matches inside string literals/comments). A more accurate approach is to look
for function-call forms like `\\bNAME\\s*\\(` (and ideally ignore quoted
strings/comments before searching).
##########
superset/sql/parse.py:
##########
@@ -1268,6 +1268,237 @@ def parse_predicate(self, predicate: str) -> str:
return predicate
+# Regex to split SQL on semicolons that are NOT inside single-quoted strings.
+_FIREBIRD_SEMI_SPLIT_RE = re.compile(r";(?=(?:[^']*'[^']*')*[^']*$)")
+
+# Match Firebird's FIRST <n> clause: SELECT FIRST 100 ...
+_FIREBIRD_FIRST_RE = re.compile(r"\bFIRST\s+(\d+)\b", re.IGNORECASE)
+
+# DML/DDL keywords that indicate a mutating statement.
+_FIREBIRD_MUTATING_KEYWORDS = frozenset(
+ {"INSERT", "UPDATE", "DELETE", "DROP", "ALTER", "CREATE", "EXECUTE",
"MERGE"}
+)
+
+
+class FirebirdStatement(BaseSQLStatement[str]):
+ """
+ A SQL statement for the Firebird database engine.
+
+ Firebird uses non-standard syntax that is not supported by sqlglot:
+
+ - ``SELECT FIRST <n>`` instead of ``LIMIT <n>``
+ - ``DATEADD(-30 DAY TO CURRENT_DATE)``
+ - ``EXTRACT(YEAR FROM col)`` (supported by some dialects but not all)
+
+ Because there is no sqlglot dialect for Firebird, this class stores the
+ SQL as a plain string and uses simple regular expressions for the few
+ operations Superset needs (limit injection, mutation detection, etc.).
+
+ This follows the same pattern as :class:`KustoKQLStatement`.
+ """
+
+ def __init__(
+ self,
+ statement: str | None = None,
+ engine: str = "firebird",
+ ast: str | None = None,
+ ) -> None:
+ super().__init__(statement, engine, ast)
+
+ @classmethod
+ def split_script(
+ cls,
+ script: str,
+ engine: str,
+ ) -> list[FirebirdStatement]:
+ """
+ Split a script into individual statements on semicolons.
+
+ Semicolons inside single-quoted string literals are preserved.
+ """
+ parts = []
+ for part in _FIREBIRD_SEMI_SPLIT_RE.split(script):
+ stripped = part.strip()
+ if stripped:
+ parts.append(cls(stripped, engine, stripped))
+ if parts:
+ return parts
+ if not script.strip():
+ return []
+ stripped = script.strip()
+ return [cls(stripped, engine, stripped)]
+
+ @classmethod
+ def _parse_statement(
+ cls,
+ statement: str,
+ engine: str,
+ ) -> str:
+ if engine != "firebird":
+ raise SupersetParseError(
+ statement,
+ engine,
+ message=f"Invalid engine: {engine}",
+ )
+
+ statements = _FIREBIRD_SEMI_SPLIT_RE.split(statement)
+ statements = [s.strip() for s in statements if s.strip()]
+ if len(statements) != 1:
+ raise SupersetParseError(
+ statement,
+ engine,
+ message="FirebirdStatement should have exactly one statement",
+ )
+ return statements[0]
+
+ @classmethod
+ def _extract_tables_from_statement(
+ cls,
+ parsed: str,
+ engine: str,
+ ) -> set[Table]:
+ """
+ Extract table references from a Firebird SQL statement.
+
+ Since we cannot reliably parse Firebird SQL without a proper parser,
+ we return an empty set and log a warning. This means that data-access
+ roles will not be enforced by Superset for Firebird databases.
+ """
+ logger.warning(
+ "Firebird SQL is not supported by sqlglot — table extraction "
+ "disabled; data-access roles will not be enforced by Superset."
+ )
+ return set()
+
+ def format(self, comments: bool = True) -> str:
+ """Return the SQL statement as-is (no AST reformatting)."""
+ return self._parsed.strip()
+
+ def get_settings(self) -> dict[str, str | bool]:
+ """Return statement-level settings. Firebird has none."""
+ return {}
+
+ def is_select(self) -> bool:
+ """Check whether the statement is a SELECT-style query."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... SELECT is a select; WITH ... INSERT/DELETE/etc. is not.
+ return not bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
+ return first_word == "SELECT"
+
+ def is_mutating(self) -> bool:
+ """Check whether the statement performs a mutating operation."""
+ sql = self._parsed.lstrip().upper()
+ if not sql:
+ return False
+ first_word = sql.split()[0]
+ if first_word == "WITH":
+ # WITH ... INSERT/DELETE/UPDATE is mutating.
+ return bool(
+ re.search(
+ r"\b(" + "|".join(_FIREBIRD_MUTATING_KEYWORDS) + r")\b",
+ sql,
+ )
+ )
Review Comment:
Both `is_select` and `is_mutating` build a regex pattern dynamically on
every call (`'|'.join(...)`). This is avoidable overhead and also relies on set
iteration order. Recommend precompiling a single `_FIREBIRD_MUTATING_RE =
re.compile(...)` once (using a stable ordering like `sorted(...)`) and reusing
it in both methods.
--
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]
