Thread: [SQLObject] Bug in foreign keys?
SQLObject is a Python ORM.
Brought to you by:
ianbicking,
phd
From: Martin B. <bl...@fu...> - 2005-12-13 20:33:48
|
Hi Not sure if I've stumbled upon a bug, I'm using the svn checkout from yesterday, and a foreign key declaration does not seem to generate the appropriate "references" code in the generated SQL (PostgreSQL). see attached files for a script to reproduce the problem as well as log out= put. |
From: Oleg B. <ph...@ma...> - 2005-12-13 21:05:55
|
On Tue, Dec 13, 2005 at 03:27:34PM -0500, Martin Blais wrote: > husband = ForeignKey('Man') husband = ForeignKey('Man', cascade=True) col.py: # cascade can be one of: # None: no constraint is generated # True: a CASCADE constraint is generated # False: a RESTRICT constraint is generated # 'null': a SET NULL trigger is generated Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Martin B. <bl...@fu...> - 2005-12-14 17:41:21
|
Excellent, thx. Another related problem: I specify the cascade parameter that you mention. When I trace the code, the code for the constraints (for PostgreSQL) is generated, but never execute.. It is returned from SQLObject.createTable() and never used (see variable extra_sql). =20 This is most likely a bug, or the constraint generation code should be removed. But the constraints seem to work nonetheless. For a while I was mesmerized by this, because if I rummage in the database itself I could not find the friggin constraint in the table schema, but after some more digging, I found that they are actually implemented by SQLObject itself, in SQLObject.destroySelf(). Any reason why we can't let the database do its own REFERENCES magic?=20 It's going to be much more efficient, because the way it's implemented now an extra select has to be done for each column dependent on a foreign key, and then an update/delete (depending on cascade=3DTrue or 'null'). On 12/13/05, Oleg Broytmann <ph...@ma...> wrote: > On Tue, Dec 13, 2005 at 03:27:34PM -0500, Martin Blais wrote: > > husband =3D ForeignKey('Man') > > husband =3D ForeignKey('Man', cascade=3DTrue) > > col.py: > # cascade can be one of: > # None: no constraint is generated > # True: a CASCADE constraint is generated > # False: a RESTRICT constraint is generated > # 'null': a SET NULL trigger is generated > > Oleg. > -- > Oleg Broytmann http://phd.pp.ru/ ph...@ph... > Programmers don't die, they just GOSUB without RETURN. > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through log fi= les > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! > http://ads.osdn.com/?ad_id=3D7637&alloc_id=3D16865&op=3Dclick > _______________________________________________ > sqlobject-discuss mailing list > sql...@li... > https://lists.sourceforge.net/lists/listinfo/sqlobject-discuss > |
From: Oleg B. <ph...@ph...> - 2005-12-14 18:16:14
|
On Wed, Dec 14, 2005 at 12:41:13PM -0500, Martin Blais wrote: > Another related problem: I specify the cascade parameter that you > mention. When I trace the code, the code for the constraints (for > PostgreSQL) is generated, but never execute.. It is returned from > SQLObject.createTable() and never used (see variable extra_sql). > This is most likely a bug, or the constraint generation code should be > removed. It is not a bug, it's a feature added not so long ago. Think one wants to create two tables that reference each other. It was impossible to do in SQLObject because SQLObject tried to create a table with all constrints... and databases return errors becuase the table to be created reference another table that hasn't been created yet. So .createTable() machinery was changed to create tabes without constrinas and return necceessary ALTER TABLES command which you should execute after table creation. Something like this: sql, constraints1 = Table1.createTableSQL() connection.query(sql) sql, constraints2 = Table2.createTableSQL() connection.query(sql) connection.query(constraints1) connection.query(constraints2) Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Martin B. <bl...@fu...> - 2005-12-14 21:40:55
|
On 12/14/05, Oleg Broytmann <ph...@ph...> wrote: > On Wed, Dec 14, 2005 at 12:41:13PM -0500, Martin Blais wrote: > > Another related problem: I specify the cascade parameter that you > > mention. When I trace the code, the code for the constraints (for > > PostgreSQL) is generated, but never execute.. It is returned from > > SQLObject.createTable() and never used (see variable extra_sql). > > This is most likely a bug, or the constraint generation code should be > > removed. > > It is not a bug, it's a feature added not so long ago. Think one wants > to create two tables that reference each other. It was impossible to do i= n > SQLObject because SQLObject tried to create a table with all constrints..= . > and databases return errors becuase the table to be created reference > another table that hasn't been created yet. > So .createTable() machinery was changed to create tabes without > constrinas and return necceessary ALTER TABLES command which you should > execute after table creation. Something like this: Thanks Oleg. Is anyone keeping track of these important API changes somewhere?=20 Because if no-one is, the task to port 0.6-based apps to 0.7 will be a perilous one. Both of the bugs/changes required from this thread do not generate warnings when upgrading. Why not make the default case nice (i.e. create the constraints by default), and letting the user prevent the constraints creation from being done when this is needed (e.g. with a keyword argument) ? BTW, the current approach is not going to work: you cannot trust that SQLObject itself can fitfully apply the constraints, it does not use a transaction for its home-made magic, and therefore exposes itself to race conditions in the code that applies the constraints. I think this is a latent bug. |
From: Oleg B. <ph...@ph...> - 2005-12-26 16:51:10
|
On Wed, Dec 14, 2005 at 04:40:52PM -0500, Martin Blais wrote: > On 12/14/05, Oleg Broytmann <ph...@ph...> wrote: > > Think one wants > > to create two tables that reference each other. It was impossible to do in > > SQLObject because SQLObject tried to create a table with all constrints... > > and databases return errors becuase the table to be created reference > > another table that hasn't been created yet. > > So .createTable() machinery was changed to create tabes without > > constrinas and return necceessary ALTER TABLES command which you should > > execute after table creation. Something like this: > > Is anyone keeping track of these important API changes somewhere? > Because if no-one is, the task to port 0.6-based apps to 0.7 will be a > perilous one. Both of the bugs/changes required from this thread do > not generate warnings when upgrading. > > Why not make the default case nice (i.e. create the constraints by > default), and letting the user prevent the constraints creation from > being done when this is needed (e.g. with a keyword argument) ? Well, I am ready to commit changes. SQLObject.createTable() got a new keyword, applyConstraints, True by default. So you do either MyTable1.createTable() MyTable2.createTable() or constrints = MyTable1.createTable(applyConstraints=False) constrints += MyTable2.createTable(applyConstraints=False) for constraint in constraints: conn.query(constraint) Is it ok? Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Martin B. <bl...@fu...> - 2005-12-27 23:41:25
|
On 12/26/05, Oleg Broytmann <ph...@ph...> wrote: > On Wed, Dec 14, 2005 at 04:40:52PM -0500, Martin Blais wrote: > > On 12/14/05, Oleg Broytmann <ph...@ph...> wrote: > > > Think one wants > > > to create two tables that reference each other. It was impossible to = do in > > > SQLObject because SQLObject tried to create a table with all constrin= ts... > > > and databases return errors becuase the table to be created reference > > > another table that hasn't been created yet. > > > So .createTable() machinery was changed to create tabes without > > > constrinas and return necceessary ALTER TABLES command which you shou= ld > > > execute after table creation. Something like this: > > > > Is anyone keeping track of these important API changes somewhere? > > Because if no-one is, the task to port 0.6-based apps to 0.7 will be a > > perilous one. Both of the bugs/changes required from this thread do > > not generate warnings when upgrading. > > > > Why not make the default case nice (i.e. create the constraints by > > default), and letting the user prevent the constraints creation from > > being done when this is needed (e.g. with a keyword argument) ? > > Well, I am ready to commit changes. SQLObject.createTable() got a new > keyword, applyConstraints, True by default. So you do either > > MyTable1.createTable() > MyTable2.createTable() > > or > > constrints =3D MyTable1.createTable(applyConstraints=3DFalse) > constrints +=3D MyTable2.createTable(applyConstraints=3DFalse) > > for constraint in constraints: > conn.query(constraint) > > Is it ok? Thanks for the work Oleg! :-) But for your question the answer is "yes and no". - "yes" it will do, in the sense that it will patch the problem. I have been rewriting my schemas "by hand" in the meantime, and since SQLObject allows binding to already existing tables, it allows me gradually remove it from my codebase, step-by-step, which is nice because my app can keep working during the transition, and I can write the constraints directly, with even more detail. - and "no" it suppose won't completely really be ok, in the sense that not using the special flag still opens up the possibiltiy for the race condition, and IMHO this needs to be either documented with ten-foot wide red-on-black blinking text somewhere, or maybe it should just not be permitted, but I guess that depends on how much "looseness" and coherency users are willing to accept when using an ORM. At least the default is to apply the constraints, that's a good choice IMO. A more important question for me is, how much "stuff" should SQLObject be allowed to do for the user? What an ORM adds on top of just easing access to SQL should be clearly delineated IMO and now I find that sqlobject does a bit too much magic behind-the-scenes stuff for my taste. It's like programming C++: if you don't have a very good idea what the compiler will do with your source, you can't really write efficient code, you just can't. Same with this: if I don't know exactly most of what happens when I use sqlobject objects, I'm bound to create terribly inefficient or buggy code-- my bugs, and the ORM's bugs. I used to be under the impression that SQLObject did a lot less that it does. cheers, |
From: Oleg B. <ph...@ph...> - 2005-12-30 20:21:28
|
On Tue, Dec 27, 2005 at 06:41:21PM -0500, Martin Blais wrote: > > MyTable1.createTable() > > MyTable2.createTable() > > > > or > > > > constrints = MyTable1.createTable(applyConstraints=False) > > constrints += MyTable2.createTable(applyConstraints=False) > > > > for constraint in constraints: > > conn.query(constraint) > > > Thanks for the work Oleg! :-) Commited at the revision 1443. > - and "no" it suppose won't completely really be ok, in the sense that > not using the special flag still opens up the possibiltiy for the race > condition Transactions, as you have suggested, wouldn't help - Data Definition Language statements are usually run in theier own transactions... Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ma...> - 2005-12-30 20:40:29
|
On Fri, Dec 30, 2005 at 11:20:54PM +0300, Oleg Broytmann wrote: > On Tue, Dec 27, 2005 at 06:41:21PM -0500, Martin Blais wrote: > > > MyTable1.createTable() > > > MyTable2.createTable() > > > > > > or > > > > > > constrints = MyTable1.createTable(applyConstraints=False) > > > constrints += MyTable2.createTable(applyConstraints=False) > > > > > > for constraint in constraints: > > > conn.query(constraint) > > > > > Thanks for the work Oleg! :-) > > Commited at the revision 1443. Documented at http://svn.colorstudy.com/SQLObject/docs/FAQ.txt in the section "Mutually referencing tables", including a short note about race conditions. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Martin B. <bl...@fu...> - 2005-12-30 22:48:09
|
On 12/30/05, Oleg Broytmann <ph...@ph...> wrote: > On Tue, Dec 27, 2005 at 06:41:21PM -0500, Martin Blais wrote: > > > MyTable1.createTable() > > > MyTable2.createTable() > > > > > > or > > > > > > constrints =3D MyTable1.createTable(applyConstraints=3DFalse) > > > constrints +=3D MyTable2.createTable(applyConstraints=3DFalse) > > > > > > for constraint in constraints: > > > conn.query(constraint) > > > > > Thanks for the work Oleg! :-) > > Commited at the revision 1443. > > > - and "no" it suppose won't completely really be ok, in the sense that > > not using the special flag still opens up the possibiltiy for the race > > condition > > Transactions, as you have suggested, wouldn't help - Data Definition > Language statements are usually run in theier own transactions... Oh, I'm not worried about the DDL statements at all, but rather about the cascades that SQLObject tries to apply by hand when you e.g. delete a row/object. Those currently don't run in transactions AFAIK. The problem is still there. cheers, |
From: Oleg B. <ph...@ph...> - 2005-12-30 23:24:05
|
On Fri, Dec 30, 2005 at 05:48:04PM -0500, Martin Blais wrote: > Oh, I'm not worried about the DDL statements at all, but rather about > the cascades that SQLObject tries to apply by hand when you e.g. > delete a row/object. Those currently don't run in transactions AFAIK. > The problem is still there. Ah, that race condition. I see now... Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2005-12-15 12:27:52
|
On Wed, Dec 14, 2005 at 04:40:52PM -0500, Martin Blais wrote: > BTW, the current approach is not going to work: you cannot trust that > SQLObject itself can fitfully apply the constraints, it does not use a > transaction for its home-made magic What is the connection between constraints and transactions? I beleive most DBMSes silently commit the current transaction before running any DDL statement (CREATE, ALTER TABLE) so you cannot rollback changes. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Martin B. <bl...@fu...> - 2005-12-15 23:48:35
|
On 12/15/05, Oleg Broytmann <ph...@ph...> wrote: > On Wed, Dec 14, 2005 at 04:40:52PM -0500, Martin Blais wrote: > > BTW, the current approach is not going to work: you cannot trust that > > SQLObject itself can fitfully apply the constraints, it does not use a > > transaction for its home-made magic > > What is the connection between constraints and transactions? I beleive > most DBMSes silently commit the current transaction before running any DD= L > statement (CREATE, ALTER TABLE) so you cannot rollback changes. I think that the constraints, when applied by the server itself, must be atomic. This is not the case with your "manual" application of constraints, i.e. you first do a query to apply the e.g. delete row, and then run other queries to apply the constraint. If you don't apply the constraints within a transaction (to insure the C in ACID), you run the chance that in between the delete and the constraint queries some other process visits the database and sees an incoherent version (i.e. not respecting the constraints). If your constraint queries were done within a transaction you do not run this risk. This is a serious bug, especially if you rely on the constraints (and you do, because if you don't why would you specify them...). |