hughhhh opened a new pull request, #45846: URL: https://github.com/apache/superset/pull/45846
### SUMMARY Charts on Hive/Impala datasets were silently dropping rows when a partition filter mapping used the suggested transform `unix_timestamp(:value)`. If the partition key stores hour- or day-start epoch seconds, that transform turns a filter bound like `10:47:23` into the raw instant rather than its bucket boundary, so the mirrored partition predicate excludes rows the original filter keeps. QA reproduced this on Hive 4.0.1 and Impala 4.4.1. The fix: - **Engine default removed.** Hive/Impala (and their subclasses: Spark, Databricks Hive, Ascend) no longer suggest a transform. A BIGINT key doesn't tell Superset its bucket size or timezone. - **Existing mappings protected.** A temporal-to-numeric mapping whose transform is exactly the bare former default stops mirroring. Its filters still apply; only the extra partition predicate is dropped. The editor shows why. Stored expressions are not rewritten. - **Explicit data check.** The editor's new **Validate against data** action takes one to three existing partition keys. It reads up to 50 rows from each, evaluates the transform fresh and compares each output with *that same row's* stored key. Results are matched, mismatch (with examples) or inconclusive. The read goes through `QueryContext` with `raise_for_access()` and RLS, with no `COUNT(*)` and no time filter. Probe caches and mirror verdicts are bypassed. A sampled match is reported as exactly that, not as proof for the whole dataset. - **Honest preview label.** The expression-only preview reads "Expression evaluated — data not checked" instead of "Valid". - **Docs.** The partition-filter-mapping guide and `UPDATING.md` give correct hourly/daily floor recipes and explain timezone and validation semantics. Operator widening and the existing timezone normalization (`adjust_time_bounds`) are unchanged. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF - New Hive/Impala mapping: transform prefilled/suggested as `unix_timestamp(:value)` → no suggestion; placeholder asks for the expression that populates the key. - Saved bare `unix_timestamp(:value)` on an hourly key: predicates mirrored, rows lost → not mirrored, with a warning explaining the missing bucket. - Preview success: "Valid" → "Expression evaluated — data not checked", plus **Validate against data**. ### TESTING INSTRUCTIONS Proof: backend unit tests for `sql/parse`, `connectors/sqla`, `datasets`, partition mirroring and the Hive/Impala specs pass (3,938 total). Jest `PartitionFilterMapping/` passes (119). Staged-file pre-commit passes, including mypy, pylint and the frontend type check. New tests execute generated SQL and compare mirrored and unmirrored row identities for hourly and daily keys across `=`, `IN`, ranges and inequalities. They also confirm that the sampling query keeps RLS and partition restrictions and issues no `COUNT(*)`. The story's chart scenarios have **not** been re-run against a live Hive/Impala warehouse for this PR. With `PARTITION_FILTER_MAPPING` enabled on a Hive or Impala dataset with a TIMESTAMP column mapped to an hour-start BIGINT partition key: 1. Set the transform to `unix_timestamp(:value)`, save, and reopen. Expect a "not mirrored" warning, and no partition predicate in **View query**. 2. Enter existing partition keys and click **Validate against data**. Expect "Partition mismatch found" with the source value, stored key and transform output. 3. Change the transform to `CAST(floor(unix_timestamp(:value) / 3600) * 3600 AS BIGINT)` and validate again. Expect "Matched N sampled rows across M partitions". 4. Compare chart counts for `=`, `IN` and partial-hour ranges with and without the mapping. They should be equal. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [x] Required feature flags: `PARTITION_FILTER_MAPPING` - [x] Changes UI - [ ] Includes DB Migration - [x] Introduces new feature or API: optional `validation_partition_values` on the partition-mapping preview endpoint, `data_validation` in its response, `safety_refusal` in the dataset's partition summary - [x] Removes existing feature or API: Hive/Impala `partition_value_transform_default` (the response field stays, now `null`) Reviewed by one independent Claude reviewer; Sol review was unavailable. Its one real finding (a tokenizer error escaping the dataset summary) is fixed with a regression test. 🤖 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]
