Re: [SQLObject] A small patch around features I need

[Ian B. <ia...@co...>]

On Fri, 2003-06-27 at 11:42, Fran=E7ois Girault wrote:
> Clarisys patch for SQLOBject 0.3

There's several things that have changed since 0.3 -- it would be
helpful to do this against CVS instead.

>  * id can now be passed at creation time when using db sequence is not =
appropriate
>=20
>     myNewEntity =3D Entity.new(id=3D25)
>=20
>     if Entity._idName is defined, you can also use this value as key to=
 specify the id :
>=20
>     class Entity(SQLObject):
>         _idName =3D 'code'
>         ...
>     myNewEntity =3D Entity.new(code=3D25) will work the same as previou=
s declaration
>=20
>  * object ids can be alphanumerical type. Note that you MUST specify
> id at creation time for the moment. Support for stored procedure to
> generate alphanumerical ids will come in a close future ;-)=20

This seems fine -- I suspected the problem with non-integer ID's was
mostly the occasional %i, which is most of what you've changed.

It should be easy enough to add stored procedures, at the same time
allowing differently-named sequences, or the current technique (that
uses .lastrowid, I think, and in Postgres would actually work for most
cases).

Note that primary keys are still considered immutable -- this assumption
exists in subtle ways in different parts of SQLObject, and the
assumption is probably propagated to most people's applications.

>  * relations between objects can use other fields than id (including
> alphanumerical ones). In _join class attribute you can add these
> parameter to Join children creation :
>=20
>     * joinField let you specify the field to join within your class
>=20
>     * otherField let you specify the field of the other table
>=20
>    Note that the specified fields MUST be declared as alternateID

That looks fine too, though I'm also planning to change (probably for
0.5) how the joins work, so that they create better SQL and create
smarter objects than just lists.

>   * for Postgresql usage only due to modification in DBConnection, and
> I have no time to write & test with other DB (help welcome :) ).

It's very helpful if you include a unit test for any new features --
this is more important to me than supporting all the databases.


A couple notes on the code itself:

-        func =3D eval('lambda self: self._SO_joinList[%i].performJoin(se=
lf)' % index)
+        func =3D eval('lambda self: self._SO_joinList[%s].performJoin(se=
lf)' % index)

That %i isn't related to the ID.

(in .new()):
+       inst._inst_id =3D None
+       if kw.has_key('id'):
+           inst._inst_id =3D kw['id']
+           del kw['id']
+       else:
+           if kw.has_key(inst._idName):
+               inst._inst_id =3D kw[inst._idName]
+               del kw[inst._idName]

I think you're allowing people to use the database name of the column
here, even though that's not allowed for any of the other columns.  I
don't think that should be allowed, nor should it be generalized to the
rest of the columns -- it just creates ambiguity in your class usage.



Anyway, I'd accept it except the _inst_id stuff, but I'd really like it
to include a unit test (and a test which the current code wouldn't pass,
of course :).  It would be helpful if you can also redo it against CVS.=20
But the unit test is more important.  I am repeating that for emphasis
;)

  Ian