Repository navigation
test(semantic-layer): validate provider-declared default time dimensions - #45893
Open
mikebridge wants to merge 4 commits into
Open
mikebridge wants to merge 4 commits into
mikebridge wants to merge 4 commits into
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #45893 +/- ##
==========================================
+ Coverage 84.99% 85.02% +0.02%
==========================================
Files 3080 3085 +5
Lines 209131 209759 +628
Branches 49278 49278
==========================================
+ Hits 177755 178342 +587
- Misses 27993 28035 +42
+ Partials 3383 3382 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
doc
sadpandajoe
added
review:checkpoint
mikebridge
marked this pull request as ready for review
November 5, 2026 02:10
mikebridge
requested review from
geido,
michael-s-molina,
msyavuz,
rusackas,
sadpandajoe and
villebro
as code owners
November 5, 2026 02:10
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Comment on lines
+116
to
+127
| assert any( | ||
| dimension.name == preferred | ||
| and ( | ||
| pa.types.is_date(dimension.type) | ||
| or pa.types.is_timestamp(dimension.type) | ||
| or pa.types.is_time(dimension.type) | ||
| ) | ||
| for dimension in view.get_dimensions() | ||
| ), ( | ||
| "preferred_temporal_dimension must name " | ||
| "an exposed temporal dimension" | ||
| ) |
Contributor
There was a problem hiding this comment.
Suggestion: When a name has a non-temporal raw variant and a temporal grain variant, this accepts it, but Superset collapses to raw and ignores the declared default.
Assessment: 🟠 Major · 🔁 Occurrence: Rarely · 🏷️ Api mismatch
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-core/src/superset_core/semantic_layers/testing.py
**Line:** 116:127
**Comment:**
*Api Mismatch: When a name has a non-temporal raw variant and a temporal grain variant, this accepts it, but Superset collapses to raw and ignores the declared default.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Contributor
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
TL;DR: Validate provider-declared default time dimensions in the reusable SDK conformance suite, and document how existing chart defaults use them.
Background: A semantic view can already declare its preferred exposed time dimension. The host validates that name and supplies it to Explore and query construction.
Problem: The conformance suite did not reject invalid or unstable declarations, and the existing documentation could imply that all saved-chart results remain unchanged when a provider adds a default.
What this changes: Check optional declarations against exposed temporal names and equivalent catalog reloads. Add metadata, frontend initialization, and explicit/implicit query-binding tests. Clarify the compatibility guarantee: explicit saved bindings remain authoritative, but omitted bindings may adopt a provider's new default and change results without saving the chart. An explicit temporal filter does not prevent omitted granularity from receiving default-date bounds for an independently supplied time range.
The check reuses existing provider fixtures and accepts Arrow date, timestamp and time types, including grain variants sharing a name. Providers that declare None remain conforming. This PR changes no host production behavior, provider selection policy, stored chart, migration, or feature-flag default.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: SDK test utility, characterization tests and documentation only. Live provider/browser acceptance is separate.
TESTING INSTRUCTIONS
PYTHONPATH=superset-core/src python -m pytest -q -o addopts='' superset-core/tests/semantic_layers/test_conformance.py(78 passed, four existing optional-capability skips).python -m pytest -q tests/unit_tests/semantic_layers/models_test.py -k 'preference or preferred_temporal'(10 passed).python -m pytest -q tests/unit_tests/common/test_query_context_factory.py(91 passed).superset-frontend, runnpm run test -- --runInBand packages/superset-ui-chart-controls/test/utils/getTemporalColumns.test.ts(10 passed).ADDITIONAL INFORMATION
SEMANTIC_LAYERSfor host semantic behavior; no default changeProposed title:
test(semantic-layer): validate provider time defaults