Doris-Breakwater commented on issue #69489: URL: https://github.com/apache/doris/issues/69489#issuecomment-6355709612
Breakwater-GitHub-Analysis-Slot: slot_fe2790011d0b The cited FE rewrite does return `0.0` for a non-null singleton input. However, code inspection also finds the same singleton behavior in the backend aggregate, so the evidence does **not yet establish an optimizer-only regression**. The requested `NULL` behavior needs validation across both execution paths before applying the suggested FE-only fix. I checked the configured local repository first, then checked the relevant files at upstream `master` commit `cc49069836975cd3a37bcdc9ba3e35a1fca717d3` (2026-10-23). This is source analysis; I have not executed the SQL on a running Doris cluster. **Verified facts** - `EliminateGroupBy` includes both sample classes in `supportedDevLikeFunctions`, checks `isUniqueAndNotNull(groupBySlots)`, and replaces their aggregate output with `IF(arg IS NULL, NULL, 0.0)` (or constant zero for a non-nullable argument). See [the rule and its eligibility checks](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateGroupBy.java#L65-L158). - The supplied `DUPLICATE KEY(k)` DDL does not establish a unique, non-null key. Distinct values in these two inserted rows do not provide that optimizer guarantee. The scan's key-derived uniqueness path applies to aggregation-family keys; the original example therefore does not demonstrate that this rewrite fired. See [scan trait derivation](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java#L1111-L1125). - Independently of that rule, both sample functions use `SampData`, whose `insert_result_into` calls `insert_default()` when the non-null input count is zero or one. For its `ColumnFloat64`, that default is numeric zero. With a nullable input containing one valid value, the nullable wrapper writes the nested result and marks it non-null; it does not convert a singleton result to `NULL`. See [sample-function creation](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/be/src/exprs/aggregate/aggregate_function_stddev.cpp#L41-L77), [singleton handling](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/be/src/exprs/aggregate/aggregate_function_stddev.h#L145-L161), [numeric column defaults](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/be/src/core/column/column_vector.h#L416-L427), and [nullable result handling](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/be/src/expr s/aggregate/aggregate_function_null.h#L306-L318). The [V2 nullable wrapper](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/be/src/exprs/aggregate/aggregate_function_null_v2.h#L392-L404) behaves the same here. - Existing regression expectations explicitly record zero for singleton `stddev_samp` and `variance_samp`, and `NULL` for null input. That documents current behavior, not proof that the desired semantics are correct: [existing baseline](https://github.com/apache/doris/blob/cc49069836975cd3a37bcdc9ba3e35a1fca717d3/regression-test/data/nereids_rules_p0/eliminate_gby_key/eliminate_group_by.out#L72-L91). **Missing evidence and a focused reproduction** Please provide the exact tested commit/build (`SELECT VERSION()` plus build hash), actual result output, and `EXPLAIN VERBOSE` output for the original query. To isolate the rewrite, also use a table with a provable non-null unique key: ```sql CREATE TABLE bw_69489_unique (k INT NOT NULL, x INT) UNIQUE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1 PROPERTIES ("replication_num" = "1"); INSERT INTO bw_69489_unique VALUES (1, 5), (2, 7), (3, NULL); SELECT k, STDDEV_SAMP(x), VAR_SAMP(x), STDDEV_POP(x), VAR_POP(x) FROM bw_69489_unique GROUP BY k ORDER BY k; ``` In a fresh session, capture both results and `EXPLAIN VERBOSE` for that SELECT with `disable_nereids_rules=''`, then with `disable_nereids_rules='ELIMINATE_GROUP_BY'`. Confirm whether the sample aggregate disappears from the plan. Retain the original duplicate-key table as a control. Source inspection predicts zero for valid singleton sample inputs on both paths at the checked revision; runtime confirmation is still needed. Disabling this rule is a diagnostic comparison, **not a verified workaround** for obtaining `NULL`. A profile or large log bundle is not needed for this first comparison. **Recommended next steps** 1. Route to the Nereids and aggregate-function maintainers to confirm the singleton sample-function contract. The `n - 1` argument supports the requested semantics, but the observed zero may come from the backend even when no elimination occurs. 2. If `NULL` for fewer than two non-null sample values is the accepted contract, address the backend result/nullability behavior and the FE rewrite together. Rewriting eligible sample aggregates to a typed `DOUBLE NULL` can implement the FE case; excluding them from `canRewrite` alone will still reach the backend's zero special case. An FE-only constant-NULL change would introduce a difference between optimized and retained aggregates unless the backend is aligned. 3. Add result and plan coverage with the rule enabled/disabled: nullable and non-nullable arguments, null-only and singleton groups, multiple-row groups with only one valid value, `DISTINCT`, empty input, and aliases. Keep population singleton results at zero for valid inputs and `NULL` for null-only inputs. Recheck output nullability even when the sample argument is declared `NOT NULL`, and regenerate the affected regression expectations after agreeing on the contract. No source changes were made. Affected release branches and a runtime discrepancy between the two plans remain unverified. -- 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]
