aminghadersohi commented on code in PR #45348:
URL: https://github.com/apache/superset/pull/45348#discussion_r4296907139
##########
superset/sql/execution/celery_task.py:
##########
@@ -240,6 +254,23 @@ def _finalize_successful_query(
payload["query"]["state"] = QueryStatus.SUCCESS.value
+def _get_payload_size(payload: dict[str, Any], serialized_payload: bytes) ->
int:
+ """Count raw Arrow and metadata, excluding duplicate nested sidecar
values."""
+ size = len(serialized_payload)
+ for statement in payload.get("statements", []):
+ if isinstance(data := statement.get("data"), str):
+ # Retain all envelope metadata without allocating another decoded
+ # copy of the Arrow data just to determine its size.
+ arrow_size = len(data) // 4 * 3 - data.endswith("=") -
data.endswith("==")
+ size -= len(data) - arrow_size
+ # Arrow retains the full nested column as text for older readers and
+ # unknown tags. Its typed copy must not charge the same values twice.
+ size -= sum(
+ len(value) for value in statement.get("nested_columns_msgpack",
{}).values()
+ )
+ return size
Review Comment:
Fixed in 6ab7fb9076e4bffd745b3decc18a2333773cab9e. Sidecars count at raw
size; only `min(raw sidecar size, matching Arrow column.nbytes)` is discounted.
I also corrected the boundary fixture/expectations and the documentation.
Regression:
`tests/unit_tests/sql/execution/test_async_results.py::test_nested_binary_payload_limit_rejects_sidecar_only_bytes`
uses 300 nested 4,097-byte binary values that Arrow replaces with
placeholders. Both transports returned SUCCESS before; both return FAILED with
`RESULT_TOO_LARGE_ERROR` and no backend write after. Restoring the old size
helper makes both cases fail again. The expanded
`test_store_results_in_backend_raw_arrow_size_boundary` covers
exact/one-byte-over limits with JSON-like/binary sidecars and present/missing
Arrow columns.
I also reproduced the 6,000-row case: the old helper counted 90,607/90,703
bytes while storing roughly 23.5 MiB compressed; the corrected counts are
24,606,610/24,606,706 bytes. The small transport difference is envelope
metadata.
Siblings checked:
- superset/sql/execution/celery_task.py:258 — fixed in
6ab7fb9076e4bffd745b3decc18a2333773cab9e
- superset/sql/execution/celery_task.py:319 — fixed in
6ab7fb9076e4bffd745b3decc18a2333773cab9e
- superset/sql/execution/celery_task.py:689 — fixed in
6ab7fb9076e4bffd745b3decc18a2333773cab9e
- superset/sql_lab.py:714 — not affected: the separate top-level backend
writer counts the entire serialized payload and produces no nested sidecar
- superset/sql_lab.py:803 — not affected: the return-results check also
counts the entire serialized payload without a sidecar discount
- superset/sql/execution/executor.py:557 — not affected: synchronous
execution returns an in-process DataFrame, without this backend envelope/limit;
`test_sync_nested_binary_does_not_use_backend_payload_limit` pins both
transport settings
- superset/sql/execution/executor.py:1266 — not affected: separate optional
data-cache writer, not a payload-limit computation
- superset/models/core.py:1689 — not affected: transparent delegate to the
tested synchronous executor
- superset/models/core.py:1705 — not affected: transparent delegate to the
async executor whose worker reaches the corrected writer
- superset/config.py:2007 — not affected: configuration declaration only
Validation: touched modules 455 passed, 2 skipped; `pytest
--cov=superset/sql/ ./tests/unit_tests/sql/ --cov-fail-under=100 -m 'not manual
and not llm_compliance'` exited 0 (2,733 passed, 2 skipped; 100.00%).
Pre-commit on all branch-changed files and mypy passed. The local coverage
process exited about 7.4 seconds after the last test; I did not reproduce the
post-test hang.
--
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]
