This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a55b236a5be fix(semantic-layer): fetch view metadata once per query
(#45104)
a55b236a5be is described below
commit a55b236a5be4fa4c4472afd964961aa44e8c7716
Author: Mike Bridge <[email protected]>
AuthorDate: Sat Oct 10 16:19:40 2026 -0600
fix(semantic-layer): fetch view metadata once per query (#45104)
---
.../src/superset_core/semantic_layers/view.py | 7 +-
superset-core/tests/semantic_layers/test_view.py | 23 ++++
superset/semantic_layers/mapper.py | 114 ++++++++++------
tests/unit_tests/semantic_layers/mapper_test.py | 144 ++++++++++++++++++++-
4 files changed, 243 insertions(+), 45 deletions(-)
diff --git a/superset-core/src/superset_core/semantic_layers/view.py
b/superset-core/src/superset_core/semantic_layers/view.py
index 3b8cc306e2e..71ab7890e1a 100644
--- a/superset-core/src/superset_core/semantic_layers/view.py
+++ b/superset-core/src/superset_core/semantic_layers/view.py
@@ -68,10 +68,9 @@ class SemanticView(ABC):
"automatically mapped to member IDs."
)
- # Implementations must expose a display name for the view.
- # Declared here as a type annotation (not abstract) so that existing
- # implementations are not required to add a formal @abstractmethod.
- name: str
+ # Optional display name. Hosts may supply their own label when it is empty;
+ # identity always comes from uid(), never from this presentation field.
+ name: str = ""
@property
def metadata_cache_token(self) -> str | None:
diff --git a/superset-core/tests/semantic_layers/test_view.py
b/superset-core/tests/semantic_layers/test_view.py
new file mode 100644
index 00000000000..abaf03015e2
--- /dev/null
+++ b/superset-core/tests/semantic_layers/test_view.py
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from superset_core.semantic_layers.view import SemanticView
+
+
+def test_view_metadata_defaults_are_conservative() -> None:
+ assert SemanticView.features == frozenset()
+ assert SemanticView.name == ""
diff --git a/superset/semantic_layers/mapper.py
b/superset/semantic_layers/mapper.py
index a1df1dea838..1c055630bb8 100644
--- a/superset/semantic_layers/mapper.py
+++ b/superset/semantic_layers/mapper.py
@@ -25,6 +25,7 @@ single dataframe.
"""
import logging
+from dataclasses import dataclass
from datetime import date, datetime, time, timedelta, tzinfo
from time import time as current_time
from typing import Any, cast, Sequence, TypeGuard
@@ -49,7 +50,7 @@ from superset_core.semantic_layers.types import (
SemanticQuery,
SemanticResult,
)
-from superset_core.semantic_layers.view import SemanticViewFeature
+from superset_core.semantic_layers.view import SemanticView,
SemanticViewFeature
from superset.common.db_query_status import QueryStatus
from superset.common.query_object import QueryObject
@@ -126,6 +127,18 @@ class ValidatedQueryObject(QueryObject):
series_limit_metric: str | None
+@dataclass(frozen=True)
+class _ViewMetadata:
+ """One query's member observation, preserving provider iteration order."""
+
+ metrics: tuple[Metric, ...]
+ dimensions: tuple[Dimension, ...]
+
+ @classmethod
+ def from_view(cls, view: SemanticView) -> "_ViewMetadata":
+ return cls(tuple(view.get_metrics()), tuple(view.get_dimensions()))
+
+
def get_results(query_object: QueryObject) -> QueryResult:
"""
Run 1+ queries based on `QueryObject` and return the results.
@@ -133,8 +146,11 @@ def get_results(query_object: QueryObject) -> QueryResult:
:param query_object: The QueryObject containing query specifications
:return: QueryResult compatible with Superset's query interface
"""
- if not validate_query_object(query_object):
+ if not query_object.datasource:
raise ValueError("QueryObject must have a datasource defined.")
+ query_object = cast(ValidatedQueryObject, query_object)
+ metadata: _ViewMetadata = _validation_metadata(query_object)
+ _validate_query_object(query_object, metadata)
# Track execution time
start_time = current_time()
@@ -148,7 +164,7 @@ def get_results(query_object: QueryObject) -> QueryResult:
# Step 1: Convert QueryObject to list of SemanticQuery objects
# The first query is the main query, subsequent queries are for time
offsets
- queries = map_query_object(query_object)
+ queries: list[SemanticQuery] = _map_query_object(query_object, metadata)
# Step 2: Execute the main query (first in the list)
main_query = queries[0]
@@ -346,17 +362,19 @@ def _normalize_column(column: str | AdhocColumn,
dimension_names: set[str]) -> s
def map_query_object(query_object: ValidatedQueryObject) ->
list[SemanticQuery]:
- """
- Convert a `QueryObject` into a list of `SemanticQuery`.
+ """Map a query using a fresh observation of the provider's metadata."""
+ metadata: _ViewMetadata = _ViewMetadata.from_view(
+ query_object.datasource.implementation
+ )
+ return _map_query_object(query_object, metadata)
- This function maps the `QueryObject` into query objects that focus less on
- visualization and more on semantics.
- """
- semantic_view = query_object.datasource.implementation
- # The SemanticView ABC contract is the get_metrics()/get_dimensions()
- # methods; never read undeclared attributes off the provider view.
- view_metrics = semantic_view.get_metrics()
- view_dimensions = semantic_view.get_dimensions()
+
+def _map_query_object(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> list[SemanticQuery]:
+ """Use the same members for the main query and every time comparison."""
+ view_metrics: tuple[Metric, ...] = metadata.metrics
+ view_dimensions: tuple[Dimension, ...] = metadata.dimensions
all_metrics = {metric.name: metric for metric in view_metrics}
all_dimensions = {dimension.name: dimension for dimension in
view_dimensions}
@@ -1127,41 +1145,54 @@ def validate_query_object(
if not query_object.datasource:
return False
- query_object = cast(ValidatedQueryObject, query_object)
+ validated: ValidatedQueryObject = cast(ValidatedQueryObject, query_object)
+ metadata: _ViewMetadata = _validation_metadata(validated)
+ _validate_query_object(validated, metadata)
+ return True
+
- query_object.datasource.implementation.validate_selection_version(
+def _validation_metadata(query_object: ValidatedQueryObject) -> _ViewMetadata:
+ """Check member identity before fetching metadata for validation."""
+ view: SemanticView = query_object.datasource.implementation
+ view.validate_selection_version(
query_object.extras.get("semantic_selection_version")
)
- _validate_metrics(query_object)
- _validate_dimensions(query_object)
- _validate_filters(query_object)
- _validate_granularity(query_object)
- _validate_group_limit(query_object)
- _validate_orderby(query_object)
+ return _ViewMetadata.from_view(view)
- return True
+def _validate_query_object(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> None:
+ _validate_metrics(query_object, metadata)
+ _validate_dimensions(query_object, metadata)
+ _validate_filters(query_object)
+ _validate_granularity(query_object, metadata)
+ _validate_group_limit(query_object, metadata)
+ _validate_orderby(query_object, metadata)
-def _validate_metrics(query_object: ValidatedQueryObject) -> None:
+
+def _validate_metrics(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> None:
"""
Make sure metrics are defined in the semantic view.
"""
- semantic_view = query_object.datasource.implementation
if any(not isinstance(metric, str) for metric in (query_object.metrics or
[])):
raise ValueError("Adhoc metrics are not supported in Semantic Views.")
- metric_names = {metric.name for metric in semantic_view.get_metrics()}
+ metric_names: set[str] = {metric.name for metric in metadata.metrics}
if not set(query_object.metrics or []) <= metric_names:
raise ValueError("All metrics must be defined in the Semantic View.")
-def _validate_dimensions(query_object: ValidatedQueryObject) -> None:
+def _validate_dimensions(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> None:
"""
Make sure all dimensions are defined in the semantic view.
"""
- semantic_view = query_object.datasource.implementation
- dimension_names = {dimension.name for dimension in
semantic_view.get_dimensions()}
+ dimension_names: set[str] = {dimension.name for dimension in
metadata.dimensions}
# Normalize all columns to dimension names
normalized_columns = [
@@ -1185,12 +1216,13 @@ def _validate_filters(query_object:
ValidatedQueryObject) -> None:
raise ValueError("All filters must have an operator defined.")
-def _validate_granularity(query_object: ValidatedQueryObject) -> None:
+def _validate_granularity(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> None:
"""
Make sure time column and time grain are valid.
"""
- semantic_view = query_object.datasource.implementation
- view_dimensions = semantic_view.get_dimensions()
+ view_dimensions: tuple[Dimension, ...] = metadata.dimensions
all_dimensions = {dimension.name: dimension for dimension in
view_dimensions}
dimension_names = set(all_dimensions.keys())
@@ -1220,7 +1252,9 @@ def _validate_granularity(query_object:
ValidatedQueryObject) -> None:
)
-def _validate_group_limit(query_object: ValidatedQueryObject) -> None:
+def _validate_group_limit(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> None:
"""
Validate group limit related features in the query object.
"""
@@ -1242,7 +1276,7 @@ def _validate_group_limit(query_object:
ValidatedQueryObject) -> None:
if any(not isinstance(col, str) for col in query_object.series_columns):
raise ValueError("Adhoc dimensions are not supported in series
columns.")
- metric_names = {metric.name for metric in semantic_view.get_metrics()}
+ metric_names: set[str] = {metric.name for metric in metadata.metrics}
if query_object.series_limit_metric and (
not isinstance(query_object.series_limit_metric, str)
or query_object.series_limit_metric not in metric_names
@@ -1251,7 +1285,7 @@ def _validate_group_limit(query_object:
ValidatedQueryObject) -> None:
"The series limit metric must be defined in the Semantic View."
)
- dimension_names = {dimension.name for dimension in
semantic_view.get_dimensions()}
+ dimension_names: set[str] = {dimension.name for dimension in
metadata.dimensions}
if not set(query_object.series_columns) <= dimension_names:
raise ValueError("All series columns must be defined in the Semantic
View.")
@@ -1265,7 +1299,9 @@ def _validate_group_limit(query_object:
ValidatedQueryObject) -> None:
)
-def _validate_orderby(query_object: ValidatedQueryObject) -> None:
+def _validate_orderby(
+ query_object: ValidatedQueryObject, metadata: _ViewMetadata
+) -> None:
"""
Validate order by elements in the query object.
"""
@@ -1280,8 +1316,10 @@ def _validate_orderby(query_object:
ValidatedQueryObject) -> None:
"Adhoc expressions in order by are not supported in this Semantic
View."
)
- elements = {orderby[0] for orderby in query_object.orderby}
- metric_names = {metric.name for metric in semantic_view.get_metrics()}
- dimension_names = {dimension.name for dimension in
semantic_view.get_dimensions()}
+ elements: set[str] = {
+ element for element, _ in query_object.orderby if isinstance(element,
str)
+ }
+ metric_names: set[str] = {metric.name for metric in metadata.metrics}
+ dimension_names: set[str] = {dimension.name for dimension in
metadata.dimensions}
if not elements <= metric_names | dimension_names:
raise ValueError("All order by elements must be defined in the
Semantic View.")
diff --git a/tests/unit_tests/semantic_layers/mapper_test.py
b/tests/unit_tests/semantic_layers/mapper_test.py
index 0a7af8aa11b..0aa63832fef 100644
--- a/tests/unit_tests/semantic_layers/mapper_test.py
+++ b/tests/unit_tests/semantic_layers/mapper_test.py
@@ -66,6 +66,7 @@ from superset.semantic_layers.mapper import (
_validate_granularity,
_validate_group_limit,
_validate_metrics,
+ _ViewMetadata,
get_results,
map_query_object,
validate_query_object,
@@ -2988,7 +2989,13 @@ def test_validate_metrics_adhoc_error(
query_object.metrics = [{"label": "adhoc", "sqlExpression": "SUM(x)"}]
with pytest.raises(ValueError, match="Adhoc metrics are not supported"):
- _validate_metrics(query_object)
+ _validate_metrics(
+ query_object,
+ _ViewMetadata(
+ tuple(mock_datasource.implementation.get_metrics()),
+ tuple(mock_datasource.implementation.get_dimensions()),
+ ),
+ )
def test_validate_filters_adhoc_column_error(
@@ -3159,6 +3166,32 @@ def
test_validate_query_object_adhoc_orderby_not_supported_error(
validate_query_object(query_object)
[email protected]("include_unknown_name", [False, True])
+def test_validate_query_object_supported_adhoc_orderby(
+ mock_datasource: MagicMock,
+ include_unknown_name: bool,
+) -> None:
+ """Accept supported expressions while still rejecting unknown named
elements."""
+ mock_datasource.implementation.features |= frozenset(
+ {SemanticViewFeature.ADHOC_EXPRESSIONS_IN_ORDERBY}
+ )
+ query_object: ValidatedQueryObject = ValidatedQueryObject(
+ datasource=mock_datasource,
+ metrics=["total_sales"],
+ columns=["category"],
+ orderby=[
+ ({"label": "custom_order", "sqlExpression": "RAND()"}, True),
+ ("undefined_column" if include_unknown_name else "category",
False),
+ ],
+ )
+
+ if include_unknown_name:
+ with pytest.raises(ValueError, match="All order by elements must be
defined"):
+ validate_query_object(query_object)
+ else:
+ validate_query_object(query_object)
+
+
def test_validate_query_object_orderby_undefined_element_error(
mock_datasource: MagicMock,
) -> None:
@@ -3369,7 +3402,13 @@ def test_validate_granularity_valid(mocker:
MockerFixture) -> None:
query_object.extras = {"time_grain_sqla": "P1D"}
# Should not raise any error - valid granularity with supported time grain
- _validate_granularity(query_object)
+ _validate_granularity(
+ query_object,
+ _ViewMetadata(
+ tuple(mock_datasource.implementation.get_metrics()),
+ tuple(mock_datasource.implementation.get_dimensions()),
+ ),
+ )
def test_validate_group_limit_valid(mocker: MockerFixture) -> None:
@@ -3399,7 +3438,13 @@ def test_validate_group_limit_valid(mocker:
MockerFixture) -> None:
query_object.group_others_when_limit_reached = True
# Should not raise any error - all settings are valid
- _validate_group_limit(query_object)
+ _validate_group_limit(
+ query_object,
+ _ViewMetadata(
+ tuple(mock_datasource.implementation.get_metrics()),
+ tuple(mock_datasource.implementation.get_dimensions()),
+ ),
+ )
def test_get_filters_from_query_object_filter_returns_none(
@@ -4235,6 +4280,99 @@ def test_abc_only_provider_validates_and_maps(mocker:
MockerFixture) -> None:
assert {dim.name for dim in queries[0].dimensions} == {"category"}
[email protected]("entrypoint", ["results", "validate", "map"])
+def test_mapper_fetches_metadata_once(
+ mock_datasource: MagicMock, mocker: MockerFixture, entrypoint: str
+) -> None:
+ """Validation and execution use one observation of each metadata
collection."""
+ provider: MockSemanticView = mock_datasource.implementation
+ metrics: MagicMock = mocker.spy(provider, "get_metrics")
+ dimensions: MagicMock = mocker.spy(provider, "get_dimensions")
+ mocker.patch.object(
+ provider,
+ "get_table",
+ create=True,
+ return_value=SemanticResult(
+ requests=[],
+ results=pa.table({"category": ["Books"], "total_sales": [10.0]}),
+ ),
+ )
+ query: ValidatedQueryObject = ValidatedQueryObject(
+ datasource=mock_datasource,
+ metrics=["total_sales"],
+ columns=["category"],
+ series_columns=["category"],
+ series_limit=2,
+ series_limit_metric="total_sales",
+ orderby=[("total_sales", False)],
+ row_limit=10,
+ )
+ if entrypoint == "results":
+ result: QueryResult = get_results(query)
+ assert result.df.to_dict("records") == [
+ {"category": "Books", "total_sales": 10.0}
+ ]
+ elif entrypoint == "validate":
+ assert validate_query_object(query)
+ else:
+ mapped: list[SemanticQuery] = map_query_object(query)
+ assert [metric.name for metric in mapped[0].metrics] == ["total_sales"]
+ assert (metrics.call_count, dimensions.call_count) == (1, 1)
+
+
+def test_execution_reuses_members_but_next_query_fetches_fresh_metadata(
+ mock_datasource: MagicMock, mocker: MockerFixture
+) -> None:
+ """A second observation cannot change mapping midway or leak to another
query."""
+ provider: MockSemanticView = mock_datasource.implementation
+ original_metrics: set[Metric] = provider.get_metrics()
+ original_dimensions: set[Dimension] = provider.get_dimensions()
+ metrics: MagicMock = mocker.patch.object(
+ provider, "get_metrics", side_effect=[original_metrics, set()]
+ )
+ dimensions: MagicMock = mocker.patch.object(
+ provider, "get_dimensions", side_effect=[original_dimensions, set()]
+ )
+ execute: MagicMock = mocker.patch.object(
+ provider,
+ "get_table",
+ create=True,
+ return_value=SemanticResult(
+ requests=[],
+ results=pa.table({"category": ["Books"], "total_sales": [10.0]}),
+ ),
+ )
+ query: ValidatedQueryObject = ValidatedQueryObject(
+ datasource=mock_datasource, metrics=["total_sales"],
columns=["category"]
+ )
+ result: QueryResult = get_results(query)
+ assert result.df.to_dict("records") == [{"category": "Books",
"total_sales": 10.0}]
+ with pytest.raises(ValueError, match="All metrics must be defined"):
+ get_results(query)
+ assert (metrics.call_count, dimensions.call_count) == (2, 2)
+ execute.assert_called_once()
+
+
+def test_execution_rejects_stale_selection_before_metadata_fetch(
+ mock_datasource: MagicMock, mocker: MockerFixture
+) -> None:
+ provider: MockSemanticView = mock_datasource.implementation
+ metrics: MagicMock = mocker.spy(provider, "get_metrics")
+ dimensions: MagicMock = mocker.spy(provider, "get_dimensions")
+ mocker.patch.object(
+ provider,
+ "validate_selection_version",
+ side_effect=ValueError("stale selection"),
+ )
+ query: ValidatedQueryObject = ValidatedQueryObject(
+ datasource=mock_datasource, metrics=["total_sales"],
columns=["category"]
+ )
+ with pytest.raises(ValueError, match="stale selection"):
+ get_results(query)
+ metrics.assert_not_called()
+ dimensions.assert_not_called()
+
+
def test_required_comparison_completeness_failure_rejects_main_result(
mock_datasource: MagicMock,
mocker: MockerFixture,
