Thanks for the comments! > I do not like this fix approach, and for that matter I don't like > f58567105. This is expensive thanks to the extra tree traversal,
I was not feeling totally convinced about this either, but was not sure if the tree traversal is that big of a problem. We should definitely avoid doing this, if we can. > f58567105 is user-unfriendly because it fails to say exactly what > or where is the construct it's rejecting, and neither patch is > following the perfectly good structure that the parser already > has for this kind of check. IMO the correct way to handle this > restriction is to check it in check_agglevels_and_constraints() based > on the ParseExprKind of the surrounding expression. You are right. This makes sense to me now. We can introduce EXPR_KIND_GRAPH_TABLE_COLUMNS and EXPR_KIND_GRAPH_TABLE_WHERE ParseExprKind's. Inside check_agglevels_and_constraints(), we can check the restriction before we walk up to the query level the aggregate belongs to. Also, transformWindowFuncCall() and check_srf_call_placement() should do the same thing, meaning the work done in f58567105is effectively reverted. The attached patch does this. Is this what you have in mind? -- Sami
v2-0001-Rework-GRAPH_TABLE-aggregate-window-SRF-rejection.patch
Description: Binary data
