mikebridge opened a new pull request, #45389:
URL: https://github.com/apache/superset/pull/45389
### SUMMARY
When a chart on a semantic view selects a time column but doesn't specify a
sort, Superset expects the rows in chronological order. Until now that rule
lived only in each semantic-layer provider. The SDK never stated it, and
Superset never applied it, so a provider written from the SDK alone could
return an unordered time series. This change writes the rule into the SDK and
has Superset apply it.
**User-visible after deploy:** on semantic views whose results Superset
caches, unordered time-series results stored before the upgrade are recomputed
once on their next request. Value keys don't change and no cache is flushed.
Rows and ordering stay the same for providers that already applied this default
themselves.
**The rule (stated on `SemanticQuery` in
`superset_core.semantic_layers.types`):**
- **When it applies:** `order` is empty (`None` or `[]`) and `dimensions`
includes a date, timestamp or time dimension.
- **What it does:** Superset sorts ascending by the first such dimension, by
its position in `dimensions`. It sorts before `offset` and `limit`. Rows that
tie on that dimension come back in no particular order, and where NULL values
sort is not specified.
- **A non-empty `order` is used exactly as given.** The default is never
added in front of it.
**Where Superset applies it:**
- **Provider dispatch:** applied in `mapper._dispatch_semantic_query`, so it
covers the main query, time-offset queries and row counts. Queries that already
have an order reach the provider unchanged.
- **Semantic containment cache:** each entry records the effective order,
including the default. Exact and projected reuse keep the cached row order, so
they require a matching order. A limited result cut by one temporal dimension
is therefore never reused for a query whose default is a different one. A
rollup comes from an unlimited entry and is re-sorted by the query's default
before its limit is applied, so the order the entry was stored in doesn't
matter.
- **Rollup reuse stays available for unordered queries.** That is why the
default is not baked into the mapped query.
- **Shared helper:** one helper, `superset/semantic_layers/ordering.py`,
serves both paths.
**HAVING:** `PredicateType.HAVING` is documented as reserved. The chart-data
path already rejects HAVING extras before mapping, so providers don't receive
HAVING predicates for chart data.
**Compatibility:**
- The rule matches what existing providers already do: the first selected
temporal dimension, ascending, and only when no order is given. Providers can
keep their own copy of the rule. A host-filled order produces the same query
they built before.
- The cache identity format version is unchanged. Descriptors written before
the upgrade are replaced on their next miss, as described above.
- **During a rolling deploy,** hosts that are not yet upgraded don't reuse
entries written by upgraded hosts for unordered time-series queries, and the
reverse also holds. Those requests miss until every host is upgraded. Results
stay correct.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: no UI change. Before, an unordered time-series query reached
the provider with no order, and the semantic cache could serve some limited or
rolled-up hits in grouping order. After, the provider receives the
chronological default, and cache hits follow the same order.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/semantic_layers/default_order_test.py \
tests/unit_tests/semantic_layers/cache_transform_test.py \
tests/unit_tests/semantic_layers/cache_policy_test.py \
tests/unit_tests/semantic_layers/cache_repository_test.py \
tests/unit_tests/semantic_layers/mapper_test.py
```
The new tests cover:
- **Dispatch:** the first temporal dimension wins when several are selected,
ASC is applied before the limit, and with no temporal dimension the query stays
unordered. Explicit orders reach the provider unchanged, as the same object.
- **Cache identity:** it stays as mapped.
- **Round trips through the real cache repository, reuse policy and
transform:**
- A rolled-up hit with a limit returns the earliest rows overall rather
than in grouping order.
- A limited entry is not reused for the same dimensions in a different
order.
- Unlimited entries still roll up, both when the time dimension is dropped
and when the entry's default sorted by a different temporal dimension.
Before the change, the default-order and cache-hit cases fail; the controls
pass.
The semantic-layer and superset-core suites pass, and changed-file
pre-commit passes, including mypy and pylint. No live semantic provider was
used.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
