Peter Suter wrote: > I'm still not sure if this is a bug in the plugin or in Trac or in > both. In http://trac.edgewall.org/ticket/10451#comment:1 the idea was > mentioned that postgresql backend might need automatic rollback on > error. A workaround was added to the TagsPlugin and the idea was > dropped / changed because of that. But it still doesn't work...
TagsPlugin is one of the only plugins that checks if its DB schema exists by selecting on a table and catching the exception if it doesn't. As you noticed, different DB backends react differently in that case. Moreover, this strategy interacts badly with nested transaction blocks (as does an explicit db.rollback() by the plugin), because it messes up the whole transaction, not only the innermost one. So we want to discourage this practice. The standard way of checking that the DB schema for a plugin exists is by adding a row to the `system` table, using the schema version as the value, and checking for the existence (and value, in the case of upgrades) of that row. I would suggest that the TagsPlugin switches to that strategy. But this still leaves the transition from the current state. It may work to open a nested transaction block in the plugin's environment_needs_upgrade() for the potentially failing query, and to call db.rollback() in case of failure. Now that each environment_needs_upgrade() is called within a separate block, it shouldn't have any effect on other plugins, and it should be reasonably safe. Same strategy for upgrade_environment(). -- Remy
signature.asc
Description: OpenPGP digital signature
