Joe Conway <mail@joeconway.com> writes:
> The patch at the root of this discussion eliminates both issues, but
> leaves us with no check at all in the INSTEAD case. Is there any way to
> do the permission checks on the original query in the INSTEAD case, even
> though the query itself will never be executed?
I thought about that a bit, but I'm not sure it's any less invasive a
change. Currently the original query doesn't get shipped to the
executor at all, if it's suppressed by INSTEAD. In place of the
original query, the rewriter would have to generate some kind of
"no-op query" that the executor would run as far as checking the rtable
permissions for, and then stop. (There is a CMD_NOTHING value of
CmdType, but I don't think it's supported anywhere outside the rule
rewriter, and the rewriter's semantics for it may not be quite right
anyway.) Adding a new CmdType would have cascading effects in a bunch
of places. While it's probably not hard to fix any one of them
individually, overall this doesn't look any easier or cleaner than
extending the RTE permissions flag representation.
The only argument I can see in favor of doing it that way would be that
by avoiding a change in RTE representation, we would have a patch that
could be applied in the 7.4.* series. (If we change RTEs then we can
only fix it in 7.5, because the change to stored rule representation
would force initdb.) But the patch would be extensive enough that I'd
be pretty hesitant to apply it to 7.4.* anyway.
What might be the best compromise is to fix it properly in 7.5 and
revert the previous patch in 7.4.*. That would restore correct checking
of view permissions for all but the case where a rule substitutes a
different query type; which is a bug that we had for a very long time
before anyone noticed, so maybe leaving it unfixed in 7.4.* is okay.
regards, tom lane