mikebridge opened a new pull request, #42052:
URL: https://github.com/apache/superset/pull/42052

   ### SUMMARY
   
   Fixes query construction for charts whose x-axis is an adhoc SQL expression 
and whose time range comes from dashboard filtering.
   
   Without an explicit granularity, the physical datetime column was not 
retained as the time-filter subject. On ClickHouse this could produce an 
effectively unbounded read and trigger `TOO_MANY_ROWS`, even though the 
dashboard supplied a narrow time range. The fix infers the dataset main 
datetime column only for bounded adhoc expression axes and keeps that inferred 
value out of the legacy axis-rewrite and temporal-filter-removal path.
   
   Regression coverage verifies that the expression remains the grouping 
dimension, the physical datetime predicate is generated, physical temporal axes 
are not rewritten, and independent temporal filters are preserved.
   
   Shortcut: 
https://app.shortcut.com/preset/story/107759/query-still-causes-too-many-rows-on-clickhouse-when-x-axis-uses-sql-expressions
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable; this is a backend query-construction fix. The behavior is 
visible as a ClickHouse `TOO_MANY_ROWS` error before the change and a rendered 
chart after it.
   
   ### TESTING INSTRUCTIONS
   
   Automated:
   
   ```bash
   pytest -q tests/unit_tests/common/test_query_context_factory.py 
tests/unit_tests/models/test_no_filter_time_range.py
   ```
   
   Expected: 49 tests pass.
   
   Manual ClickHouse reproduction:
   
   1. Configure a ClickHouse dataset with a physical main datetime column and 
an adhoc SQL expression as the chart x-axis.
   2. Set a restrictive `max_rows_to_read` value on the ClickHouse connection.
   3. Apply a narrow dashboard native time-range filter.
   4. On `master`, refresh the chart and observe `TOO_MANY_ROWS` because the 
physical time predicate is absent.
   5. On this branch, refresh the same chart and verify it renders and the 
generated SQL filters the main datetime column while grouping by the SQL 
expression.
   
   ### 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]

Reply via email to