You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "LuisSanchez (via GitHub)" <gi...@apache.org> on 2025/11/25 21:56:38 UTC
[PR] fix(SqlLab): enhance SQL formatting with Jinja template support. [superset]
LuisSanchez opened a new pull request, #36277:
URL: https://github.com/apache/superset/pull/36277
### SUMMARY
Enable Jinja-aware SQL formatting by passing database_id/template_params from the UI and processing templates server-side before formatting.
This is used on the experimental **Parameters** feature of the SQL Labs.
#### Details
- **Backend**:
- `POST /api/v1/sqllab/format_sql/` now accepts `database_id` and `template_params` (JSON string) via `FormatQueryPayloadSchema`.
- Processes Jinja templates (via `get_template_processor`) before running `SQLScript.format`.
- **Frontend (SQL Lab)**:
- `formatQuery` includes `database_id` and `template_params` (stringified when needed) from the up-to-date query editor state in the request body.
- **Tests**:
- Add unit tests for `formatQuery` payload construction and dispatch behavior.
- Add integration test verifying Jinja template resolution in `format_sql` response.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
#### Before
https://github.com/user-attachments/assets/e9ca8146-a300-4cb3-8419-d7f5d0470dee
#### After
https://github.com/user-attachments/assets/52866da2-79ca-4cb0-92f1-3749546f2ee1
### TESTING INSTRUCTIONS
1. Navigate to SQL Lab.
2. Make sure the `examples` DB is selected.
3. Click on the three ellipses on the bottom right > Parameters.
4. Set `{"tbl": "\"Vehicle Sales\""}` as the parameter value.
5. Run `select * from {{tbl}} `and validate the query works.
6. Click on the three ellipses on the bottom right > Format SQL.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue:
- [ ] Required feature flags: `ENABLE_TEMPLATE_PROCESSING`
- [ ] 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: 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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "LuisSanchez (via GitHub)" <gi...@apache.org>.
LuisSanchez commented on code in PR #36277:
URL: https://github.com/apache/superset/pull/36277#discussion_r2565580321
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -914,11 +914,26 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) {
export function formatQuery(queryEditor) {
return function (dispatch, getState) {
- const { sql } = getUpToDateQuery(getState(), queryEditor);
+ const qe = getUpToDateQuery(getState(), queryEditor);
+ const { sql, dbId, templateParams } = qe;
+ const body = { sql };
Review Comment:
Of course, and you're right, there are no more uses of the var `qe`.
--
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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "LuisSanchez (via GitHub)" <gi...@apache.org>.
LuisSanchez commented on code in PR #36277:
URL: https://github.com/apache/superset/pull/36277#discussion_r2565583891
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -914,11 +914,26 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) {
export function formatQuery(queryEditor) {
return function (dispatch, getState) {
- const { sql } = getUpToDateQuery(getState(), queryEditor);
+ const qe = getUpToDateQuery(getState(), queryEditor);
+ const { sql, dbId, templateParams } = qe;
+ const body = { sql };
+
+ // Include database_id and template_params if available for Jinja processing
+ if (dbId) {
+ body.database_id = dbId;
+ }
+ if (templateParams) {
+ // Send templateParams as a JSON string to match the backend schema
+ body.template_params =
+ typeof templateParams === 'string'
+ ? templateParams
+ : JSON.stringify(templateParams);
+ }
+
return SupersetClient.post({
endpoint: `/api/v1/sqllab/format_sql/`,
// TODO (betodealmeida): pass engine as a parameter for better formatting
Review Comment:
Forgot to remove it, we are solving it, done.
--
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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #36277:
URL: https://github.com/apache/superset/pull/36277#discussion_r2565259365
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -914,11 +914,26 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) {
export function formatQuery(queryEditor) {
return function (dispatch, getState) {
- const { sql } = getUpToDateQuery(getState(), queryEditor);
+ const qe = getUpToDateQuery(getState(), queryEditor);
+ const { sql, dbId, templateParams } = qe;
+ const body = { sql };
+
+ // Include database_id and template_params if available for Jinja processing
+ if (dbId) {
+ body.database_id = dbId;
+ }
+ if (templateParams) {
+ // Send templateParams as a JSON string to match the backend schema
+ body.template_params =
+ typeof templateParams === 'string'
+ ? templateParams
+ : JSON.stringify(templateParams);
+ }
+
return SupersetClient.post({
endpoint: `/api/v1/sqllab/format_sql/`,
// TODO (betodealmeida): pass engine as a parameter for better formatting
Review Comment:
Is this TODO relevant here?
##########
superset/sqllab/api.py:
##########
@@ -234,7 +234,34 @@ def format_sql(self) -> FlaskResponse:
"""
try:
model = self.format_model_schema.load(request.json)
- result = SQLScript(model["sql"], model.get("engine")).format()
+ sql = model["sql"]
+ template_params = model.get("template_params")
+ database_id = model.get("database_id")
+
+ # Process Jinja templates if template_params and database_id are provided
+ if template_params and database_id is not None:
+ database = DatabaseDAO.find_by_id(database_id)
+ if database:
+ try:
+ template_params = (
+ json.loads(template_params)
+ if isinstance(template_params, str)
+ else template_params
+ )
+ except json.JSONDecodeError:
+ logger.warning(
+ "Invalid template parameter %s. Skipping processing",
+ str(template_params),
+ )
+ template_params = {}
+
+ if template_params:
+ template_processor = get_template_processor(database=database)
+ sql = template_processor.process_template(
+ sql, **template_params
+ )
Review Comment:
Should this be inside the `try` block?
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -914,11 +914,26 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) {
export function formatQuery(queryEditor) {
return function (dispatch, getState) {
- const { sql } = getUpToDateQuery(getState(), queryEditor);
+ const qe = getUpToDateQuery(getState(), queryEditor);
+ const { sql, dbId, templateParams } = qe;
+ const body = { sql };
Review Comment:
Can we still do this inline? are we using the variable `qe` anywhere else?
--
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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #36277:
URL: https://github.com/apache/superset/pull/36277
--
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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #36277:
URL: https://github.com/apache/superset/pull/36277#issuecomment-3581741123
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/36277?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `70.58824%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 67.57%. Comparing base ([`76d897e`](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0ee3762`](https://app.codecov.io/gh/apache/superset/commit/0ee37623a376cc0be9ac44e3b2c2a84100b32624?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2969 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/36277?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/sqllab/api.py](https://app.codecov.io/gh/apache/superset/pull/36277?src=pr&el=tree&filepath=superset%2Fsqllab%2Fapi.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc3FsbGFiL2FwaS5weQ==) | 66.66% | [3 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/superset/pull/36277?src=pr&el=tree&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 #36277 +/- ##
==========================================
+ Coverage 60.48% 67.57% +7.09%
==========================================
Files 1931 635 -1296
Lines 76236 46724 -29512
Branches 8568 5067 -3501
==========================================
- Hits 46114 31576 -14538
+ Misses 28017 13865 -14152
+ Partials 2105 1283 -822
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.78% <11.76%> (-5.38%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.21% <70.58%> (?)` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.38% <11.76%> (-6.42%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.54% <70.58%> (+4.04%)` | :arrow_up: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.84% <70.58%> (?)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/36277/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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 Sentry](https://app.codecov.io/gh/apache/superset/pull/36277?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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36277:
URL: https://github.com/apache/superset/pull/36277#issuecomment-3612361483
⚠️ **DEPRECATED WORKFLOW** ⚠️
@geido This workflow is deprecated! Please use the new **Superset Showtime** system instead:
- Replace "testenv-up" label with "🎪 trigger-start"
- Better reliability and easier management
- See https://github.com/mistercrunch/superset-showtime for details
Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/19931246345). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "LuisSanchez (via GitHub)" <gi...@apache.org>.
LuisSanchez commented on code in PR #36277:
URL: https://github.com/apache/superset/pull/36277#discussion_r2565403923
##########
superset/sqllab/api.py:
##########
@@ -234,7 +234,34 @@ def format_sql(self) -> FlaskResponse:
"""
try:
model = self.format_model_schema.load(request.json)
- result = SQLScript(model["sql"], model.get("engine")).format()
+ sql = model["sql"]
+ template_params = model.get("template_params")
+ database_id = model.get("database_id")
+
+ # Process Jinja templates if template_params and database_id are provided
+ if template_params and database_id is not None:
+ database = DatabaseDAO.find_by_id(database_id)
+ if database:
+ try:
+ template_params = (
+ json.loads(template_params)
+ if isinstance(template_params, str)
+ else template_params
+ )
+ except json.JSONDecodeError:
+ logger.warning(
+ "Invalid template parameter %s. Skipping processing",
+ str(template_params),
+ )
+ template_params = {}
+
+ if template_params:
+ template_processor = get_template_processor(database=database)
+ sql = template_processor.process_template(
+ sql, **template_params
+ )
Review Comment:
I was thinking on the `try` block to only catch the possible `JSONDecodeError` error.
If this block fails (lines 258 - 262), we may catch another type of exception related to the `process_template`, which I think might be `"SupersetSyntaxErrorException"` or `"SupersetTemplateException"`, but I think those exceptions should propagate.
What do you suggest?
--
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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36277:
URL: https://github.com/apache/superset/pull/36277#issuecomment-3577742447
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] fix(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "LuisSanchez (via GitHub)" <gi...@apache.org>.
LuisSanchez commented on code in PR #36277:
URL: https://github.com/apache/superset/pull/36277#discussion_r2572324450
##########
superset/sqllab/api.py:
##########
@@ -234,7 +234,34 @@ def format_sql(self) -> FlaskResponse:
"""
try:
model = self.format_model_schema.load(request.json)
- result = SQLScript(model["sql"], model.get("engine")).format()
+ sql = model["sql"]
+ template_params = model.get("template_params")
+ database_id = model.get("database_id")
+
+ # Process Jinja templates if template_params and database_id are provided
+ if template_params and database_id is not None:
+ database = DatabaseDAO.find_by_id(database_id)
+ if database:
+ try:
+ template_params = (
+ json.loads(template_params)
+ if isinstance(template_params, str)
+ else template_params
+ )
+ except json.JSONDecodeError:
+ logger.warning(
+ "Invalid template parameter %s. Skipping processing",
+ str(template_params),
+ )
+ template_params = {}
+
+ if template_params:
+ template_processor = get_template_processor(database=database)
+ sql = template_processor.process_template(
+ sql, **template_params
+ )
Review Comment:
Moved the `if` statement inside the `try` block as discussed, 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(SqlLab): enhance SQL formatting with Jinja template support. [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36277:
URL: https://github.com/apache/superset/pull/36277#issuecomment-3612400521
@geido Ephemeral environment spinning up at http://44.249.161.7:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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