Re: Disallow outer-level and WHERE-clause aggregates in GRAPH_TABLE

On Tue, Aug 25, 2026 at 11:57 AM Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> > > So what I'm thinking right now is
> > > that this is a fundamental design error, and that we should nuke
> > > GraphPropertyRef altogether in favor of using a Var that references
> > > the appropriate column of the RTE_GRAPH_TABLE relation.
> >
> > I spent time today on this and I think the direction is right. In what
> > I have running
> > locally, a property reference is emitted as a plain Var over the
> > RTE_GRAPH_TABLE relation,
> > so the Var handles leveling and the rest for free. GraphPropertyRef
> > does not go away
> > entirely, though. It moves onto a side list on the RTE that the
> > rewriter uses to resolve
> > each property Var back to its graph property.
> >
>
> I share the concern raised by Tom about bugs due to missing
> GraphPropertyRef handling. However, I don't think we can use Var
> instead of GraphPropertyRef. It represents a reference to a property
> of all the graph elements that are bound to an element pattern in a
> GRAPH_TABLE. The element pattern is not represented by a range table
> and Var node does not have a field to represent an element pattern
> variable. Further the same GraphPropertyRef may be rewritten as
> different expressions depending on the element table it gets
> associated with when generating a subquery for a given path. Achieving
> this with Var will be quite complex, if not impossible. The question
> is whether we can find all the places where we need to treat
> GraphPropertyRef as a Var. Given that the GraphPropertyRefs are
> generated during transformation and vanish after rewrite, the places
> where we need to handle them are limited, as noted by Tom already.
> Many of those are already covered by the patch. Going through all the
> places, rather painfully, where we use levelsup for Var-like nodes,
> looking at var.c, I see that most of that code is applicable to
> planning, optimization and execution. The additional places which I
> think we need to handle are:
> a. locate_var_of_level_walker() to handle GraphPropertyRef so that the
> correct error location can be reported. We have separate
> locate_aggref_of_level_walker(). So first I thought that it would be
> appropriate to add a separate
> locate_graphpropertyref_of_level_walker() function. But we want to
> report an error from a place where the distinction between Var and
> GraphPropertyRef is lost. So a separate function doesn't make sense.
> b. contain_vars_of_level() should not see GraphPropertyRef since the
> latter lives within GRAPH_TABLE only. But the function is used during
> query transformation, so it will be good to add a GraphPropertyRef
> case and Assert()/throw error if it is seen. We should add a comment
> there to expect GraphPropertyRef there once we start supporting nested
> subqueries in GRAPH_TABLE.
> c. IncrementVarSublevelsUp() will need to handle GraphPropertyRef once
> we support nested subqueries in GRAPH_TABLE. For now we may want to
> add a GraphPropertyRef case and Assert()/throw error if it is seen. We
> should add a comment there to expect GraphPropertyRef there once we
> start supporting nested subqueries in GRAPH_TABLE. Please note, I
> don't expect ChangeVarNodes to handle GraphPropertyRef since there is
> no rtindex in GraphPropertyRef.
>
> I didn't find any other place where we need to handle
> GraphPropertyRef. But there are just many places where we use Vars and
> varlevelsup. So there is a non-zero possibility that I may have missed
> a few. I wish we could have differentiated between Var in the planner,
> optimizer code and ColumnRef in the parser, transformer and rewriter
> code. That would have made it easier to find the places where we need
> to handle GraphPropertyRef. But the ship has sailed long back.

[RMT hat]

RMT is wondering if the ongoing work on this is an enhancement for 20
or proposed work for 19. Does Ashutosh's reply satisfy Tom and Sami's
concerns with the design. Is there work here that is still being
proposed for v19 and is there consensus on it?

- Melanie