Обсуждение: [Patch]Add Graph* node support to expression_tree_mutator

On Tue, Apr 21, 2026 at 9:26 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:
>
> Hi hackers,
>
> expression_tree_mutator_impl() was missing case handlers for
> T_GraphPattern, T_GraphElementPattern, and T_GraphPropertyRef.
> The corresponding expression_tree_walker_impl() already handled
> all three node types, but the mutator did not, causing an
> "unrecognized node type: 106" error whenever a GRAPH_TABLE
> subquery appeared in a HAVING clause.
>
> SELECT 1 FROM hv GROUP BY id
> HAVING (SELECT COUNT(*) FROM GRAPH_TABLE(gh MATCH (a) COLUMNS (a.id AS x))) > 0;
> SET
> ERROR:  unrecognized node type: 106
>
> Attached a patch to address this.

Thanks for the report and the patch.

I first thought that the nodes needn't be part of the mutator since
they will be rewritten into some other nodes. But in this case the
mutator is being called from the transformation phase which does not
rewrite these nodes. I also found that range_table_mutator_impl()
invokes MUTATE on RangeTblEntry::graph_table and
RangeTblEntry::graph_table_columns which contain these three nodes in
their trees. So it seems like we forgot to handle these three nodes in
expression_tree_mutator(). From the name of the function, I thought it
only handles expressions (nodes which have Expr as their first
member). In that case adding GraphElementPattern and GraphPattern
should be handled somewhere else. But it seems the function also
handles Node types as well (e.g. List, TableFunc).

GraphPropertyRef has char *elvarname. Only the char * is copied by
FLATCOPY but not the character string it's pointing to. That made me a
bit uncomfortable because the mutated GraphPropertyRef would point to
the old name. But it looks like that's how other node types are
handled in that function. E.g. XmlExpr has member name which is also
FLATCOPY'ed. So we are good.

Why is MUTATE not called on GraphElementPattern::labelexpr and
GraphElementPattern::quantifier?

The test query is placed at the appropriate section in the file. I
would try to fit the query in lesser lines by placing SELECT, FROM,
GROUP BY on the same line. But that's more of a personal style.

--
Best Wishes,
Ashutosh Bapat

On Thu, Apr 23, 2026 at 12:18 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
>
> Why is MUTATE not called on GraphElementPattern::labelexpr and
> GraphElementPattern::quantifier?

The walker didn't WALK labelexpr and quantifier either. Given that
labelexpr is a boolean expression it needs to be WALKed. quantifier is
IntList which is ignored by other nodes as well when WALKing. But we
need to copy quantifier in mutator otherwise the mutated expression
will point to the same IntList as the original node. Other nodes also
copy the OidLists and IntLists when mutating them. I have modified the
test query to cover the label expression mutator.

raw_expression_walker didn't cover labelexpr either. Added it there.
The label expression at that stage contains ColumnRef and BoolExpr
which are already covered by raw expression tree walker.

--
Best Wishes,
Ashutosh Bapat

On Tue, Apr 28, 2026 at 11:05 AM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> The walker didn't WALK labelexpr and quantifier either. Given that
> labelexpr is a boolean expression it needs to be WALKed. quantifier is
> IntList which is ignored by other nodes as well when WALKing. But we
> need to copy quantifier in mutator otherwise the mutated expression
> will point to the same IntList as the original node. Other nodes also
> copy the OidLists and IntLists when mutating them. I have modified the
> test query to cover the label expression mutator.
>
> raw_expression_walker didn't cover labelexpr either. Added it there.
> The label expression at that stage contains ColumnRef and BoolExpr
> which are already covered by raw expression tree walker.

Hi,

Thanks for working on this. I ran into it independently today, and
then discovered this thread. In expression_tree_mutator_impl,
T_GraphLabelRef can be added to the "Primitive node types with no
expression subnodes" section just as was done in
expression_tree_walker_impl.

Other than that, this looks good to me.

--
Robert Haas
EDB: http://www.enterprisedb.com

On Thu, Apr 30, 2026 at 2:17 AM Robert Haas <robertmhaas@gmail.com> wrote:
>

Thanks for reviewing the patch.

> Thanks for working on this. I ran into it independently today, and
> then discovered this thread. In expression_tree_mutator_impl,
> T_GraphLabelRef can be added to the "Primitive node types with no
> expression subnodes" section just as was done in
> expression_tree_walker_impl.

Right. Fixed in the attached patch. Both GraphPropertyLabel and
GraphLabelRef need to be placed in that section since both are
primitive nodes.

--
Best Wishes,
Ashutosh Bapat

On 30.04.26 08:44, Ashutosh Bapat wrote:
> On Thu, Apr 30, 2026 at 2:17 AM Robert Haas <robertmhaas@gmail.com> wrote:
>>
> 
> Thanks for reviewing the patch.
> 
>> Thanks for working on this. I ran into it independently today, and
>> then discovered this thread. In expression_tree_mutator_impl,
>> T_GraphLabelRef can be added to the "Primitive node types with no
>> expression subnodes" section just as was done in
>> expression_tree_walker_impl.
> 
> Right. Fixed in the attached patch. Both GraphPropertyLabel and
> GraphLabelRef need to be placed in that section since both are
> primitive nodes.

Committed.  (I reformatted the test query a little bit as you had 
suggested upstream.  Also, the ordering of the switch cases was slightly 
different between the walker and the mutator, which I fixed.)