Re: Joins -- Details

Hello Anthony,

> 
> I have reread the code and thought about the problem and I really
> cannot see why there is a Join class separate from the SAlias class.
> 
> The data structures support a situation in which a given SAlias t can
> have more than one Join instance whose .toAlias = t. But this is not
> the case. As you say
> 
> >A SQuery has N SAlias.
> >A SQuery has N-1 Join.
> 
> The relationship is a simple tree, not many to many as supported by
> the data structures. This makes an already complex piece of code even
> more difficult to follow.
> 
> So unless I have missed something basic then please do merge these
> classes. I know that you have been thinking of Joins as being
> different from Aliases for some time, but I think you will find it all
> much clearer once they are unified. I cannot see how you need more
> than one list.
> 
> You already have a linked list structure. Each Join has a .fromAlias
> field that points to the parent. But .toAlias is redundant. No
> reinvention of data structures that I can see. 
> 
> This will also clear up confusion between addJoin, addTable etc. and
> reduce the code.

You are right.
So I tackled the SAlias / Join problem. I finally merged them and
renamed the resulting class to SRelation.

So now a SQuery has a list of SRelation. Each SRelation has 0 or 1
fromRelation (in fact only main table has no fromRelation).

> 
> Also, dealing with the main table differently from the join tables is
> messy, eg. in SDriver.selectSQL writing out the fields in the select
> list. Produces copied code. If we are going to have Joins/Aliases,
> then use them consistently.

Done.

> 
> Really, I do think that this needs to be done. I understand that it is
> not trivial, there are hooks everywhere. But please do it, at least in
> a fork.

Done. Thanks for insisting :)
> 
> ========
> 
> Minor notes:-
> 
> Some of the Renames are incomplete, eg. : setRsIndex, AddTable...

I tried to find them (but there might remain some).

> 
> There is copied code in SAlias constructor. call this().
> 
> I would prefer to rename the join() method innerJoin(), to be clear.

Done and deprecated join. Could be removed, but will break the existing
codebase.

> 
> findOrCreateFromResultSet.instanceMap seems to be a redundant data
> structure. Why not simply retrieve and store each join record one by
> one. A single outer loop.

Done (might have been a major performence hit before from what I see in
benchmarks tests !???).


I have also removed the Comparable interface from SRecordInstance.
You're right that instance need not to have a "natural" order.
The drawback is that a SRecordComparator based on primary keys is not
strickly consistent with equals, because two records with null key will
be the same to SRecordComparator (compare returns 0), while they are NOT
equal.
The advantage is that is removes cryptic counter (nullKeysIndex) from
SDataSet and SRecordInstance. I guess the advantage overweights the
drawback. (I added a warning in SRecordComparator's javadoc)

Regards,
Franck





------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/SimpleORM/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/SimpleORM/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:[email protected] 
    mailto:[email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/