Dean Rasheed <[email protected]> writes:
> I think it is true that RewriteQuery() doesn't need to rewrite
> individual CTEs multiple times. However, that doesn't mean that it
> doesn't need to process the cteList at all below the top-level. The
> problem is that rule actions may add additional CTEs to the list,
> which do need to be processed when recursing.
Yeah, I had come to the same conclusion but not yet built an example.
I think the core of the matter may lie in the stanza headed
* If the original query has any CTEs, copy them into the rule action. But
* we don't need them for a utility action.
at lines 565ff in rewriteHandler.c. What we are doing there, I think,
is to combine already-rewritten CTEs from the original query with
not-yet-rewritten queries from the rule action. This can't work
unless the subsequent rewriting of the merged list is idempotent.
I was toying with the idea of decoupling rewriting of WITHs from
the main recursion. This would look roughly like
1. Pull out RewriteQuery's first loop into a new function, say
RewriteQueryCTEs. Call this from QueryRewrite just before calling
RewriteQuery.
2. Also apply it to a rule action's CTE list in fireRules, before
we merge the original query's CTE list into that list.
Not sure if there are any other call sites needed (rewriteTargetView
might need its own call), or if this would even fix the problem.
We might need some more-explicit labeling of whether rewrite
has happened. That would mean querytree changes, making the
fix not-back-patchable. But given the lack of complaints
to date, maybe a HEAD-only fix is acceptable.
There's an awful lot of unfinished work about CTEs in this module,
eg the two different random implementation restrictions in the
same stanza that's combining the CTE lists, or the one at the
bottom of RewriteQuery. I wonder if we have the ambition to
actually do anything about all that. People tend to see the
rules system as a deprecated backwater, so maybe not.
regards, tom lane
