mikebridge commented on code in PR #45147:
URL: https://github.com/apache/superset/pull/45147#discussion_r4252481976
##########
superset/semantic_layers/models.py:
##########
@@ -552,42 +552,27 @@ def values_for_column(
a metric name included — which the endpoint reports as the caller's
error naming the column, exactly as a dataset does.
"""
- dimensions = {
+ dimensions: dict[str, Dimension] = {
dimension.name: dimension for dimension in self._unique_dimensions
}
if column_name not in dimensions:
raise KeyError(column_name)
- dimension = dimensions[column_name]
-
- if search:
- narrowing = Filter(
+ dimension: Dimension = dimensions[column_name]
+ narrowing: Filter | None = (
+ Filter(
type=PredicateType.WHERE,
column=dimension,
operator=Operator.LIKE,
value=f"%{search}%",
)
- try:
- with provider_completeness():
- result = self.implementation.get_values(dimension,
{narrowing})
- except SemanticResultCompletenessError:
- raise
- except Exception: # pylint: disable=broad-exception-caught
- # The narrowing filter is best-effort: a provider that cannot
- # apply it must degrade to the bounded first page (the picker
- # still narrows within it), never to an error — but say so,
- # or the degradation is the next silent failure.
- logger.warning(
- "Semantic view %s rejected the value-search filter on "
- "dimension %s; returning the unfiltered page",
- self.uuid,
- dimension.name,
- exc_info=True,
- )
- with provider_completeness():
- result = self.implementation.get_values(dimension, None)
- else:
- with provider_completeness():
- result = self.implementation.get_values(dimension, None)
+ if search
+ else None
+ )
+ result: SemanticResult = execute_semantic_query(
Review Comment:
Codex follow-up: you were right about the numeric/date picker path. I
applied your type guard: only string and large-string dimensions receive the
host-built LIKE filter. Numeric and date searches use one sorted, bounded
values call without LIKE, keeping those suggestions available without retrying
a failed filtered request.
Typed provider rejections now emit a WARNING naming the view, dimension and
closed error code; search text and provider diagnostics are omitted. No
frontend change is included. A rejected text search still returns the
classified 400 and never retries unfiltered; the current picker still treats
that 400 quietly. This remains a limitation for string providers that cannot
apply LIKE, rather than restoring a retry that drops the search constraint.
The new tests failed first for numeric/date filtering and missing logging,
then passed with the fix. They use actual integer/date/string Arrow arrays,
assert the single-call filter shape and bound, and check the warning. The
focused semantic-layer/core/async suite passes all 962 cases, including the
existing no-retry, no-cache-write and guest-sanitization cases.
Pushed in `8213a3d2ba`; changed-file hooks and independent Claude review
passed.
##########
superset/datasource/api.py:
##########
@@ -290,6 +295,8 @@ def _column_values_response(
array_elements=array_elements,
search=search,
)
+ except (SemanticLayerQueryRejectedError, SemanticLayerExecutionError)
as ex:
+ return json_error_response([ex.error], status=ex.status)
Review Comment:
Codex follow-up: I aligned this branch with the route’s documented error
envelope using `self.response(ex.status, message=ex.message)`. Typed rejections
now return `{"message": ...}` at 400 and classified provider faults use the
same shape at 500, matching the other branches and chart-data API.
The guest/non-guest response tests now assert the complete message envelope
for both classifications and still verify that provider diagnostics are absent
and failed calls are not cached. This also makes the envelope consistent with
FAB’s generic message response for a malformed provider result; I did not
change that separate result-validation path. The developer documentation
records the message shape.
Pushed in `8213a3d2ba`; changed-file hooks and independent Claude review
passed.
--
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]
