Cant Upgrade Trac After Updating to 0.10-dev

[simbateman@…]

When running the

trac-admin /path/to/project upgrade

command it errors stating there is already a session_old table. After a short look around it apprears that in root/trunk/trac/upgrades/db18.py and db15.py these scripts dont remove the session_old table when they are done with it.

I'll post patches tomorrow when I get back to the office.

[Jonas Borgström]

It shouldn't be necessary to remove the session_old tables explicitly since they are created with "CREATE TEMPORARY TABLE" and shouuld be removed by the database at the end of the transaction.

This probably means that the session_old table you have in your database was created by somebody else and not by trac. I'm closing this as invalid, please reopen if you have some more information about where they session_old table came from.

[Christian Boos]

I think the ticket is valid. The problem occurs if both upgrades/db15.py and upgrades/db18.py are executed during the same upgrade process, as the same connection will be used for both set of commands.

From SQLite's doc:

If the "TEMP" or "TEMPORARY" keyword occurs in between "CREATE" and "TABLE" then the table that is created is only visible within that same database connection and is automatically deleted when the database connection is closed.

and from our code:

• source:trunk/trac/env.py@3423#L328​: get the connection once
• source:trunk/trac/env.py@3423#L373​: even reusing the same cursor here; I think we should explicitely close() and re-obtain the connection, for the temp tables to be cleared.

[tdad]

i just wanted to mention that i upgraded a 0.9.5 envrionment (sqlite) succesfully 5 minutes ago (r3431)

[Christian Boos]

upgrades/db15.py is already there in 0.9.5, so that's why it was not a problem for you. It seems that specific problem should only occur when upgrading from pre-0.9 versions but the general issue which I explained above remains.

It's probably not a high prio issue, given that no 0.9.x versions is affected.

[Christopher Lenz]

I would think (well, hope) that temporary tables are bound to transactions and not connections.

[Christian Boos]

Even if you were right, the problem would remain, as there's currently only one transaction for all environment upgrades.

[Christopher Lenz]

Yeah, I know. The idea behind that was that upgrades should be atomic. However, DDL in transactions seems to be a bad idea anyway, so I'd be in favor of committing after each upgrade step. Closing/reopening the connection would be more painful.

[Christian Boos]

More painful, but nevertheless necessary, if we don't want to remove the table explicitely:

>>> cnx = sqlite.Connection("test.db")
>>> cnx.execute("begin")
<pysqlite2.dbapi2.Cursor object at 0x00AEBC80>
>>> cnx.execute("CREATE TEMPORARY TABLE test ( x  int )")
<pysqlite2.dbapi2.Cursor object at 0x00A3E860>
>>> cnx.commit()
>>> cnx.execute("CREATE TEMPORARY TABLE test ( x  int )")
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
pysqlite2.dbapi2.OperationalError: table test already exists

That being said, it would probably be a lot simpler to explicitely drop that session_old table rather than to overcomplexify the upgrade code.

[Jonas Borgström]

Yeah, I agree that using standard CREATE/DROP TABLE is probably the best solution.

It's unfortunate that some databases like mysql (also currently true for sqlite due to a misfeature in pysqlite2) automatically commits the transaction before DDL statements making it impossible for us to make the entire upgrade process atomic. This however should however not stop us from trying since some databases like postgresql handles this just fine.

I'll remove the CREATE TEMP TABLE usage from the upgrade scripts.

[Jonas Borgström]

Missing DROP TABLE statements were added in r3459.