aminghadersohi commented on code in PR #44370:
URL: https://github.com/apache/superset/pull/44370#discussion_r4051464574
##########
superset/datasource/api.py:
##########
@@ -751,6 +751,7 @@ def _build_query_dict(
time_range=payload["time_range"],
time_grain=payload["time_grain"],
grain_column=grain_column,
+
semantic_selection_version=payload.get("semantic_selection_version"),
Review Comment:
This covers the data path. `get_column_values` (api.py:93) accepts only `q`,
so `values_for_column` (models.py:392) resolves a name against
`_unique_dimensions` without reaching `validate_selection_version`. What gates
that route for a caller passing a saved display title?
##########
superset/common/query_object.py:
##########
@@ -405,6 +406,38 @@ def validate(
) -> QueryObjectValidationError | None:
"""Validate query object"""
try:
+ if self.datasource and self.datasource.type == "semantic_view":
+ try:
+ cast(
+ "SemanticView", self.datasource
+ ).implementation.validate_selection_version(
+ self.extras.get("semantic_selection_version")
+ )
Review Comment:
`implementation` (models.py:358) runs `json.loads(self.configuration)`
inside this `try`; `JSONDecodeError` subclasses `ValueError`. Reproduced: a
malformed config surfaces "Saved semantic selections use an older identity
format" though the version sent was current. The hoist keeps 12 tests green.
```suggestion
if self.datasource and self.datasource.type == "semantic_view":
# Resolve outside the guard: `implementation` parses the
stored
# JSON config, and its JSONDecodeError is a ValueError.
implementation = cast("SemanticView",
self.datasource).implementation
try:
implementation.validate_selection_version(
self.extras.get("semantic_selection_version")
)
```
##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -182,17 +186,34 @@ def _resolve_external_view(
error_type="AccessDenied",
)
- display_name = view.name
- valid_columns = {c.column_name for c in view.columns}
- valid_dttm_columns = {c.column_name for c in view.columns if c.is_dttm}
- valid_metrics = {m.metric_name for m in view.metrics}
+ try:
+ view.implementation.validate_selection_version(
+ request.semantic_selection_version
+ )
Review Comment:
Second site of the same shape: `view.implementation` parses stored JSON
inside this `try`, so a malformed configuration returns
`error_type="ValidationError"` with a raw parse string. Also settles the open
bot thread: a stale version here is already a ValidationError. 63 tests pass.
```suggestion
implementation = view.implementation
try:
implementation.validate_selection_version(request.semantic_selection_version)
```
--
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]
