Compare commits

...

3 Commits

Author SHA1 Message Date
Beto Dealmeida
05182f95a0 Address comments 2026-06-23 12:00:54 -04:00
Beto Dealmeida
ace7a65716 Small fixes 2026-06-23 11:50:57 -04:00
Beto Dealmeida
0a40a94ab2 fix: enforce time filter on adhoc SQL 2026-06-23 10:02:28 -04:00
5 changed files with 245 additions and 3 deletions

View File

@@ -35,6 +35,7 @@ import {
QueryFormData,
DatasourceType,
isDefined,
isAdhocColumn,
JsonValue,
NO_TIME_RANGE,
usePrevious,
@@ -70,6 +71,7 @@ import {
import Tabs from '@superset-ui/core/components/Tabs';
import { PluginContext } from 'src/components';
import { useConfirmModal } from 'src/hooks/useConfirmModal';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { getSectionsToRender } from 'src/explore/controlUtils';
import { ExploreActions } from 'src/explore/actions/exploreActions';
@@ -300,6 +302,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
const theme = useTheme();
const pluginContext = useContext(PluginContext);
const { showConfirm, ConfirmModal } = useConfirmModal();
const { addWarningToast } = useToasts();
const prevState = usePrevious(props.exploreState);
const prevDatasource = usePrevious(props.exploreState.datasource);
@@ -321,7 +324,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
const { form_data, actions } = props;
const { setControlValue } = actions;
const { x_axis, adhoc_filters } = form_data;
const { x_axis, adhoc_filters, granularity_sqla } = form_data;
const previousXAxis = usePrevious(x_axis);
@@ -370,6 +373,24 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
showConfirm,
]);
useEffect(() => {
if (
x_axis &&
(previousXAxis === undefined || x_axis !== previousXAxis) &&
isAdhocColumn(x_axis) &&
!granularity_sqla
) {
addWarningToast(
t(
`The X-axis is a SQL expression, so time range filtering is not ` +
`applied automatically. Without a time filter, queries may scan ` +
`the entire table. Add a filter on your dataset's time column in ` +
`the Filters section to limit the rows read.`,
),
);
}
}, [x_axis, previousXAxis, granularity_sqla, addWarningToast]);
useEffect(() => {
let shouldUpdateControls = false;
const removeDatasourceWarningFromControl = (

View File

@@ -308,6 +308,29 @@ class QueryContextFactory: # pylint: disable=too-few-public-methods
if filter["col"] != filter_to_remove
]
elif is_adhoc_column(x_axis) and ( # type: ignore
query_object.from_dttm or query_object.to_dttm
):
# x-axis is a SQL expression (not a physical temporal column) and a
# time range is configured, but no time column (granularity) was set.
# Fall back to the dataset's main datetime column so helpers.py adds a
# WHERE time filter — without it engines like ClickHouse perform a full
# table scan and trigger max_rows_to_read errors.
main_dttm_col = getattr(datasource, "main_dttm_col", None)
if main_dttm_col and main_dttm_col in temporal_columns:
query_object.granularity = main_dttm_col
# Remove any TEMPORAL_RANGE filter already targeting main_dttm_col
# so helpers.py doesn't add the same condition a second time via the
# filter loop (granularity covers it via from_dttm/to_dttm).
query_object.filter = [
f
for f in query_object.filter
if not (
f.get("op") == "TEMPORAL_RANGE"
and f.get("col") == main_dttm_col
)
]
def _apply_filters(self, query_object: QueryObject) -> None:
if query_object.time_range:
for filter_object in query_object.filter:

View File

@@ -123,7 +123,7 @@ class DatetimeFormatDetector:
sql = database.apply_limit_to_sql(sql, limit=self.sample_size, force=True)
# Execute query and get results
df = database.get_df(sql, dataset.schema)
df = database.get_df(sql, dataset.catalog, dataset.schema)
if df.empty or column.column_name not in df.columns:
logger.warning(

View File

@@ -384,6 +384,203 @@ class TestQueryContextFactory:
assert query_object.columns == ["ds", "other_col"]
def test_apply_granularity_expression_xaxis_sets_main_dttm_col(self):
"""SQL expression x-axis with time range falls back to main_dttm_col."""
from datetime import datetime
query_object = Mock(spec=QueryObject)
query_object.granularity = None
query_object.from_dttm = datetime(2023, 1, 1)
query_object.to_dttm = None
query_object.columns = [
{
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
"columnType": "BASE_AXIS",
}
]
query_object.filter = []
form_data = {
"x_axis": {
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
}
}
datasource = Mock()
datasource.columns = [{"column_name": "ts", "is_dttm": True}]
datasource.main_dttm_col = "ts"
self.factory._apply_granularity(query_object, form_data, datasource)
assert query_object.granularity == "ts"
def test_apply_granularity_expression_xaxis_no_time_range_no_fallback(self):
"""SQL expression x-axis without time range does not set granularity."""
query_object = Mock(spec=QueryObject)
query_object.granularity = None
query_object.from_dttm = None
query_object.to_dttm = None
query_object.columns = [
{
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
"columnType": "BASE_AXIS",
}
]
query_object.filter = []
form_data = {
"x_axis": {
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
}
}
datasource = Mock()
datasource.columns = [{"column_name": "ts", "is_dttm": True}]
datasource.main_dttm_col = "ts"
self.factory._apply_granularity(query_object, form_data, datasource)
assert query_object.granularity is None
def test_apply_granularity_expression_xaxis_no_main_dttm_col(self):
"""SQL expression x-axis with time range but no main_dttm_col stays None."""
from datetime import datetime
query_object = Mock(spec=QueryObject)
query_object.granularity = None
query_object.from_dttm = datetime(2023, 1, 1)
query_object.to_dttm = None
query_object.columns = [
{
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
"columnType": "BASE_AXIS",
}
]
query_object.filter = []
form_data = {
"x_axis": {
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
}
}
datasource = Mock()
datasource.columns = []
datasource.main_dttm_col = None
self.factory._apply_granularity(query_object, form_data, datasource)
assert query_object.granularity is None
def test_apply_granularity_expression_xaxis_main_dttm_not_temporal(self):
"""SQL expression x-axis fallback skipped if main_dttm_col is not temporal."""
from datetime import datetime
query_object = Mock(spec=QueryObject)
query_object.granularity = None
query_object.from_dttm = datetime(2023, 1, 1)
query_object.to_dttm = None
query_object.columns = [
{
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
"columnType": "BASE_AXIS",
}
]
query_object.filter = []
form_data = {
"x_axis": {
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
}
}
datasource = Mock()
# main_dttm_col points to a column not flagged as temporal
datasource.columns = [{"column_name": "ts", "is_dttm": False}]
datasource.main_dttm_col = "ts"
self.factory._apply_granularity(query_object, form_data, datasource)
assert query_object.granularity is None
def test_apply_granularity_expression_xaxis_deduplicates_temporal_filter(self):
"""Fallback removes existing TEMPORAL_RANGE filter for main_dttm_col."""
from datetime import datetime
query_object = Mock(spec=QueryObject)
query_object.granularity = None
query_object.from_dttm = datetime(2023, 1, 1)
query_object.to_dttm = None
query_object.columns = [
{
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
"columnType": "BASE_AXIS",
}
]
# Pre-existing TEMPORAL_RANGE filter for the main dttm column (e.g.
# left over from when 'ts' was the physical x-axis).
query_object.filter = [
{"col": "ts", "op": "TEMPORAL_RANGE", "val": "Last 7 days"},
{"col": "other", "op": "==", "val": "foo"},
]
form_data = {
"x_axis": {
"sqlExpression": "duration * 0.05",
"expressionType": "SQL",
"label": "duration * 0.05",
}
}
datasource = Mock()
datasource.columns = [{"column_name": "ts", "is_dttm": True}]
datasource.main_dttm_col = "ts"
self.factory._apply_granularity(query_object, form_data, datasource)
assert query_object.granularity == "ts"
# TEMPORAL_RANGE filter for 'ts' removed to prevent duplicate WHERE clause
assert not any(
f.get("col") == "ts" and f.get("op") == "TEMPORAL_RANGE"
for f in query_object.filter
)
# Unrelated filter preserved
assert any(f.get("col") == "other" for f in query_object.filter)
def test_apply_granularity_physical_string_xaxis_no_fallback(self):
"""Physical column string x-axis without granularity: no fallback."""
from datetime import datetime
query_object = Mock(spec=QueryObject)
query_object.granularity = None
query_object.from_dttm = datetime(2023, 1, 1)
query_object.to_dttm = None
query_object.columns = ["non_temporal_col"]
query_object.filter = []
form_data = {"x_axis": "non_temporal_col"}
datasource = Mock()
datasource.columns = [{"column_name": "ts", "is_dttm": True}]
datasource.main_dttm_col = "ts"
self.factory._apply_granularity(query_object, form_data, datasource)
# String x-axis is not an adhoc column so no fallback should trigger
assert query_object.granularity is None
def test_apply_granularity_x_axis_not_temporal(self):
"""Test _apply_granularity when x_axis is not a temporal column"""
query_object = Mock(spec=QueryObject)

View File

@@ -31,6 +31,7 @@ def mock_dataset() -> MagicMock:
dataset = MagicMock(spec=SqlaTable)
dataset.table_name = "test_table"
dataset.schema = "test_schema"
dataset.catalog = None
dataset.is_virtual = False
# Mock the database engine and dialect for identifier quoting
@@ -243,7 +244,7 @@ def test_detect_column_format_query_has_no_is_not_null(
captured_sql: list[str] = []
original_get_df = mock_dataset.database.get_df
def capture_sql(sql: str, schema: str) -> pd.DataFrame:
def capture_sql(sql: str, catalog: str | None, schema: str | None) -> pd.DataFrame:
captured_sql.append(sql)
return original_get_df.return_value