Joins -- Details

Hello Franck

You have been busy! 

There are many things going on here.  I would generally prefer that we do one thing at a time, check it in, review and think, then move on.

I have split this response into two emails.  This email gives my initial impressions of the code, having only spent a couple of hours on it.

The second part then considers the philosophy of SimpleORM and whether implementing full joins is a good idea in the first place.  I present an alternative approach.

One thing that is certain is that if we do add this complexity to SimpleORM it has to be squeaky clean and very clearly documented.


TABLE, JOIN, SQUERY
================

In SQuery, I suspect that "Table" should be renamed to be "Alias" -- it does not actually represent a table.  (or maybe SAlias if you prefer to import the inner class directly into SDriver.)  

I do not understand the relationship between Table, SQuery, and Join.   Table and Join appear to be one-to one.  Except for the main table.  But then there is also a SQuery.mainAlias which I would think is redundant.  

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 that the language is a bit confused.  You have SFieldReferences which have a refing and a refed SRecord, always representing a many to one.  But then Join has a referingTable which may be a refing or a refed depending upon which way the reference goes.

I think that you need different words, eg. fromAlias and toAlias.
(Words are very important in complex code like this.)

(Do you really need to be able to one-to-many joins?  Aren't they just many-to-one joins back to front?  Would make the code simpler.)

addJoin, doJoin probably belong in the Join class just to structure the code a bit.

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.

JOIN API
=======

The order of the parameters .join("dept", "manager", Employee.DEPARTMENT) is a bit confusing.  dept-empee-dept.  Maybe
  .join("dept", Employee.DEPARTMENT, "manager")
Or
  .join("Employee.DEPARTMENT, "manager", dept")

There is also far too many constructors for SQuery.  Suggest take alias out and replace with setAlias.  I think also create setSelectMode and setQueryMode and deprecate the corresponding constructors.  (The set methods should return this.)

S ALIASED FIELD
=============

The first thing that I note is that SDriver.selectSQL still contains 
   for (SFieldScalar sfld : from.getSelectList()) 

I would think that once you introduce aliases you need to have them everywhere.  So if you retrieve employees and their managers, you would need to retrieve both employee.Name and manger.Name, say.    So we would need to create a SAliasedField class.  Ouch!


COMPARE/NULL KEYS INDEX
=====================

I do not understand why you needed these.  

I am concerned about inconsistent behaviors with null keys index as the keys are assigned.  It does not feel right at all.

Do you really need to sort on primary keys?  And if you do, what is wrong with just having the null keys sorted to the beginning in update order?  (The Java sorts should be stable, ie. preserve the order of records with keys of equal value.)

OTHER
=====

I would prefer "queryRecordsInDataSet" to be called something like "findAllRecords".  Later I would like to be able to apply SQueries to SDataSets when disconnected, so would prefer to reserve the word "query" for that.  

I have used SException.Error for user errors, can be helpful if we have just an exception and no stack trace.
Certainly I would be careful about reusing Java exceptions -- the following could be misleading:-
  ClassCastException("Only homogeneous records (same meta) can be comprared");

But certainly please always include extra information that could help a user, eg. the record instances that did not compare.  Everything has a toString so that should be easy.  Need not be pretty.

=======

Let's deal with the issues in this email, and then I'll have another careful read looking for bugs.

Thanks for all the work you have done,

Anthony


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, [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/