Re: Alias syntax

Hello Franck,

All looks good.  inner/left/general Join looks clean.

I don't understand SQuery.alias.  Seems redundant (and thus a little dangerous).  Why not just search through tables.tableAlias directly?

I like not having redundant aliases in FROM clauses.  My only current concern is the readability of SELECT XX_DEPARTMENT.DEPT_ID, XX_DEPARTMENT.DNAME...  Not important, but one little trick might be that if there is only one query table then suppress the XX_DEPARTMENT. prefix.  Remember that that is the common case.  Why does the first selectSQL method prepend table names given that it can only provide one table?

The relationship between the selectSQL methods and SQueryExecute is not as clean as it could be.  Why are WHERE clauses created one way, but JOIN clauses a different way?  But let's leave that alone for now.

Bumping up the version number sounds good.  Delete the commented out deprecated methods.  There is a big comment block on top of the first innerJoin() that should be dealt with.  Maybe reorder the methods in SQuery a bit, eg. group Join related ones?  Also need to update the README.html in the website area to note that you have made these changes.  Plus the white paper.

Once you have finished I was thinking of making SRecordInstance implement Map of field names (as strings) to field values.  This means that results of queries are Lists of Maps, very standard Java.  Means it can be reflected upon by JSP, JSF etc.  I also want SConnections be able to be created by connections as well as or instead of DataSets as had been requested.

Thanks,

Anthony


At 11:30 PM 26/03/2009, you wrote:

