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

> I find that argument pretty shaky.

You are right. I was wrong to assume a GraphPropertyRef can only ever refer
to its own GRAPH_TABLE's level. Written inside a sub-select it does not.

> If I can write
> MATCH WHERE c.customer_id > 0
> why can't I write
> MATCH WHERE (SELECT c.customer_id > 0)
> ? I see that that in fact doesn't work, but that seems like a bug
> in itself.

Disallowing subqueries in GRAPH_TABLE is the current intent of the
feature. The code
in upstream today only checks p_hasSubLinks at the end of
transformRangeGraphTable, after the WHERE and COLUMNS list is transformed, so
it only catches subqueries that transform cleanly, i.e. "(SELECT 1)" gets the
intended error, but "(SELECT c.customer_id > 0)" fails with "missing FROM-clause
entry"

The attached v6 does two things. First, it rejects a SubLink under the
GRAPH_TABLE
expr kinds in transformSubLink, the moment it is seen and before the
sub-select body
is analyzed. So MATCH WHERE (SELECT c.customer_id > 0) now reports

```
ERROR: subqueries within GRAPH_TABLE reference are not supported
```

instead of

```
ERROR: missing FROM-clause entry for table "c"
```

Also, because GRAPH_TABLE now rejects subqueries, a property reference
can
never sit inside one, so it is always at its own GRAPH_TABLE's level. That lets
check_agg_arguments_walker treat a GraphPropertyRef as a Var with
varlevelsup 0,
which it otherwise cannot see since a GraphPropertyRef is not a Var.
The walker still
adjusts for the level it is found at, the same as for a Var.

This corrects the aggregate case I showed earlier. The patch includes
regression tests
for it.

> More generally, what this suggests to me is that we have a ton of
> other bugs-of-omission in places that process Vars and don't know that
> a GraphPropertyRef acts like a Var.

Agreed, and v6 is an example of this. It teaches check_agg_arguments_walker
that a GraphPropertyRef behaves like a Var.

Besides other potential bugs, It also limits the feature in the future.
Keeping subqueries out of GRAPH_TABLE is part of what makes some
of the bugs discovered fixable with the current GraphPropertyRef, but
I suspect it will be a real limitation if we try to relax these restrictions.

> 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.

The design has some open questions, but I will share if this is the direction
to go.

--
Sami Imseih
Amazon Web Services (AWS)