Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export interface ChartDataResponseResult {
// TODO(hainenber): define proper type for below attributes
rejected_filters?: any[];
applied_filters?: any[];
warning?: string | null;
Comment thread
rusackas marked this conversation as resolved.
/**
* Detected ISO 4217 currency code when AUTO mode is used.
* Returns the currency code if all filtered data contains a single currency,
Expand Down
10 changes: 9 additions & 1 deletion superset-frontend/src/components/Chart/chartAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ import {
getQuerySettings,
getChartDataUri,
} from 'src/explore/exploreUtils';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import {
addDangerToast,
addWarningToast,
} from 'src/components/MessageToasts/actions';
import { logEvent } from 'src/logger/actions';
import { Logger, LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils';
import { allowCrossDomain as domainShardingEnabled } from 'src/utils/hostNamesConfig';
Expand Down Expand Up @@ -813,6 +816,11 @@ export function exploreJSON(
}),
),
);
(queriesResponse as QueryData[]).forEach(response => {
if (response.warning) {
dispatch(addWarningToast(response.warning, { noDuplicate: true }));
}
});
Comment thread
rusackas marked this conversation as resolved.
Comment thread
rusackas marked this conversation as resolved.
return dispatch(
chartUpdateSucceeded(queriesResponse as QueryData[], key as number),
);
Expand Down
51 changes: 51 additions & 0 deletions superset-frontend/src/components/Chart/chartActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
AnnotationSourceType,
AnnotationStyle,
} from '@superset-ui/core';
import * as toastActions from 'src/components/MessageToasts/actions';
import { LOG_EVENT } from 'src/logger/actions';
import * as exploreUtils from 'src/explore/exploreUtils';
import * as actions from 'src/components/Chart/chartAction';
Expand Down Expand Up @@ -412,6 +413,56 @@ describe('chart actions', () => {
);
expect(result).toEqual([1, 2, 3]);
});

test('dispatches addWarningToast when a query response includes a warning', async () => {
const warningMessage =
'Results truncated to 1,000 rows due to memory constraints.';
fetchMock.removeRoute(MOCK_URL);
fetchMock.post(
`glob:*${MOCK_URL}*`,
{ result: [{ warning: warningMessage }] },
{ name: MOCK_URL },
);
const addWarningToastSpy = jest.spyOn(toastActions, 'addWarningToast');

const actionThunk = actions.postChartFormData(
{ viz_type: 'my_viz' } as QueryFormData,
false,
undefined,
undefined,
);
await actionThunk(
dispatch as unknown as actions.ChartThunkDispatch,
mockGetState as unknown as () => actions.RootState,
undefined,
);

expect(addWarningToastSpy).toHaveBeenCalledWith(warningMessage, {
noDuplicate: true,
});
addWarningToastSpy.mockRestore();
fetchMock.removeRoute(MOCK_URL);
setupDefaultFetchMock();
});

test('does not dispatch addWarningToast when no query response has a warning', async () => {
const addWarningToastSpy = jest.spyOn(toastActions, 'addWarningToast');

const actionThunk = actions.postChartFormData(
{ viz_type: 'my_viz' } as QueryFormData,
false,
undefined,
undefined,
);
await actionThunk(
dispatch as unknown as actions.ChartThunkDispatch,
mockGetState as unknown as () => actions.RootState,
undefined,
);

expect(addWarningToastSpy).not.toHaveBeenCalled();
addWarningToastSpy.mockRestore();
});
});

// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
Expand Down
4 changes: 4 additions & 0 deletions superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,10 @@ class ChartDataResponseResult(Schema):
required=False,
allow_none=True,
)
warning = fields.String(
metadata={"description": "Warning message when results were truncated"},
allow_none=True,
)


class DashboardFilterInfoSchema(Schema):
Expand Down
13 changes: 13 additions & 0 deletions superset/common/query_context_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,18 @@ def get_df_payload(
)
cache.df.columns = [unescape_separator(col) for col in cache.df.columns.values]

warning: str | None = None
if cache.bq_memory_limited:
row_count = cache.bq_memory_limited_row_count
chart_id = (self._query_context.form_data or {}).get("slice_id", "")
Comment thread
rusackas marked this conversation as resolved.
prefix = f"Chart {chart_id}: " if chart_id else ""
warning = _(
"%(prefix)sResults truncated to %(row_count)s rows"
" due to memory constraints.",
prefix=prefix,
row_count=f"{row_count:,}",
)
Comment thread
rusackas marked this conversation as resolved.
Comment thread
rusackas marked this conversation as resolved.

return {
"cache_key": cache_key,
"cached_dttm": cache.cache_dttm,
Expand All @@ -210,6 +222,7 @@ def get_df_payload(
"from_dttm": query_obj.from_dttm,
"to_dttm": query_obj.to_dttm,
"label_map": label_map,
"warning": warning,
}

def query_cache_key(self, query_obj: QueryObject, **kwargs: Any) -> str | None:
Expand Down
21 changes: 20 additions & 1 deletion superset/common/utils/query_cache_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from datetime import datetime, timezone
from typing import Any

from flask import current_app
from flask import current_app, g, has_request_context
from flask_caching import Cache
from pandas import DataFrame

Expand Down Expand Up @@ -86,6 +86,8 @@ def __init__(
self.cache_value = cache_value
self.sql_rowcount = sql_rowcount
self.queried_dttm = queried_dttm
self.bq_memory_limited: bool = False
self.bq_memory_limited_row_count: int = 0

# pylint: disable=too-many-arguments
def set_query_result(
Expand Down Expand Up @@ -123,6 +125,15 @@ def set_query_result(
)
self.is_loaded = True

# Capture BigQuery memory-limit flag so it survives cache hits
if has_request_context():
self.bq_memory_limited = getattr(g, "bq_memory_limited", False)
self.bq_memory_limited_row_count = getattr(
g, "bq_memory_limited_row_count", 0
)
g.bq_memory_limited = False
g.bq_memory_limited_row_count = 0

value = {
"df": self.df,
"query": self.query,
Expand All @@ -133,6 +144,8 @@ def set_query_result(
"sql_rowcount": self.sql_rowcount,
"queried_dttm": self.queried_dttm,
"dttm": self.queried_dttm, # Backwards compatibility
"bq_memory_limited": self.bq_memory_limited,
"bq_memory_limited_row_count": self.bq_memory_limited_row_count,
}
if self.is_loaded and key and self.status != QueryStatus.FAILED:
self.set(
Expand Down Expand Up @@ -193,6 +206,12 @@ def get(
"queried_dttm", cache_value.get("dttm")
)
query_cache.cache_value = cache_value
query_cache.bq_memory_limited = cache_value.get(
"bq_memory_limited", False
)
query_cache.bq_memory_limited_row_count = cache_value.get(
"bq_memory_limited_row_count", 0
)
current_app.config["STATS_LOGGER"].incr("loaded_from_cache")
except KeyError as ex:
logger.exception(ex)
Expand Down
3 changes: 3 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1452,6 +1452,9 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
# Timeout duration for SQL Lab synchronous queries
SQLLAB_TIMEOUT = int(timedelta(seconds=30).total_seconds())

# BigQuery max fetch size in MB (limits memory usage when fetching large results)
BQ_FETCH_MAX_MB = 200

# Timeout duration for SQL Lab query validation
SQLLAB_VALIDATION_TIMEOUT = int(timedelta(seconds=10).total_seconds())

Expand Down
109 changes: 102 additions & 7 deletions superset/db_engine_specs/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import logging
import re
import sys
import urllib
from datetime import datetime
from re import Pattern
Expand All @@ -27,6 +28,7 @@
import pandas as pd
from apispec import APISpec
from apispec.ext.marshmallow import MarshmallowPlugin
from flask import current_app, g, has_app_context, has_request_context
from flask_babel import gettext as __
from marshmallow import fields, Schema
from marshmallow.exceptions import ValidationError
Expand Down Expand Up @@ -106,6 +108,11 @@

ma_plugin = MarshmallowPlugin()

# Initial sample size for the progressive fetch in ``fetch_data``. Reading a
# small first batch lets us measure the row size before deciding how many
# more rows fit within ``BQ_FETCH_MAX_MB``.
_BQ_INITIAL_SAMPLE_ROWS = 1000


class BigQueryParametersSchema(Schema):
credentials_info = EncryptedString(
Expand Down Expand Up @@ -303,13 +310,101 @@ def convert_dttm(
return None

@classmethod
def fetch_data(cls, cursor: Any, limit: int | None = None) -> list[tuple[Any, ...]]:
data = super().fetch_data(cursor, limit)
# Support type BigQuery Row, introduced here PR #4071
# google.cloud.bigquery.table.Row
if data and type(data[0]).__name__ == "Row":
data = [r.values() for r in data] # type: ignore
return data
def fetch_data(cls, cursor: Any, limit: int | None = None) -> list[tuple[Any, ...]]: # noqa: C901
"""
Progressive fetch for BigQuery to prevent browser memory overload.

Samples a first batch to estimate row size, then extrapolates the
total number of rows that fit within ``BQ_FETCH_MAX_MB``.
Falls back to the parent implementation on any error.
"""
# ``BQ_FETCH_MAX_MB`` has a default in ``config.py``, so use bracket
# access in-context — a missing key should surface as a loud KeyError
# rather than be silently masked by a duplicated default here. The
# 200 fallback is only used when running outside an app context
# (e.g., direct unit-test calls to ``fetch_data``).
max_mb: int = (
current_app.config["BQ_FETCH_MAX_MB"] if has_app_context() else 200
)
Comment thread
rusackas marked this conversation as resolved.
max_bytes = max_mb * 1024 * 1024

try:
initial_batch_size = (
min(_BQ_INITIAL_SAMPLE_ROWS, limit)
if limit
else _BQ_INITIAL_SAMPLE_ROWS
)
first_batch: list[Any] = cursor.fetchmany(initial_batch_size)

if not first_batch:
if has_request_context():
g.bq_memory_limited = False
g.bq_memory_limited_row_count = 0
return []

# Support BigQuery Row objects (PR #4071)
if type(first_batch[0]).__name__ == "Row":
first_batch = [r.values() for r in first_batch]

# Estimate how many rows fit in the memory budget.
# Sum container + element sizes (one level deep) for a better
# estimate. Most BigQuery cell values are primitives (str, int,
# float, date), so one level captures the dominant allocation.
first_batch_bytes = sum(
sys.getsizeof(row) + sum(sys.getsizeof(v) for v in row)
for row in first_batch
)
rows_fetched = len(first_batch)
avg_bytes_per_row = first_batch_bytes / rows_fetched
total_rows_for_target = int(max_bytes / avg_bytes_per_row)

if limit:
total_rows_for_target = min(limit, total_rows_for_target)

remaining_rows = total_rows_for_target - rows_fetched

# First batch already covers the budget or the result set
if rows_fetched < initial_batch_size or remaining_rows <= 0:
memory_limited = (
remaining_rows <= 0 and rows_fetched == initial_batch_size
)
if has_request_context():
g.bq_memory_limited = memory_limited
g.bq_memory_limited_row_count = len(first_batch)
return first_batch

# Fetch one extra row to confirm truncation without false positives
second_batch: list[Any] = cursor.fetchmany(remaining_rows + 1) or []
if second_batch and type(second_batch[0]).__name__ == "Row":
second_batch = [r.values() for r in second_batch]

# Truncation is confirmed only when more rows exist beyond the budget
memory_limited = len(second_batch) > remaining_rows
if memory_limited:
second_batch = second_batch[:remaining_rows]

data = first_batch + second_batch

if has_request_context():
g.bq_memory_limited = memory_limited
g.bq_memory_limited_row_count = len(data)
return data

except Exception: # pylint: disable=broad-except
Comment thread
rusackas marked this conversation as resolved.
# Broad catch on purpose: any failure in the size-estimation /
# progressive-fetch path (BigQuery DB-API errors, network or
# auth timeouts mid-fetch, ``sys.getsizeof`` raising on an
# unexpected cell type, or a future ``Row`` subclass we don't
# know how to unwrap) must degrade gracefully to the parent's
# straight fetch so the user still gets data.
# Fallback to parent implementation
data = super().fetch_data(cursor, limit)
if data and type(data[0]).__name__ == "Row":
data = [r.values() for r in data] # type: ignore
if has_request_context():
g.bq_memory_limited = False
g.bq_memory_limited_row_count = len(data) if data else 0
return data

@staticmethod
def _mutate_label(label: str) -> str:
Expand Down
Loading
Loading