From dda2d2f86658356b0d883f0b3d15d20f8287f06c Mon Sep 17 00:00:00 2001 From: Hannu Krosing Date: Wed, 16 Sep 2026 13:32:07 +0000 Subject: [PATCH v3 3/5] Support UNION DISTINCT ON for non-recursive set operations Extends set operations to support duplicate elimination on a subset of columns using the syntax: select_stmt UNION DISTINCT ON (keys [ORDER BY sort_keys]) select_stmt Builds upon the setop group clause refactoring to plan both HashAgg and Sort->Unique paths for set operations with partial group lists. Also prevents invalid subquery qual pushdown onto non-distinct attributes via the UNSAFE_NOTIN_DISTINCTON_CLAUSE safety check. --- doc/src/sgml/ref/select.sgml | 8 +- src/backend/optimizer/path/allpaths.c | 19 ++++ src/backend/optimizer/plan/planner.c | 6 +- src/backend/optimizer/prep/prepunion.c | 57 +++++------ src/backend/parser/analyze.c | 126 +++++++++++++++++++++---- src/backend/parser/gram.y | 26 ++++- src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/union.out | 48 ++++++++++ src/test/regress/sql/union.sql | 28 ++++++ 9 files changed, 268 insertions(+), 51 deletions(-) diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 1ed53b2b88e..ca09f75c027 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -1251,7 +1251,7 @@ SELECT DISTINCT ON (location ORDER BY time DESC) location, time, report The UNION clause has this general form: -select_statement UNION [ ALL | DISTINCT ] select_statement +select_statement UNION [ ALL | DISTINCT | DISTINCT ON ( expression [, ...] [ ORDER BY sort_expression [ ASC | DESC ] [, ...] ] ) ] select_statement select_statement is any SELECT statement without an ORDER BY, LIMIT, FOR NO KEY UPDATE, FOR UPDATE, @@ -1282,6 +1282,12 @@ SELECT DISTINCT ON (location ORDER BY time DESC) location, time, report UNION; use ALL when you can.) DISTINCT can be written to explicitly specify the default behavior of eliminating duplicate rows. + DISTINCT ON keeps only the first row of each set of rows + that are duplicates according to the specified expressions. + If the optional ORDER BY clause is specified inside + DISTINCT ON, it determines which row is kept from each set of + duplicates (the first row according to this sort order). Otherwise, the kept + row is unpredictable. diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 295e3509c74..f01a1e64e69 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -4175,6 +4175,25 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery, */ if (subquery->setOperations == NULL) check_output_expressions(subquery, safetyInfo); + else if (subquery->hasDistinctOn) + { + ListCell *lc; + + foreach(lc, subquery->targetList) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc); + + if (tle->resjunk) + continue; + + if ((safetyInfo->unsafeFlags[tle->resno] & + UNSAFE_NOTIN_DISTINCTON_CLAUSE) == 0 && + !targetIsInSortList(tle, InvalidOid, subquery->distinctClause)) + { + safetyInfo->unsafeFlags[tle->resno] |= UNSAFE_NOTIN_DISTINCTON_CLAUSE; + } + } + } /* Are we at top level, or looking at a setop component? */ if (subquery == topquery) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 5df5641e05a..944d0789d59 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1824,7 +1824,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction, /* * Calculate pathkeys that represent result ordering requirements */ - Assert(parse->distinctClause == NIL); root->sort_pathkeys = make_pathkeys_for_sortclauses(root, parse->sortClause, root->processed_tlist); @@ -3938,7 +3937,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) false, false, &sortable, - false); + true); if (!sortable) root->setop_pathkeys = NIL; } @@ -8699,7 +8698,8 @@ get_nth_nonjunk_tle(List *tlist, int n) static List * generate_setop_child_grouplist(SetOperationStmt *op, List *targetlist) { - List *grouplist = copyObject(op->groupClauses); + List *clauses = op->sortClauses ? op->sortClauses : op->groupClauses; + List *grouplist = copyObject(clauses); ListCell *lg; foreach(lg, grouplist) diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index efbaf134627..aa65bb6c6fa 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -113,7 +113,6 @@ plan_set_operations(PlannerInfo *root) Assert(parse->groupClause == NIL); Assert(parse->havingQual == NULL); Assert(parse->windowClause == NIL); - Assert(parse->distinctClause == NIL); /* * In the outer query level, equivalence classes are limited to classes @@ -948,32 +947,34 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, * Try a hash aggregate plan on 'apath'. This is the cheapest * available path containing each append child. */ - path = (Path *) create_agg_path(root, - result_rel, - apath, - result_rel->reltarget, - AGG_HASHED, - AGGSPLIT_SIMPLE, - groupList, - NIL, - NULL, - dNumChildGroups); - add_path(result_rel, path); - - /* Try hash aggregate on the Gather path, if valid */ - if (gpath != NULL) - { - /* Hashed aggregate plan --- no sort needed */ - path = (Path *) create_agg_path(root, + path = (Path *) create_agg_path_ext(root, result_rel, - gpath, + apath, result_rel->reltarget, AGG_HASHED, AGGSPLIT_SIMPLE, groupList, NIL, NULL, - dNumChildGroups); + dNumChildGroups, + op->sortClauses); + add_path(result_rel, path); + + /* Try hash aggregate on the Gather path, if valid */ + if (gpath != NULL) + { + /* Hashed aggregate plan --- no sort needed */ + path = (Path *) create_agg_path_ext(root, + result_rel, + gpath, + result_rel->reltarget, + AGG_HASHED, + AGGSPLIT_SIMPLE, + groupList, + NIL, + NULL, + dNumChildGroups, + op->sortClauses); add_path(result_rel, path); } } @@ -981,17 +982,18 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, if (can_sort) { Path *path = apath; + List *sort_clauses = op->sortClauses ? op->sortClauses : groupList; /* Try Sort -> Unique on the Append path */ - if (groupList != NIL) + if (sort_clauses != NIL) path = (Path *) create_sort_path(root, result_rel, path, - make_pathkeys_for_sortclauses(root, groupList, tlist), + make_pathkeys_for_sortclauses(root, sort_clauses, tlist), -1.0); path = (Path *) create_unique_path(root, result_rel, path, - list_length(path->pathkeys), + list_length(groupList), dNumChildGroups); add_path(result_rel, path); @@ -1001,14 +1003,15 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root, { path = gpath; - path = (Path *) create_sort_path(root, result_rel, path, - make_pathkeys_for_sortclauses(root, groupList, tlist), - -1.0); + if (sort_clauses != NIL) + path = (Path *) create_sort_path(root, result_rel, path, + make_pathkeys_for_sortclauses(root, sort_clauses, tlist), + -1.0); path = (Path *) create_unique_path(root, result_rel, path, - list_length(path->pathkeys), + list_length(groupList), dNumChildGroups); add_path(result_rel, path); } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 232133f7470..84f64d6215f 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -78,10 +78,13 @@ static Query *transformSelectStmt(ParseState *pstate, SelectStmt *stmt, static Query *transformValuesClause(ParseState *pstate, SelectStmt *stmt); static Query *transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt); static Node *transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, - bool isTopLevel, List **targetlist); + bool isTopLevel, List **targetlist, + List *distinctClause); static void constructSetOpTargetlist(ParseState *pstate, SetOperationStmt *op, const List *ltargetlist, const List *rtargetlist, - List **targetlist, const char *context, bool recursive); + List **targetlist, const char *context, bool recursive, + List *distinctClause); +static bool col_in_distinct_on(const char *colname, int resno, List *distinctClause); static void determineRecursiveColTypes(ParseState *pstate, Node *larg, List *nrtargetlist); static Query *transformReturnStmt(ParseState *pstate, ReturnStmt *stmt); @@ -1855,6 +1858,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) ParseNamespaceColumn *sortnscolumns; int sortcolindex; int tllen; + List *distinctClause = stmt->distinctClause; + List *distinctSortClause = stmt->distinctSortClause; + List *transformed_distinctSortClause = NIL; + int distinct_tllen; qry->commandType = CMD_SELECT; @@ -1890,6 +1897,8 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) withClause = stmt->withClause; stmt->sortClause = NIL; + stmt->distinctClause = NIL; + stmt->distinctSortClause = NIL; stmt->limitOffset = NULL; stmt->limitCount = NULL; stmt->lockingClause = NIL; @@ -1917,7 +1926,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) * Recursively transform the components of the tree. */ sostmt = castNode(SetOperationStmt, - transformSetOperationTree(pstate, stmt, true, NULL)); + transformSetOperationTree(pstate, stmt, true, NULL, distinctClause)); Assert(sostmt); qry->setOperations = (Node *) sostmt; @@ -2033,11 +2042,22 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) EXPR_KIND_ORDER_BY, false /* allow SQL92 rules */ ); + distinct_tllen = list_length(qry->targetList); + if (distinctSortClause) + { + List *full_sortby = prepend_distinct_to_sortby(distinctClause, distinctSortClause); + transformed_distinctSortClause = transformSortClause(pstate, + full_sortby, + &qry->targetList, + EXPR_KIND_ORDER_BY, + false); + } + /* restore namespace, remove join RTE from rtable */ pstate->p_namespace = sv_namespace; pstate->p_rtable = list_truncate(pstate->p_rtable, sv_rtable_length); - if (tllen != list_length(qry->targetList)) + if (tllen != distinct_tllen) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("invalid UNION/INTERSECT/EXCEPT ORDER BY clause"), @@ -2046,6 +2066,26 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) parser_errposition(pstate, exprLocation(list_nth(qry->targetList, tllen))))); + if (distinct_tllen != list_length(qry->targetList)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("invalid UNION/INTERSECT/EXCEPT DISTINCT ON ORDER BY clause"), + errdetail("Only result column names can be used, not expressions or functions."), + parser_errposition(pstate, + exprLocation(list_nth(qry->targetList, distinct_tllen))))); + + qry->distinctSortClause = transformed_distinctSortClause; + sostmt->sortClauses = transformed_distinctSortClause; + + if (distinctClause) + { + qry->distinctClause = transformDistinctOnClause(pstate, + distinctClause, + &qry->targetList, + transformed_distinctSortClause ? transformed_distinctSortClause : qry->sortClause); + qry->hasDistinctOn = true; + } + qry->limitOffset = transformLimitClause(pstate, limitOffset, EXPR_KIND_OFFSET, "OFFSET", stmt->limitOption); @@ -2134,7 +2174,8 @@ makeSortGroupClauseForSetOp(Oid rescoltype, bool require_hash) */ static Node * transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, - bool isTopLevel, List **targetlist) + bool isTopLevel, List **targetlist, + List *distinctClause) { bool isLeaf; @@ -2279,7 +2320,8 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, */ op->larg = transformSetOperationTree(pstate, stmt->larg, false, - <argetlist); + <argetlist, + NIL); /* * If we are processing a recursive union query, now is the time to @@ -2295,10 +2337,11 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, */ op->rarg = transformSetOperationTree(pstate, stmt->rarg, false, - &rtargetlist); + &rtargetlist, + NIL); constructSetOpTargetlist(pstate, op, ltargetlist, rtargetlist, targetlist, - context, recursive); + context, recursive, distinctClause); return (Node *) op; } @@ -2319,7 +2362,8 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, static void constructSetOpTargetlist(ParseState *pstate, SetOperationStmt *op, const List *ltargetlist, const List *rtargetlist, - List **targetlist, const char *context, bool recursive) + List **targetlist, const char *context, bool recursive, + List *distinctClause) { ListCell *ltl; ListCell *rtl; @@ -2442,18 +2486,27 @@ constructSetOpTargetlist(ParseState *pstate, SetOperationStmt *op, */ if (op->op != SETOP_UNION || !op->all) { - ParseCallbackState pcbstate; - SortGroupClause *grpcl; + bool active = false; + if (distinctClause == NIL) + active = true; + else + active = col_in_distinct_on(ltle->resname, resno, distinctClause); - setup_parser_errposition_callback(&pcbstate, pstate, - bestlocation); + if (active) + { + ParseCallbackState pcbstate; + SortGroupClause *grpcl; + + setup_parser_errposition_callback(&pcbstate, pstate, + bestlocation); - /* If it's a recursive union, we need to require hashing support. */ - grpcl = makeSortGroupClauseForSetOp(rescoltype, recursive); - grpcl->tleSortGroupRef = resno; - op->groupClauses = lappend(op->groupClauses, grpcl); + /* If it's a recursive union, we need to require hashing support. */ + grpcl = makeSortGroupClauseForSetOp(rescoltype, recursive); + grpcl->tleSortGroupRef = resno; + op->groupClauses = lappend(op->groupClauses, grpcl); - cancel_parser_errposition_callback(&pcbstate); + cancel_parser_errposition_callback(&pcbstate); + } } /* @@ -3796,3 +3849,40 @@ test_raw_expression_coverage(Node *node, void *context) context); } #endif /* DEBUG_NODE_TESTS_ENABLED */ + +/* + * col_in_distinct_on - + * Check if a column name or its 1-based position matches any expression in distinctClause + */ +static bool +col_in_distinct_on(const char *colname, int resno, List *distinctClause) +{ + ListCell *lc; + + foreach(lc, distinctClause) + { + Node *n = (Node *) lfirst(lc); + + if (IsA(n, ColumnRef)) + { + ColumnRef *cr = (ColumnRef *) n; + + if (list_length(cr->fields) == 1 && IsA(linitial(cr->fields), String)) + { + if (strcmp(strVal(linitial(cr->fields)), colname) == 0) + return true; + } + } + else if (IsA(n, A_Const)) + { + A_Const *aconst = (A_Const *) n; + + if (IsA(&aconst->val, Integer)) + { + if (intVal(&aconst->val) == resno) + return true; + } + } + } + return false; +} diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index a8bb0a4d8d5..6247ba94fd4 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -13263,9 +13263,31 @@ simple_select: n->fromClause = list_make1($2); $$ = (Node *) n; } - | select_clause UNION set_quantifier select_clause + | select_clause UNION select_clause %prec UNION { - $$ = makeSetOp(SETOP_UNION, $3 == SET_QUANTIFIER_ALL, $1, $4); + $$ = makeSetOp(SETOP_UNION, false, $1, $3); + } + | select_clause UNION ALL select_clause %prec UNION + { + $$ = makeSetOp(SETOP_UNION, true, $1, $4); + } + | select_clause UNION distinct_clause select_clause %prec UNION + { + List *distinctClause = linitial($3); + List *distinctSortClause = lsecond($3); + Node *n = makeSetOp(SETOP_UNION, false, $1, $4); + SelectStmt *s = (SelectStmt *) n; + if (linitial(distinctClause) == NULL && distinctSortClause == NIL) + { + s->distinctClause = NIL; + s->distinctSortClause = NIL; + } + else + { + s->distinctClause = distinctClause; + s->distinctSortClause = distinctSortClause; + } + $$ = (Node *) s; } | select_clause INTERSECT set_quantifier select_clause { diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 6beecfcdfb3..62a5103e765 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2313,6 +2313,7 @@ typedef struct SetOperationStmt /* a list of SortGroupClause's */ List *groupClauses pg_node_attr(query_jumble_ignore); /* groupClauses is NIL if UNION ALL, but must be set otherwise */ + List *sortClauses pg_node_attr(query_jumble_ignore); /* inline DISTINCT ON ORDER BY clauses */ } SetOperationStmt; diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out index 2495351d907..91fd248fcad 100644 --- a/src/test/regress/expected/union.out +++ b/src/test/regress/expected/union.out @@ -1804,3 +1804,51 @@ join (select ten from tenk1 union select ten from onek) s on s.ten = t.unique1; Index Cond: (unique1 = tenk1.ten) (8 rows) +-- +-- UNION DISTINCT ON +-- +CREATE TABLE union_distinct_u1 (a int, b int); +CREATE TABLE union_distinct_u2 (a int, b int); +INSERT INTO union_distinct_u1 VALUES (1, 10), (2, 20); +INSERT INTO union_distinct_u2 VALUES (1, 100), (3, 30); +-- UNION DISTINCT ON without ORDER BY +SELECT a, b FROM union_distinct_u1 UNION DISTINCT ON (a) SELECT a, b FROM union_distinct_u2 ORDER BY a; + a | b +---+---- + 1 | 10 + 2 | 20 + 3 | 30 +(3 rows) + +-- UNION DISTINCT ON with ORDER BY (DESC) +SELECT a, b FROM union_distinct_u1 UNION DISTINCT ON (a ORDER BY b DESC) SELECT a, b FROM union_distinct_u2 ORDER BY a; + a | b +---+----- + 1 | 100 + 2 | 20 + 3 | 30 +(3 rows) + +-- UNION DISTINCT ON with ORDER BY (ASC) +SELECT a, b FROM union_distinct_u1 UNION DISTINCT ON (a ORDER BY b ASC) SELECT a, b FROM union_distinct_u2 ORDER BY a; + a | b +---+---- + 1 | 10 + 2 | 20 + 3 | 30 +(3 rows) + +-- Test subquery pushdown safety with UNION DISTINCT ON +SELECT * FROM ( + SELECT a, b FROM union_distinct_u1 + UNION DISTINCT ON (a ORDER BY b DESC) + SELECT a, b FROM union_distinct_u2 +) s WHERE b > 15 ORDER BY a; + a | b +---+----- + 1 | 100 + 2 | 20 + 3 | 30 +(3 rows) + +DROP TABLE union_distinct_u1, union_distinct_u2; diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql index 8fce4528a3f..e6813edc244 100644 --- a/src/test/regress/sql/union.sql +++ b/src/test/regress/sql/union.sql @@ -710,3 +710,31 @@ select null::int[] union all select null::int[] union all select null::bigint[]; explain (costs off) select * from tenk1 t join (select ten from tenk1 union select ten from onek) s on s.ten = t.unique1; + +-- +-- UNION DISTINCT ON +-- + +CREATE TABLE union_distinct_u1 (a int, b int); +CREATE TABLE union_distinct_u2 (a int, b int); +INSERT INTO union_distinct_u1 VALUES (1, 10), (2, 20); +INSERT INTO union_distinct_u2 VALUES (1, 100), (3, 30); + +-- UNION DISTINCT ON without ORDER BY +SELECT a, b FROM union_distinct_u1 UNION DISTINCT ON (a) SELECT a, b FROM union_distinct_u2 ORDER BY a; + +-- UNION DISTINCT ON with ORDER BY (DESC) +SELECT a, b FROM union_distinct_u1 UNION DISTINCT ON (a ORDER BY b DESC) SELECT a, b FROM union_distinct_u2 ORDER BY a; + +-- UNION DISTINCT ON with ORDER BY (ASC) +SELECT a, b FROM union_distinct_u1 UNION DISTINCT ON (a ORDER BY b ASC) SELECT a, b FROM union_distinct_u2 ORDER BY a; + +-- Test subquery pushdown safety with UNION DISTINCT ON +SELECT * FROM ( + SELECT a, b FROM union_distinct_u1 + UNION DISTINCT ON (a ORDER BY b DESC) + SELECT a, b FROM union_distinct_u2 +) s WHERE b > 15 ORDER BY a; + +DROP TABLE union_distinct_u1, union_distinct_u2; + -- 2.56.0.176.g3575e65682-goog