mikebridge commented on code in PR #45147:
URL: https://github.com/apache/superset/pull/45147#discussion_r4253218535
##########
superset/semantic_layers/models.py:
##########
@@ -552,42 +556,40 @@ 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
+ and (
+ pa.types.is_string(dimension.type)
+ or pa.types.is_large_string(dimension.type)
+ )
Review Comment:
Codex follow-up: reproduced and fixed. The guard now accepts string-view
plus dictionary-encoded string, large-string and string-view values. Dictionary
types are inspected through their value type, while the original Dimension is
retained in the LIKE filter. Numeric/date dimensions and numeric dictionaries
still use the bounded initial page without LIKE.
The parametrized test uses real Arrow arrays across all nine cases and
checks the single provider call, filter presence and bound. The four newly
supported text cases failed before the fix. The backend/core/async suite now
passes all 983 cases; hooks and independent Claude review passed.
Pushed in `1347e8348d`.
--
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]
