eschutho opened a new pull request, #42366:
URL: https://github.com/apache/superset/pull/42366
### SUMMARY
Virtual dataset SQL templates that index into `filter_values()` (e.g.
`filter_values('col')[0]`) can raise a Jinja `UndefinedError` when no dashboard
filter is active for that column. This fix catches that error in
`get_rendered_sql()` and surfaces it as a clear, chart-level validation error
instead.
### PROBLEM
When a virtual dataset's SQL template does something like:
```sql
WHERE col = {{ filter_values('col')[0] }}
```
and there are no active dashboard filters for `col`, `filter_values('col')`
returns an empty list. Depending on how the templated value is used further in
the query, indexing or operating on that empty list can raise
`jinja2.exceptions.UndefinedError` (e.g. `list object has no element 0`) during
Jinja rendering. This exception was not translated into a Superset-specific
validation error, so it could surface as a raw, unhandled 500 instead of a
clear, actionable error message shown at the chart level.
### FIX
`get_rendered_sql()` in `superset/models/helpers.py` now explicitly catches
`jinja2.exceptions.UndefinedError` around template rendering and re-raises it
as a `QueryObjectValidationError` with a descriptive message (`"Virtual dataset
template error: %(msg)s"`), following the same pattern already used for other
template-rendering failures in that function.
### TESTING INSTRUCTIONS
- Added a regression test in `tests/unit_tests/jinja_context_test.py` that
creates a virtual dataset whose SQL template indexes into
`filter_values('col')[0]` with no active filters, and asserts that
`get_rendered_sql()` raises `QueryObjectValidationError` (instead of an
unhandled `UndefinedError`).
- Ran `pytest tests/unit_tests/jinja_context_test.py
tests/unit_tests/models/helpers_test.py` locally — all tests pass.
- Ran `ruff check`, `ruff format --check`, `pylint`, and `mypy` on the
changed files.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] 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]
