Re: [HACKERS] Read-only transactions
Tom Lane writes:
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.
Well, I had one big check at the top so it doesn't have to be spread out
so much:
static void
check_xact_readonly(Node *parsetree)
{
if (!XactReadOnly)
return;
switch (nodeTag(parsetree))
{
case T_AlterDatabaseSetStmt:
case T_AlterDomainStmt:
case T_AlterGroupStmt:
[...]
case T_DropUserStmt:
case T_GrantStmt:
case T_TruncateStmt:
elog(ERROR, transaction is read-only);
break;
}
}
--
Peter Eisentraut [EMAIL PROTECTED]
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] Read-only transactions
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.) 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. As such it's not clear to me why vacuum and checkpoint are included in the forbidden list. They don't logically change any data. The same might be said of reindex. You're right. I'll allow that class of statements. -- Peter Eisentraut [EMAIL PROTECTED] ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] Read-only transactions
Peter Eisentraut [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] Read-only transactions
Peter Eisentraut [EMAIL PROTECTED] writes: I would like to implement read-only transactions following the SQL spec, ... I think it's light-weight and marginally useful. Light-weight would depend on your intended implementation, I suppose. Where are you planning to check this? Also, the fact that you are excluding temp tables seems to suggest that this is a very high-level, abstract notion of read-only-ness; it's certainly got little to do with whether we try to write on the disk. As such it's not clear to me why vacuum and checkpoint are included in the forbidden list. They don't logically change any data. The same might be said of reindex. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
