mikebridge opened a new pull request, #45912:
URL: https://github.com/apache/superset/pull/45912
### SUMMARY
**TL;DR:** When a semantic query has no sort order, Superset sorts it by its
first date column, oldest first. NULL dates now always go last, both from the
provider and from Superset's cache, so a limited chart shows the same rows
either way. **Upgrading recomputes every cached semantic-layer result once**,
because the cache key format changes.
**Background:**
- A semantic query without an explicit order gets a default: ascending by
the first selected date, timestamp or time dimension. The host sends that order
to the provider and also uses it to re-sort rows it rolls up from its cache.
- The SDK never said where NULL values of that dimension go. The cache put
them last, while some engines put NULL first in ascending order (BigQuery,
MySQL, Databricks; Snowflake depends on a session setting).
**Problem:**
- With a limit, those two answers differ. For dates `[NULL, January,
February]` and a limit of 2, a NULL-first provider returns NULL and January,
but a cache rollup returns January and February.
- The provider also couldn't tell the generated default apart from a user's
explicit "date ascending", so it couldn't apply a different rule to the default
alone.
**Changes:**
- **SDK (`superset-core`):** an order term can now carry an optional NULL
placement.
- The new `OrderTerm(member, direction, nulls=None)` still unpacks as a
`(member, direction)` pair, so existing providers keep working unchanged.
- `null_placement(term)` reads the placement from either form; `None`
means the provider's default, as before.
- A term with a placement differs from the plain pair; one without equals
it.
- **Host:** the generated default is sent as `OrderTerm(dimension, ASC,
NullPlacement.LAST)`. Explicit orders are sent exactly as given and keep the
provider's NULL placement.
- **Cache:** the cache identity includes the placement, so a page cut with
NULL values last is never reused for an explicit ascending query, or the
reverse. Rolled-up rows sort NULL values last explicitly. The identity format
moves to `v6`, so existing semantic cache entries are recomputed once.
- **Conformance kit:** a new required `null_placement` query case checks
that a provider applies a placement before `offset`/`limit`. The SQLite
reference provider renders `NULLS FIRST/LAST`.
- **Docs:**
- `UPDATING.md` covers the provider obligation, the kit case and the
one-time cache recompute;
- the provider testing guide describes the new case.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; no UI change. For a chart whose time column has NULL values
and that has a row limit:
- **Before:** a NULL-first provider and a cache hit could show different
rows.
- **After:** both show the earliest dates, with NULL dates last.
### TESTING INSTRUCTIONS
- `pytest superset-core/tests/semantic_layers
tests/unit_tests/semantic_layers`
- These tests failed before the change:
-
`default_order_test.py::test_default_order_differs_from_an_explicit_ascending_order`;
- `test_limited_hit_is_not_shared_between_default_and_explicit_ascending`;
- `test_conformance.py::...::test_query_outputs[null_placement]` against a
provider that ignores the placement.
### 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))
- [x] Introduces new feature or API (`OrderTerm`, `NullPlacement`,
`null_placement` in the semantic-layer SDK; a new required conformance-kit case)
- [ ] Removes existing feature or API
**Notes for reviewers:**
- **Expected conflicts:** with the open semantic-layer transport PR
(#45553), in the kit's imports and the top of `UPDATING.md`. Both are textual.
- **`dataclasses.asdict`:** it works on a query holding an `OrderTerm` but
keeps only member and direction (documented on the type).
--
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]
