Skip to content

Commit 6038f1b

Browse files
craig[bot]ajstorm
andcommitted
Merge #158162
158162: optbuilder: preserve ORDER BY in set-returning UDFs with OUT parameters r=drewkimball a=ajstorm Fixes #144013 When set-returning UDFs with OUT parameters are called directly in a SELECT list (e.g., `SELECT f()`), CockroachDB wraps multiple result columns into a single tuple column. During this transformation, two functions created new scopes without preserving ordering information, causing ORDER BY clauses in the UDF body to be ignored. ## Solution This PR fixes the issue by calling `copyOrdering()` in two places in `pkg/sql/opt/optbuilder/routine.go`: 1. **`combineRoutineColsIntoTuple`** - preserves ordering when wrapping columns into a tuple 2. **`maybeAddRoutineAssignmentCasts`** - preserves ordering when adding type casts The `copyOrdering()` method not only copies the ordering metadata but also adds the columns referenced by the ordering to `extraCols`, ensuring they remain available for the optimizer to enforce the ordering. ## Testing Added a new regression test `out_params_ordering` in `pkg/sql/logictest/testdata/logic_test/udf_setof` that verifies ORDER BY is preserved for both ascending and descending order with OUT parameters. ## Release note Release note (bug fix): Fixed a bug where ORDER BY clauses in set-returning SQL user-defined functions with OUT parameters were ignored when the function was called directly in a SELECT list (e.g., SELECT f()). The ordering is now properly preserved and enforced. Co-authored-by: Adam Storm <storm@cockroachlabs.com>
2 parents 26a5f93 + 1a95ec7 commit 6038f1b

File tree

4 files changed

+135
-7
lines changed

4 files changed

+135
-7
lines changed

pkg/ccl/logictestccl/testdata/logic_test/plpgsql_srf

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,7 @@ CREATE FUNCTION f() RETURNS TABLE (x INT, y INT) LANGUAGE PLpgSQL AS $$
413413
END
414414
$$;
415415

416-
# TODO(144013): we should be able to use nosort here, but the ordering is
417-
# dropped.
418-
query T rowsort
416+
query T nosort
419417
SELECT f();
420418
----
421419
(1,2)
@@ -432,6 +430,22 @@ SELECT * FROM f();
432430
statement ok
433431
DROP FUNCTION f;
434432

433+
# Test that ordering is preserved with array_agg
434+
statement ok
435+
CREATE FUNCTION f_agg() RETURNS TABLE (x INT, y INT) LANGUAGE PLpgSQL AS $$
436+
BEGIN
437+
RETURN QUERY SELECT * FROM xy ORDER BY xy.x DESC;
438+
END
439+
$$;
440+
441+
query T
442+
SELECT array_agg(f) FROM f_agg() AS f;
443+
----
444+
{"(5,6)","(3,4)","(1,2)"}
445+
446+
statement ok
447+
DROP FUNCTION f_agg;
448+
435449
subtest end
436450

437451
subtest record

pkg/sql/logictest/testdata/logic_test/udf_setof

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,3 +325,114 @@ statement ok
325325
DROP FUNCTION f145414;
326326

327327
subtest end
328+
329+
# Test that ORDER BY is preserved in SQL UDFs with OUT parameters
330+
subtest out_params_ordering
331+
332+
statement ok
333+
CREATE TABLE test_ordering (x INT PRIMARY KEY, y INT);
334+
335+
statement ok
336+
INSERT INTO test_ordering VALUES (1, 10), (2, 20), (3, 30), (4, 40);
337+
338+
statement ok
339+
CREATE FUNCTION f_out_desc(OUT a INT, OUT b INT) RETURNS SETOF RECORD AS $$
340+
SELECT x, y FROM test_ordering ORDER BY x DESC;
341+
$$ LANGUAGE SQL;
342+
343+
query T nosort
344+
SELECT f_out_desc();
345+
----
346+
(4,40)
347+
(3,30)
348+
(2,20)
349+
(1,10)
350+
351+
query II nosort
352+
SELECT * FROM f_out_desc();
353+
----
354+
4 40
355+
3 30
356+
2 20
357+
1 10
358+
359+
statement ok
360+
DROP FUNCTION f_out_desc;
361+
362+
statement ok
363+
CREATE FUNCTION f_out_asc(OUT a INT, OUT b INT) RETURNS SETOF RECORD AS $$
364+
SELECT x, y FROM test_ordering ORDER BY x ASC;
365+
$$ LANGUAGE SQL;
366+
367+
query T nosort
368+
SELECT f_out_asc();
369+
----
370+
(1,10)
371+
(2,20)
372+
(3,30)
373+
(4,40)
374+
375+
query II nosort
376+
SELECT * FROM f_out_asc();
377+
----
378+
1 10
379+
2 20
380+
3 30
381+
4 40
382+
383+
statement ok
384+
DROP FUNCTION f_out_asc;
385+
386+
# Test with assignment casts using RETURNS SETOF RECORD and column definition list.
387+
# The function returns INT columns, but the column definition list specifies
388+
# FLOAT, which triggers assignment casts at query time.
389+
statement ok
390+
CREATE FUNCTION f_record_cast() RETURNS SETOF RECORD AS $$
391+
SELECT x, y FROM test_ordering ORDER BY x DESC;
392+
$$ LANGUAGE SQL;
393+
394+
query RR nosort
395+
SELECT * FROM f_record_cast() AS f(a FLOAT, b FLOAT);
396+
----
397+
4 40
398+
3 30
399+
2 20
400+
1 10
401+
402+
statement ok
403+
DROP FUNCTION f_record_cast;
404+
405+
# Test that ordering is preserved with array_agg
406+
statement ok
407+
CREATE FUNCTION f_out_for_agg(OUT a INT, OUT b INT) RETURNS SETOF RECORD AS $$
408+
SELECT x, y FROM test_ordering ORDER BY x DESC;
409+
$$ LANGUAGE SQL;
410+
411+
query T
412+
SELECT array_agg(f) FROM f_out_for_agg() AS f;
413+
----
414+
{"(4,40)","(3,30)","(2,20)","(1,10)"}
415+
416+
statement ok
417+
DROP FUNCTION f_out_for_agg;
418+
419+
# Test that ordering is preserved when a UDF returns a tuple and is used as a
420+
# data source. This exercises the expandRoutineTupleIntoCols code path.
421+
statement ok
422+
CREATE FUNCTION f_tuple() RETURNS SETOF RECORD AS $$
423+
SELECT ROW(x, y) FROM test_ordering ORDER BY x DESC;
424+
$$ LANGUAGE SQL;
425+
426+
query II nosort
427+
SELECT * FROM f_tuple() AS f(a INT, b INT);
428+
----
429+
4 40
430+
3 30
431+
2 20
432+
1 10
433+
434+
statement ok
435+
DROP FUNCTION f_tuple;
436+
DROP TABLE test_ordering;
437+
438+
subtest end

pkg/sql/opt/optbuilder/routine.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,7 @@ func (b *Builder) finalizeRoutineReturnType(
658658
// into a single tuple column.
659659
func (b *Builder) combineRoutineColsIntoTuple(stmtScope *scope) *scope {
660660
outScope := stmtScope.push()
661+
outScope.copyOrdering(stmtScope)
661662
elems := make(memo.ScalarListExpr, len(stmtScope.cols))
662663
typContents := make([]*types.T, len(stmtScope.cols))
663664
for i := range stmtScope.cols {
@@ -682,6 +683,7 @@ func (b *Builder) expandRoutineTupleIntoCols(stmtScope *scope) *scope {
682683
}
683684
tupleColID := stmtScope.cols[0].id
684685
outScope := stmtScope.push()
686+
outScope.copyOrdering(stmtScope)
685687
colTyp := b.factory.Metadata().ColumnMeta(tupleColID).Type
686688
for i := range colTyp.TupleContents() {
687689
varExpr := b.factory.ConstructVariable(tupleColID)
@@ -726,6 +728,7 @@ func (b *Builder) maybeAddRoutineAssignmentCasts(
726728
return stmtScope
727729
}
728730
outScope := stmtScope.push()
731+
outScope.copyOrdering(stmtScope)
729732
for i, col := range stmtScope.cols {
730733
scalar := b.factory.ConstructVariable(col.id)
731734
if !col.typ.Identical(desiredTypes[i]) {

pkg/sql/opt/optbuilder/testdata/udf

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -865,9 +865,9 @@ project
865865
├── params: i:1
866866
└── body
867867
└── project
868-
├── columns: column8:8
868+
├── columns: column8:8 b:3
869869
├── project
870-
│ ├── columns: column7:7
870+
│ ├── columns: column7:7 b:3
871871
│ ├── limit
872872
│ │ ├── columns: a:2!null b:3 c:4!null
873873
│ │ ├── internal-ordering: -3
@@ -911,9 +911,9 @@ project
911911
├── params: i:1
912912
└── body
913913
└── project
914-
├── columns: column8:8
914+
├── columns: column8:8 b:3
915915
├── project
916-
│ ├── columns: column7:7
916+
│ ├── columns: column7:7 b:3
917917
│ ├── limit
918918
│ │ ├── columns: a:2!null b:3 c:4!null
919919
│ │ ├── internal-ordering: -3

0 commit comments

Comments
 (0)