sql: preserve filter semantics in distinct aggregations

[Alignyx]

Fixes #131087.

Queries combining DISTINCT aggregates and FILTER clauses can return incorrect results when the same aggregate argument occurs in rows with different filter outcomes. This change prevents a pre-aggregation DISTINCT stage from discarding rows that a filtered aggregate still needs.

Problem and root cause

For the input below, the three counts should be (4, 4, 1). Before this fix, they are (4, 4, 0):

WITH data AS (
  SELECT * FROM unnest(
    ARRAY[1, 2, 3, 4, 1],
    ARRAY[true, true, true, true, false]
  ) AS t(id, value)
)
SELECT count(DISTINCT id),
       count(DISTINCT id) FILTER (WHERE value IS TRUE),
       count(DISTINCT id) FILTER (WHERE value IS FALSE)
FROM data;

DistSQLPlanner.planAggregators inserts local DISTINCT processors when every aggregation specification has Distinct=true. The DISTINCT key is the union of the aggregates' argument columns (ColIdx); their filter columns (FilterColIdx) are omitted.

For this query, that key is just id. The rows (1, true) and (1, false) are deduplicated before either filtered aggregate evaluates its predicate. If the first row survives, the FALSE-filtered count loses its only contributing value. Reversing the input can instead lose a value from the TRUE-filtered count. Even aggregates sharing the same FILTER can be affected when duplicate arguments have different filter outcomes.

The aggregate processors already implement the required order: apply each aggregate's FILTER, then deduplicate the arguments that passed that filter. The incorrect result originates in the earlier physical-plan optimization.

How the problem was localized

The investigation used a metamorphic repair workflow with the issue's expected full result (4, 4, 1) as an explicit oracle:

1. Reproduce the complete three-column query and retain its incorrect result as the failing observation.
2. Generate equivalent queries that preserve all three output columns. One transformation replaces count(DISTINCT id) FILTER (WHERE p) with count(DISTINCT CASE WHEN p THEN id END). Other references compute the aggregates in separate scalar subqueries. All four generated references returned the expected full result.
3. Compare query plans and instrumented Go coverage for the failing query and the references to guide source inspection. The coverage samples include test-server startup and background work, so they were treated as process-level execution evidence. Inspection of DISTINCT-stage construction and the aggregate's filter-before-distinct processing established the failure mechanism.
4. Independently inspect EXPLAIN (VEC) before and after the fix. The failing plan contains colexec.UnorderedDistinct before rowexec.orderedAggregator; the repaired plan omits the former. An unfiltered DISTINCT control retains its pre-aggregation DISTINCT operator and its correct result.

The initial diagnosis and one-line eligibility change were proposed by DeepSeek. The proposed fix was then independently checked by source inspection and deterministic result comparisons, including additional inputs outside the original repair run.

Fix

The pre-aggregation DISTINCT optimization now requires every aggregate to be DISTINCT and to have no FILTER:

if !e.Distinct || e.FilterColIdx != nil {
    allDistinct = false
    break
}

The final aggregate specifications retain their DISTINCT and FILTER settings, so each aggregate sees all of the rows it needs before performing its own deduplication. Unfiltered all-DISTINCT aggregations remain eligible for the optimization. A comment records why filtered aggregates must be excluded.

This is a conservative eligibility restriction. It can increase the rows processed or transmitted for duplicate-heavy filtered aggregations. Extending the pre-aggregation key to include all filter columns could be investigated separately; this PR does not claim a performance improvement.

Regression coverage and validation

The existing aggregate SQL logic test file gains a standalone distinct_filter subtest with nine queries covering:

• The complete issue query and its equivalent CASE form.
• Reversed input order and NULL arguments/predicates.
• Identical FILTER predicates on COUNT and SUM.
• Different aggregate argument columns.
• A 5,000-row table scan with duplicates across input batches.
• Unfiltered DISTINCT aggregates and empty input.

Validated against upstream commit 8812064a015d2faf99d3fc7e15880f94042954b0:

Native configuration	New subtest before fix	New subtest after fix	Complete aggregate file after fix
local	FAIL: (4,4,0) vs (4,4,1)	PASS	685 checks, 0 failures
local-vec-off	FAIL: (4,4,0) vs (4,4,1)	PASS	685 checks, 0 failures
fakedist (3 nodes)	FAIL: (4,4,0) vs (4,4,1)	PASS	685 checks, 0 failures
fakedist-vec-off (3 nodes)	FAIL: (4,4,0) vs (4,4,1)	PASS	685 checks, 0 failures

The baseline runner stops at the first mismatch; the before-fix result above is the original issue query. After the fix, each focused run executes all nine SQL queries plus setup/cleanup. The complete-file runs have no skipped subtests. The 685 checks include statements and queries, rather than 685 separate Go test functions.

The native targets can be run with:

bazel test \
  //pkg/sql/logictest/tests/local:local_test \
  //pkg/sql/logictest/tests/local-vec-off:local-vec-off_test \
  //pkg/sql/logictest/tests/fakedist:fakedist_test \
  //pkg/sql/logictest/tests/fakedist-vec-off:fakedist-vec-off_test \
  --test_sharding_strategy=disabled \
  --test_filter='^TestLogic_aggregate$/^distinct_filter$' \
  --test_arg=-test.count=1

Repeating with --test_filter='^TestLogic_aggregate$' runs the surrounding aggregate file. Local execution used --norun_validations; crlfmt -fast -tab 2 on the changed Go file, gofmt, and git diff --check passed separately. No generated test registration or BUILD file changes are needed because the existing aggregate test entry reads this fixture.

Prior independent experiments on the initial checkout ran 52 SQL observations across vectorize on/off: the baseline matched 20 expected results, and the candidate matched all 52. The configured colexec suite also passed all 82 tests. These are supplementary results; the native tests above validate the actual PR base and patch.

For clarity, in this checkout FILTER aggregates use the row processor even with vectorize=on. Running both settings checks their respective plans and surrounding operators; it does not establish independent native-vectorized FILTER aggregation coverage. The full CockroachDB repository suite and performance benchmarks have not been run.

Release note (bug fix): Fixed incorrect results from queries combining DISTINCT aggregates and FILTER clauses when rows with the same aggregate arguments had different filter outcomes. Each aggregate now retains the rows required by its own filter before deduplicating its arguments.

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}