Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> Where are you planning to check this?
> In general, I'm trying to align it like a (self-imposed) permission check.
> For the query-like statements I'm looking at ExecCheckRTPerms(). (That
> also handles EXECUTE and EXPLAIN most easily.)
If you put it there then EXPLAIN UPDATE ... will bomb out. EXPLAIN
ANALYZE UPDATE *should* bomb out, but it'd be nice not to for the other
case. Not sure if it's worth kluging things to make that happen, though.
The executor doesn't currently know the difference between EXPLAIN and
EXPLAIN ANALYZE.
> Utility statements have a
> check in tcop/utility.c, COPY does it in DoCopy() (out of convenience).
> In any case you don't pay more than a 'if (XactReadOnly && ...)' if it's
> not activated.
Yeah, one if-test per statement isn't much overhead. What I'm more
worried about is making sure that all the places that need to check it
will check it; particularly in the utility-statement area, we shall
surely be adding more and more things that need to check it.
If it's done in ProcessUtility for utility statements then it's probably
fairly hard to miss for new statements. May I suggest that each
case branch that does not need to check it include an explicit comment to
that effect, eg.
case T_VacuumStmt: /* No XactReadOnly check since this logically changes no data */
vacuum((VacuumStmt*) parsetree); break;
Then it'll be hard to miss the need to think about this when adding a
new statement.
regards, tom lane