From: torikoshia <torikoshia@oss.nttdata.com>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails
Date: Fri, 10 Sep 2021 23:10:43 +0900
Message-ID: <d70fa4dfd333b1de1024e50c2ad1a6ea@oss.nttdata.com> (raw)
In-Reply-To: <b6fecc71-60b9-bd44-4b78-f2a9dd74f46f@enterprisedb.com>
References: <5bafa66ad529e11860339565c9e7c166@oss.nttdata.com>
<3208199.1631039493@sss.pgh.pa.us>
<b6fecc71-60b9-bd44-4b78-f2a9dd74f46f@enterprisedb.com>
On 2021-09-09 19:03, Peter Eisentraut wrote:
> On 07.09.21 20:31, Tom Lane wrote:
>> torikoshia <torikoshia@oss.nttdata.com> writes:
>>> While working on [1], we found that EXPLAIN(VERBOSE) to CTE with
>>> SEARCH
>>> BREADTH FIRST ends up ERROR.
>>
>> Yeah. It's failing here:
>>
>> * We're deparsing a Plan tree so we don't have a
>> CTE
>> * list. But the only place we'd see a Var
>> directly
>> * referencing a CTE RTE is in a CteScan plan
>> node, and we
>> * can look into the subplan's tlist instead.
>>
>> if (!dpns->inner_plan)
>> elog(ERROR, "failed to find plan for CTE %s",
>> rte->eref->aliasname);
>>
>> The problematic Var is *not* in a CteScan plan node; it's in a
>> WorkTableScan node. It's not clear to me whether this is a bug
>> in the planner's handling of SEARCH BREADTH FIRST, or if the plan
>> is as-intended and ruleutils.c is failing to cope.
>
> The search clause is resolved by the rewriter, so it's unlikely that
> the planner is doing something wrong. Either the rewriting produces
> something incorrect (but then one might expect that the query results
> would be wrong), or the structures constructed by rewriting are not
> easily handled by ruleutils.c.
>
> If we start from the example in the documentation
> <https://www.postgresql.org/docs/14/queries-with.html#QUERIES-WITH-RECURSIVE;:
>
> """
> WITH RECURSIVE search_tree(id, link, data, depth) AS (
> SELECT t.id, t.link, t.data, 0
> FROM tree t
> UNION ALL
> SELECT t.id, t.link, t.data, depth + 1
> FROM tree t, search_tree st
> WHERE t.id = st.link
> )
> SELECT * FROM search_tree ORDER BY depth;
>
> To get a stable sort, add data columns as secondary sorting columns.
> """
>
> In order to handle that part about the stable sort, the query
> constructed internally is something like
>
> WITH RECURSIVE search_tree(id, link, data, seq) AS (
> SELECT t.id, t.link, t.data, ROW(0, id, link)
> FROM tree t
> UNION ALL
> SELECT t.id, t.link, t.data, ROW(seq.depth + 1, id, link)
> FROM tree t, search_tree st
> WHERE t.id = st.link
> )
> SELECT * FROM search_tree ORDER BY seq;
>
> The bit "seq.depth" isn't really valid when typed in like that, I
> think, but of course internally this is all wired together with
> numbers rather than identifiers. I suspect that that is what
> ruleutils.c trips over.
Thanks for your advice, it seems right.
EXPLAIN VERBOSE can be output without error when I assigned testing
purpose CoercionForm to 'seq.depth + 1'.
I've attached the patch for the changes made for this test for your
reference, but I'm not sure it's appropriate for creating a new
CoercionForm to fix the issue..
--
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:v1-0001-fix_err_explain_verbose_cte_breadth.patch (text/x-diff, 1.7K) downloaddiff --git a/src/backend/rewrite/rewriteSearchCycle.c b/src/backend/rewrite/rewriteSearchCycle.cindex c50ebdba24..659a5c440f 100644--- a/src/backend/rewrite/rewriteSearchCycle.c+++ b/src/backend/rewrite/rewriteSearchCycle.c@@ -508,7 +508,7 @@ rewriteSearchAndCycle(CommonTableExpr *cte)
fs->resulttype = INT8OID;
fs->resulttypmod = -1;
- fexpr = makeFuncExpr(F_INT8INC, INT8OID, list_make1(fs), InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL);+ fexpr = makeFuncExpr(F_INT8INC, INT8OID, list_make1(fs), InvalidOid, InvalidOid, COERCE_NOMINAL_CALL);
lfirst(list_head(search_col_rowexpr->args)) = fexpr;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.cindex 8ff4e5dc07..c5a5fa06f3 100644--- a/src/backend/utils/adt/ruleutils.c+++ b/src/backend/utils/adt/ruleutils.c@@ -9648,6 +9648,12 @@ get_func_expr(FuncExpr *expr, deparse_context *context,
return;
}
+ if (expr->funcformat == COERCE_NOMINAL_CALL)+ {+ appendStringInfo(context->buf, "nominal func");+ return;+ }+
/*
* Normal function: display as proname(args). First we need to extract
* the argument datatypes.
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.hindex c04282f91f..a3d600d1a2 100644--- a/src/include/nodes/primnodes.h+++ b/src/include/nodes/primnodes.h@@ -483,7 +483,8 @@ typedef enum CoercionForm
COERCE_EXPLICIT_CALL, /* display as a function call */
COERCE_EXPLICIT_CAST, /* display as an explicit cast */
COERCE_IMPLICIT_CAST, /* implicit cast, so hide it */
- COERCE_SQL_SYNTAX /* display with SQL-mandated special syntax */+ COERCE_SQL_SYNTAX, /* display with SQL-mandated special syntax */+ COERCE_NOMINAL_CALL
} CoercionForm;
/*
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: torikoshia@oss.nttdata.com, peter.eisentraut@enterprisedb.com, tgl@sss.pgh.pa.us
Subject: Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails
In-Reply-To: <d70fa4dfd333b1de1024e50c2ad1a6ea@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox