Skip to content

Commit 1a95ec7

Browse files
committed
optbuilder: preserve ORDER BY in set-returning UDFs with OUT parameters
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. This commit fixes the issue by calling copyOrdering() in two places: 1. combineRoutineColsIntoTuple - when wrapping columns into a tuple 2. maybeAddRoutineAssignmentCasts - 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. Fixes #144013 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. Epic: None
1 parent ae0b3d7 commit 1a95ec7

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
@@ -653,6 +653,7 @@ func (b *Builder) finalizeRoutineReturnType(
653653
// into a single tuple column.
654654
func (b *Builder) combineRoutineColsIntoTuple(stmtScope *scope) *scope {
655655
outScope := stmtScope.push()
656+
outScope.copyOrdering(stmtScope)
656657
elems := make(memo.ScalarListExpr, len(stmtScope.cols))
657658
typContents := make([]*types.T, len(stmtScope.cols))
658659
for i := range stmtScope.cols {
@@ -677,6 +678,7 @@ func (b *Builder) expandRoutineTupleIntoCols(stmtScope *scope) *scope {
677678
}
678679
tupleColID := stmtScope.cols[0].id
679680
outScope := stmtScope.push()
681+
outScope.copyOrdering(stmtScope)
680682
colTyp := b.factory.Metadata().ColumnMeta(tupleColID).Type
681683
for i := range colTyp.TupleContents() {
682684
varExpr := b.factory.ConstructVariable(tupleColID)
@@ -721,6 +723,7 @@ func (b *Builder) maybeAddRoutineAssignmentCasts(
721723
return stmtScope
722724
}
723725
outScope := stmtScope.push()
726+
outScope.copyOrdering(stmtScope)
724727
for i, col := range stmtScope.cols {
725728
scalar := b.factory.ConstructVariable(col.id)
726729
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)