mikebridge opened a new pull request, #42464:
URL: https://github.com/apache/superset/pull/42464
### SUMMARY
ClickHouse enforces `max_rows_to_read` against its **pre-execution row
estimate** derived from index/part analysis, which ignores `LIMIT`. Any
full-table-shaped query on a table above the configured cap (observed: a
~4.3B-row table vs a 4B cap on ClickHouse Cloud) is rejected up front with
`Code: 158 (TOO_MANY_ROWS)` — no matter how small the `LIMIT` is.
Superset authors several **system-generated** sampling/metadata queries with
exactly this shape, all of which fail on large ClickHouse tables:
- **Filter-values dropdown** (`values_for_column`): `SELECT DISTINCT col
FROM table LIMIT n` — user-facing error when it fails.
- **Samples tab / dataset preview and SQL Lab table preview**
(`select_star`, samples query action): `SELECT * FROM table LIMIT n` —
user-facing error.
- **Datetime format detection** on dataset save: `SELECT col FROM table
LIMIT n` — failure is caught, but detection silently never works and the logs
report success anyway.
This PR adds an engine-spec hook, `apply_sampling_read_limit_override`, that
lets system-authored sampling SQL survive the read limit. On ClickHouse it
appends `SETTINGS read_overflow_mode='break'`, so the operator's own
`max_rows_to_read` cap becomes the read *bound*: the query stops reading at the
cap and returns the partial result instead of erroring. The base implementation
is a no-op for all other engines.
Design decisions:
- **Only Superset-authored SQL over physical tables gets the override.**
Virtual datasets and any user-authored SQL stay fully governed by the
operator's read limits. The samples query action marks its query object with a
`system_sampling` extras flag; `adhoc_column_to_sqla`'s zero-row type probe is
explicitly excluded (covered separately by #42298).
- **Per-database opt-out**: operators can set
`disable_sampling_read_limit_override: true` in the database `extra` to keep
every query subject to configured limits. All call sites go through
`Database.apply_sampling_read_limit_override` so the opt-out is honored in one
place.
- **The override is applied before `mutate_sql_based_on_config`**, so
operator SQL mutators wrap the final query text.
- The override is idempotent (a query already carrying the suffix is
returned unchanged).
Also makes the datetime format detector truthful about failures:
- A read-limit rejection now logs at WARNING instead of ERROR (it is an
expected, handled condition — detection is skipped for that column).
- The summary line reported `Detected formats for %d columns` using the
count of *attempted* columns, so a 0-for-4 failure logged as apparent full
success. It now reports `Detected formats for %d of %d temporal columns` and
logs at WARNING when nothing was detected.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (backend-only; the user-visible effect is that filter dropdowns, the
Samples tab, and table previews return bounded results on very large ClickHouse
tables instead of erroring with `TOO_MANY_ROWS`).
### TESTING INSTRUCTIONS
Unit tests cover the engine-spec hook (base no-op, ClickHouse family,
idempotency, per-database opt-out), the sampling call sites (filter values,
samples action, `select_star`, format detector), the physical-vs-virtual
distinction, the type-probe exclusion, and the detector logging changes:
```
pytest tests/unit_tests/db_engine_specs/test_clickhouse.py \
tests/unit_tests/models/helpers_test.py \
tests/unit_tests/datasets/test_datetime_format_detector.py \
tests/unit_tests/common/test_query_actions.py
```
Manual verification against ClickHouse:
1. Connect a ClickHouse database and pick a table whose row count exceeds
`max_rows_to_read` (or set `max_rows_to_read` below the table's row count to
simulate).
2. Create a physical dataset on it: dataset save completes and datetime
format detection logs detected-of-attempted counts instead of `TOO_MANY_ROWS`
errors.
3. Open a filter on a column of that dataset: the values dropdown populates
(bounded read) instead of erroring.
4. Open the dataset's Samples tab / SQL Lab table preview: rows are returned.
5. Set `"disable_sampling_read_limit_override": true` in the database's
`extra` and confirm the queries are again subject to the configured limit.
### 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
