From d57534740b7682c2816ce6c54318521787fcfacf Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 16 Oct 2022 19:18:08 -0400 Subject: [PATCH] Fix EXPLAIN of SEARCH BREADTH FIRST with a constant initial value. If the non-recursive term of a SEARCH BREADTH FIRST recursive query has only constants in its target list, the planner will fold the starting RowExpr added by rewrite into a simple Const of type RECORD. The executor doesn't have any problem with that --- but EXPLAIN VERBOSE will encounter the Const as the ultimate source of truth about what the field names of the SET column are, and it didn't know what to do with that. Fortunately, we can pull the identifying typmod out of the Const, in much the same way that record_out would. For reasons that remain a bit obscure to me, this only fails with SEARCH BREADTH FIRST, not SEARCH DEPTH FIRST or CYCLE. But I added regression test cases for both of those options too, just to make sure we don't break it in future. Per bug #17644 from Matthijs van der Vleuten. Back-patch to v14 where these constructs were added. Discussion: https://postgr.es/m/17644-3bd1f3036d6d7a16@postgresql.org --- src/backend/utils/adt/ruleutils.c | 3 +- src/backend/utils/fmgr/funcapi.c | 34 ++++++++ src/test/regress/expected/with.out | 120 +++++++++++++++++++++++++++++ src/test/regress/sql/with.sql | 52 +++++++++++++ 4 files changed, 208 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 855d48372a..70d723e80c 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -7500,7 +7500,8 @@ get_name_for_var_field(Var *var, int fieldno, /* * If it's a RowExpr that was expanded from a whole-row Var, use the - * column names attached to it. + * column names attached to it. (We could let get_expr_result_tupdesc() + * handle this, but it's much cheaper to just pull out the name we need.) */ if (IsA(var, RowExpr)) { diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c index a1fe50ffca..78eb60c3e8 100644 --- a/src/backend/utils/fmgr/funcapi.c +++ b/src/backend/utils/fmgr/funcapi.c @@ -339,6 +339,40 @@ get_expr_result_type(Node *expr, *resultTupleDesc = BlessTupleDesc(tupdesc); return TYPEFUNC_COMPOSITE; } + else if (expr && IsA(expr, Const) && + ((Const *) expr)->consttype == RECORDOID && + !((Const *) expr)->constisnull) + { + /* + * When EXPLAIN'ing some queries with SEARCH/CYCLE clauses, we may + * need to resolve field names of a RECORD-type Const. The datum + * should contain a typmod that will tell us that. + */ + HeapTupleHeader rec; + Oid tupType; + int32 tupTypmod; + + rec = DatumGetHeapTupleHeader(((Const *) expr)->constvalue); + tupType = HeapTupleHeaderGetTypeId(rec); + tupTypmod = HeapTupleHeaderGetTypMod(rec); + if (resultTypeId) + *resultTypeId = tupType; + if (tupType != RECORDOID || tupTypmod >= 0) + { + /* Should be able to look it up */ + if (resultTupleDesc) + *resultTupleDesc = lookup_rowtype_tupdesc_copy(tupType, + tupTypmod); + return TYPEFUNC_COMPOSITE; + } + else + { + /* This shouldn't really happen ... */ + if (resultTupleDesc) + *resultTupleDesc = NULL; + return TYPEFUNC_RECORD; + } + } else { /* handle as a generic expression; no chance to resolve RECORD */ diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 7f2e32d8b0..f3fd1cd32a 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -790,6 +790,83 @@ select * from search_graph order by seq; 4 | 5 | arc 4 -> 5 | (1,4,5) (7 rows) +-- a constant initial value causes issues for EXPLAIN +explain (verbose, costs off) +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search depth first by x set y +select * from test limit 5; + QUERY PLAN +----------------------------------------------------------------------------------------- + Limit + Output: test.x, test.y + CTE test + -> Recursive Union + -> Result + Output: 1, '{(1)}'::record[] + -> WorkTable Scan on test test_1 + Output: (test_1.x + 1), array_cat(test_1.y, ARRAY[ROW((test_1.x + 1))]) + -> CTE Scan on test + Output: test.x, test.y +(10 rows) + +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search depth first by x set y +select * from test limit 5; + x | y +---+----------------------- + 1 | {(1)} + 2 | {(1),(2)} + 3 | {(1),(2),(3)} + 4 | {(1),(2),(3),(4)} + 5 | {(1),(2),(3),(4),(5)} +(5 rows) + +explain (verbose, costs off) +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search breadth first by x set y +select * from test limit 5; + QUERY PLAN +-------------------------------------------------------------------------------------------- + Limit + Output: test.x, test.y + CTE test + -> Recursive Union + -> Result + Output: 1, '(0,1)'::record + -> WorkTable Scan on test test_1 + Output: (test_1.x + 1), ROW(int8inc((test_1.y)."*DEPTH*"), (test_1.x + 1)) + -> CTE Scan on test + Output: test.x, test.y +(10 rows) + +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search breadth first by x set y +select * from test limit 5; + x | y +---+------- + 1 | (0,1) + 2 | (1,2) + 3 | (2,3) + 4 | (3,4) + 5 | (4,5) +(5 rows) + -- various syntax errors with recursive search_graph(f, t, label) as ( select * from graph0 g @@ -1132,6 +1209,49 @@ select * from search_graph; 2 | 3 | arc 2 -> 3 | N | {"(1,4)","(4,5)","(5,1)","(1,2)","(2,3)"} (25 rows) +explain (verbose, costs off) +with recursive test as ( + select 0 as x + union all + select (x + 1) % 10 + from test +) cycle x set is_cycle using path +select * from test; + QUERY PLAN +----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + CTE Scan on test + Output: test.x, test.is_cycle, test.path + CTE test + -> Recursive Union + -> Result + Output: 0, false, '{(0)}'::record[] + -> WorkTable Scan on test test_1 + Output: ((test_1.x + 1) % 10), CASE WHEN (ROW(((test_1.x + 1) % 10)) = ANY (test_1.path)) THEN true ELSE false END, array_cat(test_1.path, ARRAY[ROW(((test_1.x + 1) % 10))]) + Filter: (NOT test_1.is_cycle) +(9 rows) + +with recursive test as ( + select 0 as x + union all + select (x + 1) % 10 + from test +) cycle x set is_cycle using path +select * from test; + x | is_cycle | path +---+----------+----------------------------------------------- + 0 | f | {(0)} + 1 | f | {(0),(1)} + 2 | f | {(0),(1),(2)} + 3 | f | {(0),(1),(2),(3)} + 4 | f | {(0),(1),(2),(3),(4)} + 5 | f | {(0),(1),(2),(3),(4),(5)} + 6 | f | {(0),(1),(2),(3),(4),(5),(6)} + 7 | f | {(0),(1),(2),(3),(4),(5),(6),(7)} + 8 | f | {(0),(1),(2),(3),(4),(5),(6),(7),(8)} + 9 | f | {(0),(1),(2),(3),(4),(5),(6),(7),(8),(9)} + 0 | t | {(0),(1),(2),(3),(4),(5),(6),(7),(8),(9),(0)} +(11 rows) + -- multiple CTEs with recursive graph(f, t, label) as ( diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index 0f5730797f..f876c66351 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -414,6 +414,41 @@ with recursive search_graph(f, t, label) as ( ) search breadth first by f, t set seq select * from search_graph order by seq; +-- a constant initial value causes issues for EXPLAIN +explain (verbose, costs off) +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search depth first by x set y +select * from test limit 5; + +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search depth first by x set y +select * from test limit 5; + +explain (verbose, costs off) +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search breadth first by x set y +select * from test limit 5; + +with recursive test as ( + select 1 as x + union all + select x + 1 + from test +) search breadth first by x set y +select * from test limit 5; + -- various syntax errors with recursive search_graph(f, t, label) as ( select * from graph0 g @@ -561,6 +596,23 @@ with recursive search_graph(f, t, label) as ( ) cycle f, t set is_cycle to 'Y' default 'N' using path select * from search_graph; +explain (verbose, costs off) +with recursive test as ( + select 0 as x + union all + select (x + 1) % 10 + from test +) cycle x set is_cycle using path +select * from test; + +with recursive test as ( + select 0 as x + union all + select (x + 1) % 10 + from test +) cycle x set is_cycle using path +select * from test; + -- multiple CTEs with recursive graph(f, t, label) as (