mirror of
https://github.com/apache/superset.git
synced 2026-06-26 09:59:21 +00:00
Compare commits
3 Commits
chore/ci-f
...
fix-clickh
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
05182f95a0 | ||
|
|
ace7a65716 | ||
|
|
0a40a94ab2 |
@@ -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 = (
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user