You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "mikebridge (via GitHub)" <gi...@apache.org> on 2026/10/08 20:10:52 UTC

[PR] fix(semantic-layer): fetch view metadata once per query [superset]

mikebridge opened a new pull request, #45104:
URL: https://github.com/apache/superset/pull/45104

   ### SUMMARY
   
   The semantic mapper can call `get_metrics()` four times and `get_dimensions()` five times while validating and executing a grouped chart query. Providers that resolve metadata remotely may pay for each call, and validation and mapping can observe different members.
   
   Capture each metadata collection once per execution and pass that observation through validation and mapping. Preserve provider iteration order, existing result transformations, and the public standalone validation/mapping signatures. Each subsequent execution fetches fresh metadata; nothing is cached across queries or requests.
   
   Also give the public `superset-core` `SemanticView.name` attribute an empty-string default. This is a backward-compatible optional display label; existing provider names and UID-based identity are unchanged. The host continues using its own model name.
   
   Known follow-up: if a metadata lookup itself fails, its provider error can surface before an invalid query input is rejected. Preserving the earlier error ordering would require lazy metadata retrieval; this draft does not change that case.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable; no UI changes.
   
   ### TESTING INSTRUCTIONS
   
   Run the focused host and SDK contract tests with the checkout's core package on the import path:
   
   ```sh
   TZ=UTC PYTHONPATH=.:superset-core/src python -m pytest -o addopts= tests/unit_tests/semantic_layers/ superset-core/tests/semantic_layers/ -q
   ```
   
   With the project's test environment:
   
   1. Run `test_mapper_fetches_metadata_once`: a grouped execution must call each metadata getter once and retain the expected result rows. The pre-fix control calls them four and five times.
   2. Run `test_execution_reuses_members_but_next_query_fetches_fresh_metadata`: changing provider metadata is observed on the next execution of the same query object.
   3. Run `test_execution_rejects_stale_selection_before_metadata_fetch`: an obsolete member-selection version fails before metadata is fetched.
   4. Run `test_view_metadata_defaults_are_conservative` for the SDK defaults. Existing mapper tests cover grains, filtering, grouping, ordering, limits, comparisons and row counts.
   
   Verification: 817 focused tests passed; changed-file pre-commit passed. The whole unit suite produced 22,412 passes, 40 skips, 2 expected failures and 3 failures. All three failures reproduce on untouched master in the same environment: two arm64 long-double cases and the Babel script fixture, which strips `python` from its PATH. Live provider latency was not measured.
   
   ### 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
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #45104:
URL: https://github.com/apache/superset/pull/45104#discussion_r4224009253


##########
superset/semantic_layers/mapper.py:
##########
@@ -125,15 +126,30 @@ class ValidatedQueryObject(QueryObject):
     series_limit_metric: str | None
 
 
+@dataclass(frozen=True)
+class _ViewMetadata:
+    """One query's member observation, preserving provider iteration order."""
+
+    metrics: tuple[Metric, ...]
+    dimensions: tuple[Dimension, ...]
+
+    @classmethod
+    def from_view(cls, view: SemanticView) -> "_ViewMetadata":
+        return cls(tuple(view.get_metrics()), tuple(view.get_dimensions()))
+
+
 def get_results(query_object: QueryObject) -> QueryResult:
     """
     Run 1+ queries based on `QueryObject` and return the results.
 
     :param query_object: The QueryObject containing query specifications
     :return: QueryResult compatible with Superset's query interface
     """
-    if not validate_query_object(query_object):
+    if not query_object.datasource:
         raise ValueError("QueryObject must have a datasource defined.")
+    query_object = cast(ValidatedQueryObject, query_object)
+    metadata: _ViewMetadata = _validation_metadata(query_object)
+    _validate_query_object(query_object, metadata)

