Re: Joins -- Details

Hello Franck,

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.

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.

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.

========

Minor notes:-

Some of the Renames are incomplete, eg. : setRsIndex, AddTable...

There is copied code in SAlias constructor.  call this().

I would prefer to rename the join() method innerJoin(), to be clear.

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.

I have also generally preferred package scope to private so that third parties can hack without actually changing our code.

But please address Join/Alias.  Then I'll have another look.  

(I really do not have time for this at the moment, but it is important that if we put this in that we put it in as cleanly as we can.)

Regards,

Anthony


At 12:51 AM 24/03/2009, you wrote:

>> 
>> I strongly suspect that you only need one inner class, "Alias". An
>> instance is created for the top level table, but with its reference
>> left null. Instances are then created for each joined table. Related
>> fields are then removed from SQuery.
>> 
>I think going this way will make things much more complex, as we will
>end up with a linkedlist strucutre. Traveling this structure will
>requiere than we reinvent the wheel of java collection API. Having a
>java.util.List a SAlias, and a java.util.List of joins (the relations
>betwen them) seems simpler to me.
>
>> More internal documentation on SQuery as to what the data structures
>> mean would be helpful. Maybe an example query and then what goes
>> where. 
>> 
>> I don't understand what currentRsindex means. Should be using a
>> LinkedHashMap for "table"? Can you handle multi column keys? What does
>> "RS" stand for? Looks dubious.
>
>currentRsIndex is an instance field of SQuery that holds the current
>number of fields that are to be retrieved when the query will be
>executed. Each SAlias will have its own firstFieldIndex when created,
>computed from currentRsIndex. This is used later to find the fields we
>want to retrieve from the resultSet. (what we where doing before was
>iterate over every field of the select list for each record we wanted to
>retrieve : this solution was quite ineffective, and not suited for table
>that were added multiple times.
>> 
>> JOIN API
>> =======
>> 
>> The order of the parameters .join("dept", "manager",
>> Employee.DEPARTMENT) is a bit confusing. dept-empee-dept.
>
>Well, it is really 'join( to "dept" , from "manager" , using "a
>reference than makes the link between dept and manager" and that happens
>to belong to manager)'
>
>> Maybe
>> .join("dept", Employee.DEPARTMENT, "manager")
>> Or
>> .join("Employee.DEPARTMENT, "manager", dept")
>> 
>
>Also consider english is not my mother tongue, so parameters order is a
>bit less natural to me (in french we tend to have another order than in
>english for the words in the sentence...). So I really don't know :)
>
>
>> 
>
>> At 12:51 AM 21/03/2009, you wrote:
>> >Hi Anthony,
>> >
>> >I've come up with a working implementation of "complex" joins in
>> >simpleorm that can do the following:
>> >
>> >- aliasing tables
>> >- joining the same table multiple times
>> >- left joins
>> >- one to many joins
>> >- join on joined tables (not only on the main table of the query)
>> >
>> >I have added tests, that work on hsql, postgresql and oracle.
>> >
>> >There is still a point that bothers me about semantics...
>> >
>> >Right now, ses.query returns a List, which is actually a Set that
>> >retains the order. A LinkedHashSet could have been more accurate, but
>> >it's an implementation, not an interface.
>> >
>> >That said, having many to one joins opens the door for requests that
>> >will return several time the same instance of the main table of the
>> >query, so the question is, should we :
>> >
>> >1) return a List with several times the same record ?
>> >2) return a List which would really be a set retaining the order of
>> >insertion (but then an order by on a joined table would be
>> problematic,
>> >as if we have R1,R2,R1,R3, the set would be R1,R2,R3) ?
>> >3) should we change the API :-( to return a LinkedHashSet ?
>> >4) should we add a method querySet and offer the alternative (either
>> a
>> >List with potentially several time the same record and true order by,
>> or
>> >a set with semi-order by)...
>> >
>> >Right now I have opted for 2), but I'm not sure.
>> >What is your opinion on this ?
>> >
>> >Franck
>> 
>> Dr Anthony Berglas, <mailto:anthony%40berglas.org>[email protected] Mobile: +61 4 4838 8874
>> Just because it is possible to push twigs along the ground with ones
>> nose
>> does not necessarily mean that is the best way to collect firewood.
>> 
>> 
>> 
>> 
>> 

Dr Anthony Berglas, [email protected]       Mobile: +61 4 4838 8874
Just because it is possible to push twigs along the ground with ones nose
does not necessarily mean that is the best way to collect firewood.



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

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/