> 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)
v6-0001-Rework-GRAPH_TABLE-aggregate-window-SRF-rejection.patch
Description: Binary data
