mapledan commented on PR #42785:
URL: https://github.com/apache/superset/pull/42785#issuecomment-5713619630
All six are addressed, and pushed — replies on each thread rather than
repeated
here. They had one shape in common, so most of the fix was to stop doing
separately what the execution path already does: the detector is gone in
favour
of `SqlQueryRenderImpl`'s undeclared-variable check (now
`BaseTemplateProcessor.get_undefined_parameters`, called by both paths), the
rendered SQL is re-authorized as `_validate_rendered_access` does, and the
processor takes a schema.
One addition rather than a removal, since it belongs to no single thread: SQL
that stops parsing *because* a parameter was left in it is now read as
written
rather than reported as a syntax error. Without it `SELECT * FROM {{ tbl }}`
goes back to complaining about a brace — the report this PR was opened for —
with the parameter sitting right there to name. It is one
`except SupersetParseError: pass` around the comment-stripping, and it
applies
to Run too, which is where the behaviour was already intended.
Verified locally, since the re-review environment could not run the suite:
`tests/unit_tests` passes 11335, with the same 34 pre-existing environment
failures the unmodified base produces (duckdb, prophet, s3, cron — none in
sqllab or jinja; I ran the base with the change stashed to confirm the count
matches). `pre-commit` clean. Behaviour checked through a real
`JinjaTemplateProcessor` rather than a mock:
| SQL, no params | rendered | reported |
|---|---|---|
| `SELECT '{{ ds }}' AS d` | unchanged | `ds` is undefined |
| `SELECT * FROM {{ tbl }}` | unchanged | `tbl` is undefined |
| `SELECT 1 {# note #}` | `SELECT 1` | nothing, estimated |
| `{% set tf = get_time_filter('ds') %}SELECT 1 AS n {% if tf %}{% endif %}`
| `SELECT 1 AS n` | nothing, estimated |
The last row is the original report; rendering alone fixes it.
@rusackas — this removes the code your commit fixed the test for. I left that
commit in history rather than squashing it away, so the sequence reads as it
happened.
--
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]
