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

Sami Imseih <samimseih(at)gmail(dot)com> writes:
> postgres=# SELECT EXISTS(SELECT nm FROM GRAPH_TABLE (myshop MATCH (c
> IS customers WHERE count(c.customer_id + o.customer_id) > 0) COLUMNS
> (c.name AS nm)) t) FROM customers o;
> ERROR: Upper-level Var found where not expected

Ouch.

> So, based on your explanation earlier, this led me to find that the
> leveling is still wrong. Specifically, min_varlevel > 0, since the
> level-zero GraphPropertyRef is not accounted for in
> check_agg_arguments_walker. In this case min_varlevel should be 0.

Right, I came to the same conclusion.

> The fix is to teach check_agg_arguments_walker that a GraphPropertyRef
> carries a level, the GRAPH_TABLE's own level. This is safe because a
> GraphPropertyRef can only ever refer to its own GRAPH_TABLE's level
> and can never appear inside a sub-select, so treating it as a level-zero
> reference is always correct.

I find that argument pretty shaky. 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.

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. 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. We have
learned the hard way in the past that you don't want to invent things
that act like Vars but aren't Vars; if you must, you are going to be
doing a heck of a lot of work to patch up everyplace that will need to
know about them. The hazard here is limited somewhat by the fact that
the planner and executor will never see GraphPropertyRefs, but I'm
afraid that still leaves plenty of scope for bugs (in the rewriter, in
particular).

regards, tom lane