[zahariash]

This work was produced with the help of language models.

Fixes #1293.

Over P(id INT64 PRIMARY KEY, flag BOOLEAN) with ids 1, 2, 3, MATCH (p:P) WITH DISTINCT p.id AS id WITH DISTINCT coalesce(id, 0) AS v RETURN v returned [1] instead of [1], [2], [3]. CASE WHEN f THEN id ELSE 0 END and [id] collapsed the same way, also under RETURN DISTINCT, and a UNION ALL branch returning [id] gave [1] on every row. With runtime checks on, as in CI, these queries failed with Assertion failed in file ".../src/include/planner/operator/schema.h" on line 26: !flat instead, and so did id + 1, a join on id and an UNWIND over it.

Fix. RemoveUnnecessaryDistinctOptimizer ran after FactorizationRewriter, so a removed DISTINCT left its FLATTEN behind, and the FLATTEN placed for the operator above flattened the same chunk again. The pass and AggKeyDependencyOptimizer, whose key resolution it needs, now run just before FactorizationRewriter, which then places flattens only for the operators that remain. The removal pass now recomputes flat schemas and no longer walks through FLATTEN, which can't occur there. Resolving keys first doesn't change the set of flattened groups, since a DISTINCT or aggregate flattens its keys and payloads together. Across the e2e suite, 76,333 of 76,350 optimized plans print identically to main's under LBUG_DUMP_LOGICAL; the rest are the new test, two other removed DISTINCTs that lose their leftover FLATTEN, and run-to-run noise.

Speed. Without the leftover FLATTEN these plans stay vectorized. Per execution over 2M nodes, single-threaded:

Query	Instructions, main → here	Mallocs, main → here
MATCH (p:P) RETURN DISTINCT p.id	2,868 M → 228 M	6,001,437 → 4,301
MATCH (p:P) WITH DISTINCT p.id AS id RETURN count(*)	766 M → 52 M	2,162 → 2,088
MATCH (p:P) WITH DISTINCT p.id AS id RETURN sum(id)	1,112 M → 356 M	2,334 → 2,262

RETURN DISTINCT p.name and RETURN p.name, count(*) over the same nodes stay within 0.01% of main.

ORDER BY removal. RemoveUnnecessaryOrderByOptimizer also runs after FactorizationRewriter and stays there. It has its own wrong-result bug (#1352), and before factorization it would find more ORDER BYs to remove.

Tests. optimizer/remove_unnecessary_distinct.test runs eight queries over four nodes, one with a null flag: the issue's cases, RETURN DISTINCT, v + 1, a join, an UNWIND and a UNION ALL. On the 0.21.2 and nightly wheels the CASE, both coalesce statements and the list return one row, and the UNION ALL returns [1] five times. With runtime checks, main fails all eight on the assertion. Full ctest with runtime checks: 3164/3164.

[adsharma]

Low (non-blocking, fail-closed today): computeFlatSchema() unconditionally throws on FLATTEN (logical_flatten.cpp:13-14). The invariant holds today (RemoveFactorizationRewriter strips FLATTENs first and this pass runs before FactorizationRewriter), but it is implicit. Consider a one-line DASSERT/comment at the pass entry, or defensively bypassing FLATTEN nodes, so a future pass-order change fails loudly at the right place.

[zahariash]

Done in 6efd2e0. With runtime checks, the pass now asserts at entry that the plan has no FLATTEN, next to a one-line comment saying why. RemoveUnnecessaryDistinctRequiresFlatPlan runs the pass on an optimized plan and expects the error from that assertion; without it, the test fails on LogicalFlatten::computeFlatSchema() should never be used. Release builds skip the walk and still throw in computeFlatSchema. I kept FLATTEN failing rather than bypassing it, since skipping it would hide a reorder.

[adsharma]

Two notes from review (inline hardening suggestion posted separately):

1. Plan-equivalence (2,247 tinysnb queries) is asserted in the body but not evidenced in-PR. I verified the reasoning independently (getGroupsPosToFlatten covers keys∪payloads, so the reorder preserves the flattened-group set; aggregate FlattenAllButOne nondeterminism pre-exists) — treat as residual risk pending full CI on the merge commit.
2. Pre-existing observation, no action: isChildMultiplicityOne accepts INDEX_LOOK_UP as a multiplicity-1 leaf without checking the PK belongs to the scanned table. Unchanged by this PR and conservatively safe in tested paths — noting only so it isn't mistaken for a regression.

[zahariash]

1. Plan equivalence. I ran the whole e2e suite with LBUG_DUMP_LOGICAL=1 on main (bdd271c) and on this branch, and matched the optimized plans by test and position. 76,333 of 76,350 print identically. The dump doesn't show which group a FLATTEN flattens, so that part still rests on the reasoning you checked. Of the 17 that differ, 7 are in the new test, which fails on main. Two are other removed DISTINCTs, RETURN DISTINCT a.* ORDER BY a.name DESC in order_by_after_aggregate.test and TCK return_orderby2 Scenario9, which lose the leftover FLATTEN under the ORDER BY. The other 8 differ between any two runs: 6 temp paths in ATTACH and 2 statements of dml_tinysnb_serial_execution that ran in swapped order. 3531dd2 pins the plan shape this PR changes: RETURN DISTINCT n.ID and a count(*) over WITH DISTINCT n.ID plan with no DISTINCT and no FLATTEN, which fails on main.

2. INDEX_LOOK_UP. Agreed, unchanged here. Only COPY creates it (plan_copy.cpp), so no query plan reaches that case. A query's primary-key lookup is QUERY_PRIMARY_KEY_LOOKUP, which falls to default and keeps the DISTINCT.