Review Comment:
   **Suggestion:** For supported adhoc order expressions, this invokes validation that hashes the expression dictionary and raises `TypeError`, aborting the query.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Type error`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=38481f9061ec4f98b25f7be40127ad8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=38481f9061ec4f98b25f7be40127ad8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/semantic_layers/mapper.py
   **Line:** 152:152
   **Comment:**
   	*Type Error: For supported adhoc order expressions, this invokes validation that hashes the expression dictionary and raises `TypeError`, aborting the query.
   
   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
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45104&comment_hash=936d701ba25623371827571df68ff5024037cec6e013b1cc2af980741df6fa74&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45104&comment_hash=936d701ba25623371827571df68ff5024037cec6e013b1cc2af980741df6fa74&reaction=dislike'>👎</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.

mikebridge commented on PR #45104:
URL: https://github.com/apache/superset/pull/45104#issuecomment-6068700340

   @aminghadersohi @rebenitez1802 ready for review: the semantic-layer mapper now fetches a view's metadata once per query and threads it through, instead of calling the provider four or five times. Results are unchanged.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on PR #45104:
URL: https://github.com/apache/superset/pull/45104#issuecomment-6068833983

   ## CodeAnt PR Risk: Low Risk
   
   - The PR appears safe to merge; validation and mapping now use one consistent metadata snapshot per query.
   - Tests cover metadata fetch counts, fresh metadata on subsequent queries, and stale selections being rejected before metadata is fetched.
   
   **Assessed commit:** `5eb61712fa2b`


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.

codecov[bot] commented on PR #45104:
URL: https://github.com/apache/superset/pull/45104#issuecomment-6068193208

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45104?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 82.67%. Comparing base ([`34b3d65`](https://app.codecov.io/gh/apache/superset/commit/34b3d65cc208600e0152017ba0db705a3d2b57bd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5eb6171`](https://app.codecov.io/gh/apache/superset/commit/5eb61712fa2ba9091d2259c23b38b64ab674aaf5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #45104      +/-   ##
   ==========================================
   + Coverage   82.64%   82.67%   +0.02%     
   ==========================================
     Files        3007     3012       +5     
     Lines      187318   187916     +598     
     Branches    43399    43398       -1     
   ==========================================
   + Hits       154811   155354     +543     
   - Misses      29742    29797      +55     
     Partials     2765     2765              
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/45104/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [superset-extensions-cli](https://app.codecov.io/gh/apache/superset/pull/45104/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `91.24% <ø> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45104?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.

aminghadersohi commented on code in PR #45104:
URL: https://github.com/apache/superset/pull/45104#discussion_r4226120693


##########
superset/semantic_layers/mapper.py:
##########
@@ -1278,7 +1314,7 @@ def _validate_orderby(query_object: ValidatedQueryObject) -> None:
         )
 
     elements = {orderby[0] for orderby in query_object.orderby}

Review Comment:
   Pre-existing on master, not introduced here: with `ADHOC_EXPRESSIONS_IN_ORDERBY` advertised, a dict orderby element raises `TypeError: unhashable type: 'dict'` here, although mapping supports it. Skipping non-str elements fixes it and still rejects unknown names.
   ```suggestion
       elements = {
           element for element, _ in query_object.orderby if isinstance(element, str)
       }
   ```



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.

mikebridge commented on code in PR #45104:
URL: https://github.com/apache/superset/pull/45104#discussion_r4231471687


##########
superset/semantic_layers/mapper.py:
##########
@@ -1278,7 +1314,7 @@ def _validate_orderby(query_object: ValidatedQueryObject) -> None:
         )
 
     elements = {orderby[0] for orderby in query_object.orderby}

Review Comment:
   Thanks, applied your suggestion in a09648e575: only string elements are checked against the view's metric and dimension names, so a dict order-by element no longer raises `TypeError`.
   
   Two new mapper tests cover a dict element when `ADHOC_EXPRESSIONS_IN_ORDERBY` is advertised, and confirm an unknown string name is still rejected.
   



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.

mikebridge commented on PR #45104:
URL: https://github.com/apache/superset/pull/45104#issuecomment-6090691141

   @aminghadersohi the codeant thread at `mapper.py:153` is resolved now (addressed in a09648e575), so this should be ready for your approval whenever you have a moment. Thanks!


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.

netlify[bot] commented on PR #45104:
URL: https://github.com/apache/superset/pull/45104#issuecomment-6083463892

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | a09648e5753dbad7ceb2e58ef6f8966db1e71127 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ac900b942301d0008b39521 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45104--superset-docs-preview.netlify.app](https://deploy-preview-45104--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ1MTA0LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.ZsVulAsrXbGVnQog1JPSHwrfxisGU8ZCnYSRUuENNoY)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcoxDoAgDADAv3SWpKAS6w98BhQMg0CDLMb4d9luuBek1SwddoAJfHOF0_DNSmsitCo7kdjUGTsnVQvH0UKUqz5HGNE6JkRPi5lRB0Tc_Eyr0fD9) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-45104--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(semantic-layer): fetch view metadata once per query [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.

aminghadersohi merged PR #45104:
URL: https://github.com/apache/superset/pull/45104


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org