aminghadersohi commented on code in PR #45147:
URL: https://github.com/apache/superset/pull/45147#discussion_r4252876108
##########
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:
`get_column_type` (line 118) classifies every non-numeric, non-temporal,
non-boolean Arrow type as STRING, so the picker treats these columns as text.
This guard only narrows `string` and `large_string`. With a dimension typed
`pa.string_view()` or `pa.dictionary(pa.int32(), pa.string())`,
`values_for_column("c", search="zz")` calls `get_values(dim, None)` and returns
the unfiltered first page. On https://github.com/apache/superset/pull/45110 the
same call sent `LIKE '%zz%'`, so values past the first page can no longer be
found. I reproduced this with a scratch test. Suggested change: also accept
`pa.types.is_string_view(t)`, and a dictionary whose `value_type` passes the
same check.
--
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]
