Dean Rasheed <[email protected]> writes:
> The majority of RewriteQuery() is safe if it's called a second time on
> the same query, because it fully expands all non-SELECT rules and
> auto-updatable target views, so the second time round, it would do
> nothing of that kind. However, evidently it's not safe to call
> rewriteTargetListIU() twice. Of course, we could attempt to make
> rewriteTargetListIU() idempotent, but that might be more effort, and
> in any case, processing parts of the query multiple times is a waste
> of effort even if it doesn't fail, so I think the approach taken by
> the patch is the right one.
Not sure. As you say, RewriteQuery() was idempotent up until the
addition of generated columns. Who knows what other places are
relying on that property, that haven't yet been tested against
cases including generated columns? I think there's a good case for
trying to fix it by restoring that idempotency.
Of course, that might prove impractical. Bernice's fix does have
the attraction of being short and sweet.
On the third hand ... if we need to rewrite the query twice in
the first place, is it really true that "Data-modifying WITH clauses
need only be rewritten once per query"? How do we know that whatever
prompted the repeat rewrite didn't change the WITH clauses?
I've not dug into the code yet, just reacting to what I see in this
thread.
regards, tom lane