>Hello,
>
>Le jeudi 26 mars 2009 à  10:41 +1000, <mailto:anthony%40berglas.org>[email protected] a écrit :
>
>> 
>> I really like .as(). This (finally) makes it clear. Well done. (Maybe
>> we could do the same for selectMode.)
>
>I have done it. I added two select() method, one for SSelectMode, the
>other for SFiedlScalar[]. Both apply to last added table.
>
>Then I removed all construtors and method that where using selectList or
>selectMode.
>So now you do :
>new SQuery<Department>(Department.meta).select(SDESCRIPTIVE).as("dept")
>.innerJoin("dept", Employee.DEPARTMENT).select(Employee.NAME);
>
>> 
>> I think that a tables ArrayList might be better than the Map. This
>> means you can change the alias more easily. Given that the number of
>> joined tables is small (<5, normally 1) this would probably be much
>> more efficient anyway. 
>
>Replaced the map with a list. I also added a List for the aliases, maybe
>I could iterate over the table list instead when needed (as there will
>only be a few tables), but on the other hand, ArrayList<String> is not
>expansive either. Not sure.
>> 
>> I really do not like mustQuote. I would prefer that if the alias is
>> the same as the table name then it simply not be listed in the FROM
>> clause. Otherwise we are asking for database incompatibility issues.
>> (And I think that it is OK to insist that people use aliases that do
>> not need quoting!) (Using table without alias is common, eg. SECECT
>> FOO.BAR FROM FOO WHERE...)
>
>Removed mustQuote. So now either you give an alias, and then it must not
>contains any special characters, or tableName is used, an then there is
>not aliased at all.
>So we now have :
>
>select FOO.BAR FROM FOO; // no alias given
>or
>select F.BAR FROM FOO F; // F alias given
>or
>select "FO$O".BAR FROM "FO$O" // no alias and quoted indentifier
>requested by user
>
>> 
>> I also do not like checkAlias. If we are going to do this sort of
>> thing we should do it for column and table names first. But I'm happy
>> to let the database engine raise the error rather getting into the
>> business of figuring out what identifiers are legal for any particular
>> database.
>
>I commented it out.
>> 
>> I like generated aliases because they make the generated queries much
>> easier to read. How about using the first and last letters of the
>> table name followed by the number of times *that* alias has already
>> bean used +1. This makes them stable almost all of the time. And then
>> we do need a getDefaultAliasFor(SRecordMeta) to provide an alias for
>> the eq() etc. operators. A few extra lines of code but I think worth
>> it given we have got to here. (Always appending a number avoids
>> generating keyword aliases like "AS".)
>
>Well, I went the way where we don't give any alias unless the user want
>one. So the need for generated aliases seems gone.
>> 
>> You probably need to have a generalJoin() method that includes
>> JoinType as a parameter to cover the other cases. Then the other *join
>> methods could call it, reducing a little bit of copied code.
>
>Done. (also removed all join methods that were taking selectMode).
>> 
>> Good to run code coverage. 65% is good. If it highlights areas that
>> should be tested then make a list. (I don't care about exceptions and
>> debug trace, convenience polymorphisms, generated get/set etc.)
>> 
>> (Some people insist on 100%, but they then write tests just to satisfy
>> the statistic, which in turn makes the statistic meaningless. It also
>> discourages internal consistency checks which is bad.) Improving our
>> tests does not necessarily mean improving the statistic, it means
>> covering more combinations of real world cases.
>
>I agree with you, I was happy to see the level of coverage (I wish our
>internal project had this coverage level :)
>
>The latest changes broke the API a bit. Maybe it would be time for a
>version bump ? (3.5 ?)
>
>Regards,
>Franck
>> 
>> Regards,
>> 
>> Anthony
>> 
>> At 10:06 PM 25/03/2009, you wrote:
>> 
>> >Generating aliases is definitely a bad idea, as when we use the
>> >alias-less methods for eq(), gt(), etc. we would have to tell back
>> what
>> >alias has been assigned to the table we are dealing with... this
>> would
>> >complicate things.
>> >
>> >So I implemented the as(alias) method, that apply on last added
>> table.
>> >The only caveat is if you want to use the same table several times in
>> a
>> >query, you must give an alias each time you use this table. No a big
>> >deal I think...
>> >
>> >I also removed all depracated methods from SQuery. This breaks
>> existing
>> >code...
>> >
>> >As usual, we can always revert if necessary.
>> >
>> >Franck
>> >
>> >Le mercredi 25 mars 2009 à 12:25 +0100, Franck Routier a écrit :
>> >> One caveat is that then the table will first be added with its
>> default
>> >> alias, which might be used already...
>> >> So maybe we should really generate alias as firstletter
>> >> +tables.size()...
>> >> 
>> >> Le mercredi 25 mars 2009 à 12:09 +0100, Franck Routier a écrit :
>> >> > Hi,
>> >> > 
>> >> > I like the setAlias() solution, that changes the alias for the
>> last
>> >> > added table.
>> >> > 
>> >> > I also suggest to call it simply as().
>> >> > 
>> >> > SQuery<Employee> subQ2 = new
>> >> > SQuery<Employee>(Employee.EMPLOYEE).as("emp")
>> >> > .join("emp", Employee.MANAGER, SSelectMode.SNONE).as("manager")
>> >> > .join("manager", Employee.DEPARTMENT)
>> >> > .eq(Department.MISSION, "Count Pennies")
>> >> > .descending("emp", Employee.SALARY);
>> >> > 
>> >> > What do you think of this ?
>> >> > 
>> >> > 
>> >> > Le mercredi 25 mars 2009 Ã 20:55 +1000, <mailto:anthony%
>> 40berglas.org><mailto:anthony%40berglas.org>[email protected] a
>> >> écrit :
>> >> > > Hello Franck,
>> >> > > 
>> >> > > I was just updating the white paper with
>> >> > > 
>> >> > > SQuery<Employee> subQ2 = new
>> >> > > SQuery<Employee>(Employee.EMPLOYEE).setAlias("emp")
>> >> > > .join("manager", "emp", Employee.MANAGER, SSelectMode.SNONE)
>> >> > > .join("dept", "manager", Employee.DEPARTMENT)
>> >> > > .eq("dept", Department.MISSION, "Count Pennies")
>> >> > > .descending("emp", Employee.SALARY);
>> >> > > 
>> >> > > I note that we should not really need to specify the "dept"
>> alias
>> >> at
>> >> > > all. But it cannot be removed from the second join statement
>> >> because
>> >> > > manager needs to be specified. If the parameter order did not
>> have
>> >> two
>> >> > > strings next to each other that could be achieved. 
>> >> > > 
>> >> > > I am wondering whether
>> >> > > .join("dept", Employee.DEPARTMENT, "manager")
>> >> > > is clearer. And thus
>> >> > > .join(Employee.DEPARTMENT, "manager")
>> >> > > Not sure. 
>> >> > > 
>> >> > > But then we end up with far to many polymorphic methods -- I
>> wish
>> >> Java
>> >> > > had keyword optional arguments!
>> >> > > (Also, the join parameter names do not make it clear which is
>> >> which.
>> >> > > Maybe newAlias and previousAlias would be better?)
>> >> > > 
>> >> > > Maybe
>> >> > > .join(...).setSelectMode(SNONE)
>> >> > > ie. set* affects the previous SQueryTable (including the main
>> one
>> >> if
>> >> > > at the beginning).
>> >> > > Thus maybe setAlias(), setJoinedAlias()?
>> >> > > 
>> >> > > (Should replace .join with innerJoin everywhere.)
>> >> > > 
>> >> > > (I'd like a good example for the white paper in the test cases,
>> >> > > include leftJoin.)
>> >> > > 
>> >> > > Anthony
>> >> > > 
>> >> > > Dr Anthony Berglas, <mailto:anthony%
>> 40berglas.org><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, <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/