aminghadersohi commented on code in PR #44604:
URL: https://github.com/apache/superset/pull/44604#discussion_r4139575061
##########
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:
Follow-up fixed in 53718267d279b0d6c8c0e112d6aa988d26a6e2bc: BigQuery probes
for one omitted row before marking a full memory-limited initial sample as
truncated. An exactly 1,000-row result with a 1 MB cap now reports
truncated=false. Six regression cases cover 999, 1,000 and 1,001 rows, with and
without a request limit, including the bounded cursor probe. Before the fix: 4
failed, 2 passed; after: all 6 passed. The affected suites pass with 4,273
passed and 7 skipped; the SQL coverage gate passes with 2,208 tests and 100%
coverage, and all 12 applicable pre-commit hooks pass. Apache master was merged
normally in b6c7f041e8e582efeca3ec2376d9e2688d156f30, preserving both release
notes.
--
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]
