Menu
▾
▴
#71 fromDatabase and delColumn for sqlite
This patch implements both fromDatabase and delColumn
for sqlite. I added sqlite to the appropriate
existing tests; it passes them.
fromDatabase is done by parsing the table creation
sql from the sqlite_master table. This will work with
any sqlite 3, and I think dates back into sqlite 2,
but I'm not sure.
delColumn is implemented by renaming the table,
creating a new table without the one column, then
copying all the data over. This is obviously not as
efficient as on DBs that support drop column
directly, but it works.
This implementation of dropColumn does deal correctly
with triggers, foreign keys, and indices that
reference the table whose column is being dropped. It
does not deal correctly with triggers & indices
attached directly to the table being altered.
Attached items will be lost. Extending the
implementation to support re-creating those indices &
triggers should be straightforward, but is beyond my
needs. You are welcome to do so.
Finally, this patch slightly improves the
implementation of addColumn for sqlite. It performs a
vacuum operation after each column add. See the
sqlite documentation for why this is a good idea.
Logged In: YES
user_id=4799
Thank you very much! recreateTableWithoutColumn() is an
excellent solution!
I have some questions about the patch. Why have you added
semicolons? I think they are not needed.
Why VACUUM after addColumn()? It is more logical to do
VACUUM after delColumn(), I suppose. And, BTW, "This command
will fail if there is an active transaction." (from the
SQLite documentation).
Your patch deletes .delColumn(). But I have a number of
programs that relies on a presence of the method (even if
it's empty). I think you could just call
recreateTableWithoutColumn() from delColumn() or raise a
warning or an exception.
Logged In: YES
user_id=1027257
Why vacuum after addColumn:
According to SQLite's docs, alter table add column doesn't
add the column to the table's main location in the file,
but rather creates it at the end of the file and the DB
then takes care of pretending that the column adjoins all
the other columns.
Unfortunately, older versions of SQLite don't do this
pretending, so can't read a table that has had a column
added. Vacuum re-creates the table as a single table,
meaning that all versions can then read it.
Delete column, being not supported directly by SQLite,
doesn't do anything funny. Since my implementation is just
to create a new table without the column being dropped,
all the tables remain together without any weird extra
space, and a vacuum is not needed.
Why I added semicolons:
No, they are not needed, if you are using pysqlite.
However, they are required in the interactive SQLite
interpreter. I added the semicolons so that I could
quickly paste generated SQL statements into the
interactive console while I was trying to get things
working.
Since the semicolons have no effect with pysqlite, I just
left them in for the next time I wan't to do this. Feel
free to take them out if you'd like.
On the name of delColumn():
Feel free to name it delColumn. I gave the longer and
clear name to the low-level implementation primarily to
highlight the performance implications involved. The
public-facing objects still expose delColumn; that is
translated to a call to recreateTableWithoutColumn() if
the sqliteconnection is being used.
So, external programs use it as deleteColumn(), but my
patch is easier to understand and the performance
implications are clearer.
Mostly, I was hoping that that would speed patch
approval. ;) Feel free to name the function as you will.
On transactions:
Yeah, you can't vacuum in transactions. However, sqlite's
support for transactions is already fairly problematic. I
find I can't use transactions with SQLite on any real
problems anyway, so I don't worry about it. When I need
transactions, then I generally need a more full-featured
DBMS than SQLite.
If you have a different opinion on this, you could remove
the call to vacuum in addColumn(). You are trading off
read speed & the ability for older versions of sqlite to
read such a modified table. Yer call.
Logged In: YES
user_id=1027257
I've slightly update my previous patch. Feel free to use
whichever version you'd like - I just want to get these
new functions into the main code line as quickly as
possible. I need them in order to support migrations in
TurboGears.
This update does two things:
1. svn:ignore *.pyc in a bunch of directories.
2. Add sqlite to the -transactions list in the
supportsMatrix.
Why:
1: I find it easier to see what needs to get added to svn
if I don't have clutter on the various "modified files"
lists. Thus, I make sure to always tell svn how to handle
each file: ignore it or commit it. I find it reduces the
chances of bad checkins.
2: sqlite fails several tests that require transactions.
It really doesn't handle them well. This may be dues to my
adding vacuuming to addColumn, or it may be new failures
because I added sqlite to the list of DBs that support
fromDatabase() and delColumn. In any case, I found this to
be the fastest reasonable way to express what SQLite can
do. Feel free to modify if you want to go another way.
update: supports('transactions') return false for SQLite.
Logged In: YES
user_id=4799
The second patch is invalid because patch program doesn't
understand svn ignore lists. This must be done manually if
at all. I just run a script
find . -name '*.py[co]' | xargs rm
before commit.
And what are the problems with SQLIte and transactions? All
tests are now passed. Can you write a test script that
demonstrates the problem(s)?
Logged In: YES
user_id=4799
Originator: NO
A version extended by Glenn and me has been committed in the revision 2154. Thank you for the job!