Thread: [SQLObject] InheritableSQLObject.addColumn fix
SQLObject is a Python ORM.
Brought to you by:
ianbicking,
phd
From: Johan D. <jd...@as...> - 2006-08-15 19:27:05
Attachments:
inheritablesqlobject-addcolumn.diff
|
InheritableSQLObject.addColumn is not updating sqlmeta.columns, meaning that you cannot use sqlbuilder to access this column. FWIW here's the patch. Admittedly, I don't understand why childUpdate is set, but when it's set it prevents the class from chaining up which is necessary, or sqlmeta.columns will not be updated. Johan |
From: Oleg B. <ph...@ma...> - 2006-08-15 19:46:20
|
On Tue, Aug 15, 2006 at 04:26:35PM -0300, Johan Dahlin wrote: > FWIW here's the patch. Thank you. Can you add a test? Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Johan D. <jd...@as...> - 2006-08-15 19:48:43
|
Oleg Broytmann wrote: > On Tue, Aug 15, 2006 at 04:26:35PM -0300, Johan Dahlin wrote: >> FWIW here's the patch. > > Thank you. Can you add a test? A test is already included. Johan |
From: Oleg B. <ph...@ma...> - 2006-08-15 20:22:52
|
On Tue, Aug 15, 2006 at 04:47:58PM -0300, Johan Dahlin wrote: > Oleg Broytmann wrote: > > On Tue, Aug 15, 2006 at 04:26:35PM -0300, Johan Dahlin wrote: > >> FWIW here's the patch. > > > > Thank you. Can you add a test? > > A test is already included. Oops, sorry, I haven't realized that the first chunk is a test. I'll test it. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2006-10-06 17:05:48
|
Hello! On Tue, Aug 15, 2006 at 04:26:35PM -0300, Johan Dahlin wrote: > InheritableSQLObject.addColumn is not updating sqlmeta.columns, meaning that > you cannot use sqlbuilder to access this column. It should not update columns list. It only should update the q-magic reference to the existing column. > FWIW here's the patch. The patch is incorrect but it inspired me to dive into those waters deeper. I extended your test and found that addColumn() didn't do the q-magic and delColumn() didn't work at all. I fixed them both. Revisions 1988-1989. Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Johan D. <jd...@as...> - 2006-10-18 19:02:28
|
Oleg Broytmann wrote: > Hello! > > On Tue, Aug 15, 2006 at 04:26:35PM -0300, Johan Dahlin wrote: >> InheritableSQLObject.addColumn is not updating sqlmeta.columns, meaning that >> you cannot use sqlbuilder to access this column. > > It should not update columns list. It only should update the q-magic > reference to the existing column. > >> FWIW here's the patch. > > The patch is incorrect but it inspired me to dive into those waters > deeper. I extended your test and found that addColumn() didn't do the > q-magic and delColumn() didn't work at all. I fixed them both. Revisions > 1988-1989. Thanks Okay, this seems to work for the use cases I submitted. Recently I've begun to use selectBy quite a bit, which excepted the attribute to be in the column list. So, the following code is broken: Employee.selectBy(person_attribute=...) ForeignKey are also broken, neither obj or objID are queryable. I'll see what I can do to solve that on top of your patch. -- Johan Dahlin <jd...@as...> Async Open Source |
From: Johan D. <jd...@as...> - 2006-10-18 20:39:40
Attachments:
inheritence-bug.diff
|
> Okay, this seems to work for the use cases I submitted. > Recently I've begun to use selectBy quite a bit, which excepted the > attribute to be in the column list. > > So, the following code is broken: > > Employee.selectBy(person_attribute=...) > > ForeignKey are also broken, neither obj or objID are queryable. > > I'll see what I can do to solve that on top of your patch. Rather tricky and the code is not easy to follow, understand or modify. I testcase is attached to prove that I'm not dreaming. Any hints on how to solve this? Johan |
From: Johan D. <jd...@as...> - 2006-10-19 15:06:18
Attachments:
sqlobject-selectby-inherited.diff
|
Oleg Broytmann wrote: > On Wed, Oct 18, 2006 at 05:38:53PM -0300, Johan Dahlin wrote: >> Any hints on how to solve this? > > I need to look into it. You know, the code is tricky... Okay, I found out what needed to be done. selectBy does not use the .q magic so we need to build up the query manually. It turned out to be not so easy: SQLObject.selectBy uses DBAPI._SO_columnClause which cannot handle inheritence, it was not designed with that in mind. I had to copy over parts of _SO_columnClause into InheritedSQLObject and build up a query which does a join on the parent classes. Tested using postgres and sqlite. Johan |
From: Johan D. <jd...@as...> - 2006-10-19 18:58:26
Attachments:
sqlobject-selectby-inherited-v2.diff
|
Johan Dahlin wrote: > Oleg Broytmann wrote: >> On Wed, Oct 18, 2006 at 05:38:53PM -0300, Johan Dahlin wrote: >>> Any hints on how to solve this? >> I need to look into it. You know, the code is tricky... > > Okay, I found out what needed to be done. > > selectBy does not use the .q magic so we need to build up the query > manually. It turned out to be not so easy: > > SQLObject.selectBy uses DBAPI._SO_columnClause which cannot handle > inheritence, it was not designed with that in mind. > I had to copy over parts of _SO_columnClause into InheritedSQLObject and > build up a query which does a join on the parent classes. > > Tested using postgres and sqlite. I found a couple of problems when using more than two layers of inheritance, attaching an updated patch. Johan |
From: Johan D. <jd...@as...> - 2006-10-27 20:20:17
|
Oleg Broytmann wrote: > Hello! > > On Thu, Oct 19, 2006 at 03:57:44PM -0300, Johan Dahlin wrote: >> I found a couple of problems when using more than two layers of inheritance, >> attaching an updated patch. > > The test is good, but the patch is not. We do not need to copy > _SO_prepareSelectBy(), instead, InheritableSQLObject.selectBy() must handle > foreign keys itself. > Which it now does. See the commit 2039. Thank you for the inspiration > and the tests! Thanks, your code seems quite a bit smaller indeed! I forgot to include a test which tests 3 levels off inheritance that the following code was needed for: + tableName = cls.__name__ + for table in tables: + for c in [table.q.childName == tableName, + table.q.id == cls.q.id]: + clauses.append(conn.sqlrepr(c)) + tableName = table.__name__ Let's see if I can come up with a good test for that. Johan |
From: Oleg B. <ph...@ph...> - 2006-10-27 20:33:09
|
On Fri, Oct 27, 2006 at 05:09:37PM -0300, Johan Dahlin wrote: > Thanks, your code seems quite a bit smaller indeed! The point was not about size, but about correctness and maintainability. > I forgot to include a test which tests 3 levels off inheritance that the > following code was needed for: > > + tableName = cls.__name__ > + for table in tables: > + for c in [table.q.childName == tableName, > + table.q.id == cls.q.id]: > + clauses.append(conn.sqlrepr(c)) > + tableName = table.__name__ Where to apply it? And what for? Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2006-10-19 08:24:49
|
On Wed, Oct 18, 2006 at 05:38:53PM -0300, Johan Dahlin wrote: > Any hints on how to solve this? I need to look into it. You know, the code is tricky... Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |
From: Oleg B. <ph...@ph...> - 2006-10-27 15:31:20
|
Hello! On Thu, Oct 19, 2006 at 03:57:44PM -0300, Johan Dahlin wrote: > I found a couple of problems when using more than two layers of inheritance, > attaching an updated patch. The test is good, but the patch is not. We do not need to copy _SO_prepareSelectBy(), instead, InheritableSQLObject.selectBy() must handle foreign keys itself. Which it now does. See the commit 2039. Thank you for the inspiration and the tests! Oleg. -- Oleg Broytmann http://phd.pp.ru/ ph...@ph... Programmers don't die, they just GOSUB without RETURN. |