mapledan opened a new pull request, #42785:
URL: https://github.com/apache/superset/pull/42785
### SUMMARY
`QueryEstimationCommand.run()` renders Jinja only when `template_params` is
non-empty. But `get_time_filter()`, `current_username()` and friends need no
declared parameter, and SQL Lab always POSTs `template_params: {}` to
`/api/v1/sqllab/estimate/` — so rendering is skipped, the raw `{%` reaches
`SQLScript()`, and the user gets **Issue 1003, "there is a syntax error in the
SQL query, perhaps there was a misspelling or a typo"**, for a query that runs
fine in SQL Lab.
Rendering unconditionally would silence the error but estimate the wrong
thing: a template expands at run time, and with no dashboard in play
`get_time_filter()` yields no filter at all, so the reported cost would be for
a query missing its time predicate. The query is refused instead, with an
explanation.
Whether SQL carries a template is asked of the template processor (new
`has_template()`), which lexes with its own environment — so
`ENABLE_TEMPLATE_PROCESSING` being off, and SQL that merely contains braces
such as `'{{1,2},{3,4}}'`, both behave correctly.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
#### Before
A valid query is reported as a typo.
<img width="1280" height="720" alt="before"
src="https://github.com/user-attachments/assets/b18a3c07-9816-4438-8a86-1537421a2924"
/>
#### After
The reason is stated, and what to do about it.
<img width="1280" height="720" alt="after"
src="https://github.com/user-attachments/assets/68790873-4d8b-440a-b83c-1f440d9a9102"
/>
### TESTING INSTRUCTIONS
1. Enable `ESTIMATE_QUERY_COST` and `ENABLE_TEMPLATE_PROCESSING` in
`FEATURE_FLAGS`.
2. Add `"cost_estimate_enabled": true` to a Postgres database's `extra`.
3. In SQL Lab against that database, run this query — it succeeds:
```sql
SELECT 1 AS n
{% set tf = get_time_filter(strftime="%Y-%m-%d") %}
{% if tf.from_expr %} WHERE 1 = 1 {% endif %}
```
4. Click **Estimate cost**. Before this change: `Error parsing near '{%' at
line 2:2`. After: the explanation above.
5. Confirm plain SQL is unaffected — `SELECT 1` still estimates normally.
6. Confirm SQL that merely looks templated is unaffected — `SELECT
'{{1,2},{3,4}}'::int[]` still estimates normally.
Unit tests: `pytest tests/unit_tests/commands/sql_lab/test_estimate.py
tests/unit_tests/jinja_context_test.py`
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] 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: [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]