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]

Reply via email to