bito-code-review[bot] commented on PR #44604:
URL: https://github.com/apache/superset/pull/44604#issuecomment-5841850267
<!-- Bito Reply -->
The review comment correctly identifies a discrepancy between the schema
documentation and the actual implementation behavior. The schema description in
`superset/mcp_service/sql_lab/schemas.py` states that an absent SQL limit adds
one row, but the implementation logic (as described in the updated
documentation and the PR changes) is designed to cap the result set at the
minimum of the SQL limit and the request limit, rather than simply adding a
row.
To resolve this, the schema description should be updated to accurately
reflect the capping behavior. The current change in the PR already attempts to
clarify this, but the comment suggests the wording remains confusing or
inconsistent with the actual execution logic. I recommend updating the
`description` field in `ExecuteSqlRequest` to clearly state that it acts as a
safety cap on the outer SQL limit, rather than implying it adds a row.
**superset/mcp_service/sql_lab/schemas.py**
```
description=(
"Maximum rows returned. "
"Omitted: respects SQL LIMIT. "
"If set, acts as a safety cap on the last statement's outer
LIMIT, "
"taking the minimum of the SQL LIMIT and this value. "
"Never raises stricter SQL limits or changes inner limits. "
"Explicit limits also obey server SQL_MAX_ROW."
),
```
--
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]
