You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "lbotino (via GitHub)" <gi...@apache.org> on 2026/05/22 00:16:16 UTC
[PR] feat: pass chart context to SQL query mutator [superset]
lbotino opened a new pull request, #40338:
URL: https://github.com/apache/superset/pull/40338
Expose dashboard and slice identifiers to SQL_QUERY_MUTATOR for chart queries so warehouse query comments can map jobs back to Superset dashboards.
--
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] feat: pass chart context to SQL query mutator [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40338:
URL: https://github.com/apache/superset/pull/40338#issuecomment-4513963978
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | d2356b8304541fbfa69810b03261a7914abea2a3 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a0fa69bd7d2b20008644ebf |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-40338--superset-docs-preview.netlify.app](https://deploy-preview-40338--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><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#DctJDoAgDEDRu3QtkRiDww08RoESFzJYayIx3l12f_H-C4VzLAIrQAeWMbm9dSCU_joPdd7EVcVbUDIrtyOLcjkJPdK8p3Lkuvl2GNQBzWL95Ac7aK1nM45kA3w_) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40338--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] feat: pass chart context to SQL query mutator [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40338:
URL: https://github.com/apache/superset/pull/40338#issuecomment-4524232097
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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] feat: pass chart context to SQL query mutator [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 #40338:
URL: https://github.com/apache/superset/pull/40338#discussion_r3285081371
##########
superset/models/helpers.py:
##########
@@ -916,7 +917,10 @@ def get_query_str_extended(
sql = self._apply_cte(sql, sqlaq.cte)
if mutate:
- sql = self.database.mutate_sql_based_on_config(sql)
+ sql = self.database.mutate_sql_based_on_config(
+ sql,
+ **(mutation_context or {}),
+ )
Review Comment:
**π Architect Review β HIGH**
When app.config["MUTATE_AFTER_SPLIT"] is True, SQL_QUERY_MUTATOR is only invoked from the split-path (Database._execute_sql_with_mutation_and_logging / db_engine_specs.BaseEngineSpec.estimate_query_cost) where mutate_sql_based_on_config is called with is_split=True and no mutation_context, while the pre-split call in get_query_str_extended (which is now the only place that receives dashboard_id/slice_id) is skipped by the MUTATE_AFTER_SPLIT gate, so chart queries executed in this valid configuration never pass dashboard/slice context to the mutator.
**Suggestion:** Thread the same mutation_context through to the is_split=True calls to Database.mutate_sql_based_on_config (e.g. via get_df/_execute_sql_with_mutation_and_logging), so SQL_QUERY_MUTATOR receives identical chart metadata regardless of the MUTATE_AFTER_SPLIT setting.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=906afd9c62884fa3874257ccc69d670b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=906afd9c62884fa3874257ccc69d670b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings β not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** superset/models/helpers.py
**Line:** 920:923
**Comment:**
*HIGH: When app.config["MUTATE_AFTER_SPLIT"] is True, SQL_QUERY_MUTATOR is only invoked from the split-path (Database._execute_sql_with_mutation_and_logging / db_engine_specs.BaseEngineSpec.estimate_query_cost) where mutate_sql_based_on_config is called with is_split=True and no mutation_context, while the pre-split call in get_query_str_extended (which is now the only place that receives dashboard_id/slice_id) is skipped by the MUTATE_AFTER_SPLIT gate, so chart queries executed in this valid configuration never pass dashboard/slice context to the mutator.
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.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix β do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
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>
--
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] feat: pass chart context to SQL query mutator [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40338:
URL: https://github.com/apache/superset/pull/40338#issuecomment-4513830215
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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] feat: pass chart context to SQL query mutator [superset]
Posted by "lbotino (via GitHub)" <gi...@apache.org>.
lbotino closed pull request #40338: feat: pass chart context to SQL query mutator
URL: https://github.com/apache/superset/pull/40338
--
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] feat: pass chart context to SQL query mutator [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 #40338:
URL: https://github.com/apache/superset/pull/40338#discussion_r3285092995
##########
superset/common/query_context_processor.py:
##########
@@ -276,7 +290,10 @@ def get_query_result(self, query_object: QueryObject) -> QueryResult:
# todo(hugh): add logic to manage all sip68 models here
result = query_context.datasource.exc_query(query_object.to_dict())
else:
- result = query_context.datasource.query(query_object.to_dict())
+ result = query_context.datasource.query(
+ query_object.to_dict(),
+ mutation_context=self._get_sql_mutation_context(),
+ )
Review Comment:
**Suggestion:** The new mutation context is only wired into the non-`Query` datasource path, so charts backed by `query`/`saved_query` datasources still bypass this context entirely. That means dashboard and slice identifiers are not exposed for those chart queries, which makes the feature incomplete and breaks job-to-dashboard mapping for that datasource type. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β οΈ Saved-query charts lack dashboard_id in SQL mutator.
- β οΈ Saved-query charts lack slice_id in SQL mutator.
- β οΈ Job-to-dashboard mapping incomplete for query datasources.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Configure a SQL query mutator in `superset/config.py` (e.g. `SQL_QUERY_MUTATOR =
my_mutator`) that inspects incoming kwargs and logs `dashboard_id` and `slice_id` when
present.
2. Create two charts: (a) a regular chart based on a physical table datasource (SqlaTable)
and (b) a chart based on a `query`/saved_query datasource (backed by the `Query` model
referenced in `superset/common/query_context_processor.py:702` via
`isinstance(self._qc_datasource, Query)`), and add both to the same dashboard.
3. Load the dashboard so that Flask hits the chart data endpoint, which builds a
`QueryContext` and invokes `QueryContextProcessor.get_query_result()` in
`superset/common/query_context_processor.py:281`.
4. Observe that for the table-based chart, `get_query_result` takes the `else` branch and
calls `query_context.datasource.query(query_object.to_dict(),
mutation_context=self._get_sql_mutation_context())` at
`superset/common/query_context_processor.py:293β296`, so the mutator receives
`dashboard_id`/`slice_id`; but for the saved-query chart, `query_context.datasource` is a
`Query` instance so `get_query_result` uses
`query_context.datasource.exc_query(query_object.to_dict())` at
`superset/common/query_context_processor.py:291` and the separate
`processing_time_offsets` path uses
`self._qc_datasource.exc_query(query_object_clone_dct)` at
`superset/common/query_context_processor.py:702`βneither call path passes
`mutation_context`, so the configured `SQL_QUERY_MUTATOR` never sees
`dashboard_id`/`slice_id` for query-backed charts.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c560f23800df40d587dabc4d21864745&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c560f23800df40d587dabc4d21864745&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/common/query_context_processor.py
**Line:** 293:296
**Comment:**
*Incomplete Implementation: The new mutation context is only wired into the non-`Query` datasource path, so charts backed by `query`/`saved_query` datasources still bypass this context entirely. That means dashboard and slice identifiers are not exposed for those chart queries, which makes the feature incomplete and breaks job-to-dashboard mapping for that datasource type.
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%2F40338&comment_hash=99f7daa377259d466aec787ad3af7f87871714a6974e7040f7098cf6dc683305&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40338&comment_hash=99f7daa377259d466aec787ad3af7f87871714a6974e7040f7098cf6dc683305&reaction=dislike'>π</a>
##########
superset/models/core.py:
##########
@@ -666,6 +671,7 @@ def mutate_sql_based_on_config(self, sql_: str, is_split: bool = False) -> str:
sql_,
security_manager=security_manager,
database=self,
+ **kwargs,
)
Review Comment:
**Suggestion:** Forwarding arbitrary `**kwargs` directly into `SQL_QUERY_MUTATOR` introduces a backward-incompatible runtime break for existing mutator functions that only accept `sql`, `security_manager`, and `database` (as documented in existing examples). Once chart queries start passing `dashboard_id`/`slice_id`/`form_data`, those deployments will raise `TypeError: got an unexpected keyword argument ...`. Keep compatibility by filtering supported kwargs or gracefully falling back when the mutator doesn't accept extra parameters. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Chart queries fail when SQL_QUERY_MUTATOR lacks **kwargs.
- β Dashboards break on instances with existing mutators.
- β οΈ Deployments risk runtime errors after upgrading.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. In `superset/config.py`, configure `SQL_QUERY_MUTATOR` with the previously documented
signature that does not accept arbitrary kwargs, for example:
`def my_mutator(sql, security_manager=None, database=None): return sql`; then set
`SQL_QUERY_MUTATOR = my_mutator`.
2. Create a regular chart based on a physical table datasource (SqlaTable), add it to a
dashboard, and open the dashboard so that Flask invokes the chart data API, which builds a
`QueryContext` and calls `QueryContextProcessor.get_query_result()` in
`superset/common/query_context_processor.py:281`.
3. In `get_query_result`, for non-`Query` datasources the code calls
`query_context.datasource.query(query_object.to_dict(),
mutation_context=self._get_sql_mutation_context())` at
`superset/common/query_context_processor.py:293β296`, where `_get_sql_mutation_context()`
includes `dashboard_id`, `slice_id`, and `form_data` (lines 267β279) in the
`mutation_context` dict.
4. Inside the SQLAlchemy datasource implementation (`SqlaTable.query` in
`superset/connectors/sqla/models.py`), the `mutation_context` dict is forwarded down to
the database layer, which calls `Database.mutate_sql_based_on_config(...)` in
`superset/models/core.py:652β657`; the new implementation invokes `sql_mutator(sql_,
security_manager=security_manager, database=self, **kwargs)` at
`superset/models/core.py:671β675`, causing Python to raise `TypeError: my_mutator() got an
unexpected keyword argument 'dashboard_id'` (or similar) because `my_mutator` does not
accept these extra kwargs, resulting in a 500 error for the chart query.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f7d38c2760894922ad187858ed225b93&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f7d38c2760894922ad187858ed225b93&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/core.py
**Line:** 671:675
**Comment:**
*Api Mismatch: Forwarding arbitrary `**kwargs` directly into `SQL_QUERY_MUTATOR` introduces a backward-incompatible runtime break for existing mutator functions that only accept `sql`, `security_manager`, and `database` (as documented in existing examples). Once chart queries start passing `dashboard_id`/`slice_id`/`form_data`, those deployments will raise `TypeError: got an unexpected keyword argument ...`. Keep compatibility by filtering supported kwargs or gracefully falling back when the mutator doesn't accept extra parameters.
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%2F40338&comment_hash=a171803f820f1567579330a082376d28b9cd25b6b1eba71fe0a950c7f104c31c&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40338&comment_hash=a171803f820f1567579330a082376d28b9cd25b6b1eba71fe0a950c7f104c31c&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] feat: pass chart context to SQL query mutator [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40338:
URL: https://github.com/apache/superset/pull/40338#issuecomment-4513854980
<!-- Bito Reply -->
The pr_comments.csv file appears to be empty or contains only header information with no actual comment data. I cannot analyze or provide information about review comments when there is no content to process.
--
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