aminghadersohi commented on code in PR #44604:
URL: https://github.com/apache/superset/pull/44604#discussion_r4138361829
##########
superset/sql/execution/executor.py:
##########
@@ -271,7 +324,18 @@ def execute_sql_with_cursor(
# Fetch results from ALL statements
description = cursor.description
if description:
- rows = database.db_engine_spec.fetch_data(cursor)
+ fetch_cursor = cursor
+ # SQL restrictions cannot always be safely wrapped or replaced.
+ # Match SQL limit application: cap only the last statement, and
+ # only when the caller supplied a limit (also honoring
SQL_MAX_ROW).
+ if i == total - 1 and query.limit is not None:
+ row_limit: int = query.limit
+ if sql_max_row := app.config.get("SQL_MAX_ROW"):
+ row_limit = min(row_limit, sql_max_row)
+ fetch_cursor = _LimitedCursor(cursor, row_limit)
Review Comment:
Fixed in f06ce97e7b: StatementResult and StatementInfo expose fetch
truncation using one probe row, distinguishing exact caps from omitted rows,
including metadata and RETURNING results. The flag survives async execution and
caching.
##########
superset/sql/parse.py:
##########
@@ -1784,22 +1794,65 @@ def _has_limit_by(self) -> bool:
def get_limit_value(self) -> int | None:
"""
- Parse a SQL query and return the `LIMIT` or `TOP` value, if present.
+ Return a fixed outer `LIMIT`, `TOP`, or `FETCH` row count, if known.
"""
# `LIMIT 2 BY id` bounds each group, not the result set, so reporting 2
# here would make `_set_query_limit()` clamp the whole query to 2 rows.
if self._has_limit_by():
return None
if limit_node := self._parsed.args.get("limit"):
- literal = limit_node.args.get("expression") or getattr(
- limit_node, "this", None
- )
+ options = limit_node.args.get("limit_options")
+ if options and (
+ options.args.get("percent") or options.args.get("with_ties")
+ ):
+ return None
Review Comment:
Documented in f06ce97e7b: SQL Lab intentionally treats PERCENT and WITH TIES
as non-fixed counts and uses the dropdown cap, so TOP 5 PERCENT with a 1000-row
dropdown can return up to 1000 rows rather than five. UPDATING.md also covers
literal FETCH FIRST and parenthesized limits.
##########
superset/mcp_service/sql_lab/schemas.py:
##########
@@ -72,9 +72,12 @@ class ExecuteSqlRequest(BaseModel):
limit: int | None = Field(
default=None,
description=(
- "Maximum number of rows to return. "
- "If not specified, respects the LIMIT in your SQL query. "
- "If specified, overrides any SQL LIMIT clause."
+ "Maximum rows returned. "
Review Comment:
Fixed in f06ce97e7b: the limit description says “Maximum rows returned by
the last statement” and explicitly describes adding a LIMIT clause when absent.
##########
superset/sql/execution/executor.py:
##########
@@ -186,6 +186,60 @@ def build_statement_blocks(
return parsed_script, blocks
+class _LimitedCursor:
+ """Bound cumulative cursor reads without bypassing engine fetch
processing."""
+
+ def __init__(self, cursor: Any, limit: int) -> None:
+ """Wrap a cursor with a shared budget for all row-reading methods."""
+ self._cursor = cursor
+ self._remaining = limit
+
+ def __getattr__(self, name: str) -> Any:
+ """Delegate metadata and driver-specific methods to the real cursor."""
+ return getattr(self._cursor, name)
+
+ @property
+ def arraysize(self) -> int:
+ """Expose the driver's default fetch batch size."""
+ return self._cursor.arraysize
+
+ @arraysize.setter
+ def arraysize(self, value: int) -> None:
+ """Preserve engine-specific cursor batch-size configuration."""
+ self._cursor.arraysize = value
+
+ def fetchmany(self, size: int | None = None) -> list[Any]:
+ """Read no more than the remaining budget, including across batches."""
+ size = self.arraysize if size is None else size
+ size = max(0, min(size, self._remaining))
+ # Some drivers interpret zero as unbounded, so do not call them at all.
+ if not size:
+ return []
+ # Reserve before reading so an engine's error fallback cannot retry
+ # beyond the budget after a driver has partially consumed a batch.
+ self._remaining -= size
+ return self._cursor.fetchmany(size)
Review Comment:
Budget accounting was fixed in e781f0d7c7: only successful driver reads
consume the budget, preserving it for retries and BigQuery fallback. f06ce97e7b
retains that behavior and exposes fetch truncation, including for RETURNING,
while documenting that returned-row caps do not limit affected rows.
--
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]
