On Fri, Oct 16, 2026 at 8:23 PM shihao zhong <[email protected]> wrote: > > Hi Haibo, > > Thanks for your v3. I think it breaks some thing that is right today. > > On master with v3: > > SELECT 1 AS one, 1 AS uno GROUP BY ROLLUP(one); > > gives uno = NULL in the grand-total row. Master, 17 and 18 give 1. > > The parser turns the second 1 into the group Var, and with the shared > phid it now gets nulled. Andrey Rachitskiy's patch for bug 19752 stops > that [1]. > > With his patch, v3 is supposed to be ok. So that created some dependency > on submission. > > [1] > https://www.postgresql.org/message-id/CAB8bMiv2JLSxs1aYF42G4887y5LdY=1-e8flj_wgmkfooqw...@mail.gmail.com > Thanks, > Shihao
Hi hackers, After Shihao's report about v3, I investigated the interaction between grouping-expression matching in parse_agg.c and PHV identity in the planner. I agree that v3 is not safe as submitted. Sharing a PHV ID by RTE_GROUP attribute is only correct if the parser has identified genuine references to the same grouping output. Currently, equal() can replace independently written expressions with the same group Var. This seems closely related to Tom's suggestions in the discussion of bug #19752: https://www.postgresql.org/message-id/[email protected] https://www.postgresql.org/message-id/[email protected] I would like to confirm whether the following matching rules are reasonable: A target entry explicitly identified as a grouping item should reference its own RTE_GROUP attribute, selected by ressortgroupref, rather than the first structurally equal grouping item. Other subexpressions should be matched structurally only against grouping items containing a Var of the current query level. Grouping items without such a Var would only be referenced through their explicitly identified target entries. The second rule would leave independent constants, stable expressions, Params, and outer-only correlated expressions untouched, while still allowing expressions involving current-level Vars to be recognized as grouping references. For example, consider: CREATE TABLE t (a int NOT NULL); INSERT INTO t VALUES (1); SELECT DISTINCT a IS NOT NULL AS c1, a IS NOT NULL AS c2 FROM t GROUP BY CUBE(c1, c2) ORDER BY 1 NULLS LAST, 2 NULLS LAST; The two grouping outputs have different ressortgrouprefs, even though their expressions are equal. They should remain distinct. The query returns four rows on current master, but only two with v3. There is also a query-level question: SELECT s.c1, s.c2 FROM t, LATERAL ( SELECT t.a + 1 AS c1, ARRAY[t.a + 1] AS c2 FROM (VALUES (1)) v(x) GROUP BY ROLLUP(c1) ) s ORDER BY s.c1 NULLS LAST; Current master retains {2} in c2 for the grand-total row. I think this is appropriate because c2 is an independently written expression depending only on the outer query. It should not become a reference to the inner grouping output merely because a subtree is equal. I have experimented with the two parser rules above together with v3's PHV-sharing mechanism. The results are encouraging: the original PG19 wrong result, the independent-expression cases, and the CUBE example behave as expected. Core and isolation regression tests pass on master and REL_19_STABLE. Before preparing another patch revision, I'd appreciate opinions on whether these two matching rules reflect the intended semantics, particularly the level-zero-Var criterion. The parser changes also overlap with Andrey's work on bug #19752, so I would like to resolve the intended behavior before deciding how best to separate or coordinate the fixes. Tom, your suggestion in the bug #19752 discussion about restricting structural matching to grouping items containing a level-zero Var seems relevant here. Do you think that rule, together with matching explicitly identified target entries by ressortgroupref, would be the right direction? Thanks, Haibo
