You are viewing a plain text version of this content. The canonical link for it is here.
Posted to torque-dev@db.apache.org by Scott Eade <se...@backstagetech.com.au> on 2004/08/06 16:31:36 UTC
Re: submitted patch which allows left joins, right joins and explicit
inner joins
Thomas Fischer wrote:
>Hi,
>
>I have added functionality to Torque that allows one to do left joins,
>right joins (and explicit inner joins, in case anybody needs it). I posted
>the diffs on scarab, the ticket id is TRQS219.
>(http://nagoya.apache.org/scarab/issues/id/TRQS219)
>
Wow - great stuff.
>
>To create a left join, the following code has to be used:
>criteria.addJoin(AuthorPeer.AUTHOR_ID, BookPeer.AUTHOR_ID,
>Criteria.LEFT_JOIN);
>It is, of course, still possible to use
>criteria.addJoin(AuthorPeer.AUTHOR_ID, BookPeer.AUTHOR_ID);
>This is equivalent to
>criteria.addJoin(AuthorPeer.AUTHOR_ID, BookPeer.AUTHOR_ID, null);
>and produces a join in the as it was done up to now, i.e. by adding the
>join condition to the "where" clause of the query.
>I tested my changes on an Oracle9i database. I do not know whether the code
>runs on any other database (it produces SQL like "SELECT author.AUTHOR_ID,
>author.NAME FROM book RIGHT JOIN author ON book.AUTHOR_ID=author.AUTHOR_ID
>LEFT JOIN book b ON author.AUTHOR_ID=b.AUTHOR_ID").
>
>Note that I did NOT change the code in generator which generates code for
>selets with joins, e.g. BookPeer.doSelectJoinAuthor(). In my opinion, it
>would be nice to use a left join there to get also the books without an
>author, but I did not want to make too many changes at one time.
>
For backwards compatibility we would need to leave the existing methods
as is. There are I guess two ways forward, either we generate a second
set of methods that do left joins or we add an option to the generator
to determine if the existing or a left join should be used (I think the
latter might be the best option).
>
>I hope that I have made the changes such that fully qualified table names
>(database.schema.columnname) are possible, though I did not test it.
>
>Below, I summarize the changes which I made, then I give an example how the
>joins are handled internally, and then I discuss some special cases.
>
>Any comments or questions are welcome.
>
More comments below.
>
> Cheers
>
> Thomas
>
>
>Changes:
>
>added the necessary Constants (Join types and "ON") to SqlEnum and Criteria
>
>added the methods equals(Object) and hashCode() to SqlEnum. Thinking it
>through, this would not have been strictly necessary, because SqlEnum has
>no public constructor and only the predefined constants are used, so
>Object.equals() should give the same result as the new equals()-method.
>However, in my opinion adding the new method makes the code more robust and
>better readable, but this modification can be left out if the developers
>think it's not necessary.)
>
>Added a new method addJoin(String left, String right, SqlEnum joinType) to
>the Criteria class
>
>Added an inner class called Join to the Criteria class. A Join contains the
>left column, the right column and the join type. Replaced the Lists joinL
>and joinR by a new list joins which contains elements of type Join.
>
>Added an inner Class called FromElement to Query. A fromElement contains
>the table name in the FROM clause. It might also contain the join type and
>the join condition.
>
>Added a private helper function Criteria.getTablenameForFromClause(String
>tableOrAliasName, Criteria criteria) which returns either the tablename
>itself if tableOrAliasName is not an alias, or a String of the form
>"tableName tableOrAliasName" if tableOrAliasName is an alias for a table
>name.
>
>Added a public helper function getRawColumnName(String
>fullyQualifiedColumnName) which strips the database (and schema) names from
>columns. This is useful for working with aliases, as the constants for
>column names in the peer classes are fully qualified.
>
>Found that most code in BasePeer.createQuery(Criteria) was duplicated in
>BasePeer.createPreparedStatement(Criteria, StringBuffer, List). As I had to
>make changes in both methods anyway, I decided to split the code and put
>the parts which are (almost) equal in both methods into private the methods
>processSelectColumns(..), processAsColumns(..), processJoins(..),
>processModifiers(..), processCriterions(..) and processOrderBy(..). I am
>not sure why BasePeer.createQuery(..) processes the groupBy- and
>Having-Lists of Criteria while BasePeer.createPreparedStatement(..)
>doesn't, but I left it as it was, though I think it's a bit fishy. Also,
>the criteria.getLimit() and criteria.getOffset() are processed differently
>by BasePeer.createQuery(..) and BasePeer.createPreparedStatement(..). I
>could see no reason for this, but I left it also as it was.
>
I agree that it sounds fishy, I guess we need someone that uses prepared
statements to chime in and indicate whether or not they have problems
using group by or having clauses. In any case if you have retained the
existing behaviour you should not have added to the problem.
>
>Changed the treatment of select clauses in BasePeer.createQuery(..) and
>basePeer.createPreparedStatement(..). Both treatments are quite similar,
>except that the checks on column names differ: In normal statements, a
>column does not have to contain a "." to be valid, if it contains a "*".
>However, criteria.addSelectColumn("*") fails in Torque 3.1 with an
>StringIndexOutOfBoundsException. Does anybody know why this is so ? I did
>not really know what to do, so I changed the behaviour in
>basePeer.createPreparedStatement(..) so that it is like the behaviour in
>BasePeer.createQuery(..). Should not "*" be a valid "column name" to select
>from ???
>
I guess it is better to either add the desired columns specifically or
to let torque add all of the columns in the table - if you were to allow
"*", would the information necessary to pull the data out of the result
be available?
>
>
>Example illustrating the used algorithm:
>
>A Criteria should contain a single left join. The SQL snippet which is
>expected is
>table_a left join table_b on table_a.id = table_b.a_id
>
>First, the method criteria.addJoin("table_a.id", "table_b.a_id",
>Criteria.LEFT_JOIN) is called.
>In this function call, criteria.joins would be populated with a single
>element of type Join where the left column is table_a.id, the right column
>is table_b.a_id and the join type is SqlEnum.LEFT_JOIN.
>
>calling BasePeer.createQuery(criteria) would create a Query element where
>fromTables contains two elements of type FromElement. In the first
>FromElement, the table name is table_a and the join type and the join
>condition are both null. In the second FromElement, the table name is
>table_b, the join type is SqlEnum.LEFT_JOIN and the join condition is
>table_a.id = table_b.a_id
>
>In BasePeer.toString(), the first element of the fromTables would be
>appended at the appropriate place by calling its toString() method. Every
>other element is checked whether it contains a join type. If not, a comma
>is appended before the element itself is appended; if yes, a space is
>appended before the element itself is appended. This leads to the desired
>result.
>
>
>Special cases which are considered in the code:
>
>1) criteria.addJoin("table_a.id", "table_b.a_id") is called (corresponds to
>criteria.addJoin("table_a.id", "table_b.a_id", null) ).
>In this case, criteria.joins would still be populated with a single element
>of type Join where the left column is table_a.id and the right column is
>table_b.a_id. However, in contrast to the previous example, the join type
>is null. On calling BasePeer.createQuery(criteria), a query element would
>be created where the to fromElements contain only the two tablenames, and
>the join condition would be added to the where clause (and not to the
>second fromElement, as was done in the previous example).
>
>2) "Simple" multiple joins are considered.
>criteria.addJoin("table_a.id", "table_b.a_id", Criteria.LEFT_JOIN);
>criteria.addJoin("table_b.id", "table_c.b_id", Criteria.LEFT_JOIN);
>The sql should be
>table_a left join table_b on table_a.id = table_b.a_id left join table_c on
>table_b.id = table_c.second_a_id
>i.e. it must be taken into account that table_b is already "in use", it
>must not be added a second time to query.fromTables.
>This is implemented as follows: In the past, this behaviour was ensured by
>using a uniqueList in the fromTables. To retain that behaviour, one could
>define that two FromElements are equal if their tablenames are equal.
>However, this is not the "intuitive" behaviour of equals() which is
>expected from a FromElement, therefore this approach was not implemented.
>Instead, the method BasePeer.createQuery(criteria) checks "by hand" (i.e.
>by using the method BasePeer.fromClauseContainsTableName(..)) to see if a
>tablename already exists
>
>3) "More difficult" multiple joins : consider the following code snippet:
>criteria.addAlias("b", "table_b");
>criteria.addJoin("table_a.id", "table_b.a_id", Criteria.LEFT_JOIN);
>criteria.addJoin("b.second_a_id", "table_a.id", Criteria.RIGHT_JOIN);
>The expected SQL is NOT
>table_a left join table_b on table_a.id = table_b.a_id, table_b b right
>join table_a on b.second_a_id = table_a.id
>but is is instead
>table_a left join table_b on table_a.id = table_b.a_id left join table_b b
>on table_a.id = b.second_a_id
>i.e. if the second table name is already "in use", the join order and the
>join type should be "reversed". This is done in
>BasePeer.createQuery(criteria). (Note that this works only because joins
>are now processed before other code which may also add tables to the from
>clause). If both columns are already "in use", an exception is thrown.
>
>4) Tablenames can also be added to a criteria by adding columns from
>foreign tables explicity to criteria : Consider
>criteria.addSelectColumn("table_b.id");
>criteria.addJoin("table_a.id", "table_b.a_id", Criteria.LEFT_JOIN);
>as opposed to
>criteria.addSelectColumn("table_c.id");
>criteria.addJoin("table_a.id", "table_b.a_id", Criteria.LEFT_JOIN);
>It is ensured that in the first case, table_b is not added (again) to the
>from clause, while in the second case, table_c must be added to the from
>clause. (This was not an issue in Torque 3.1 because the table names were
>stored in an UniqueList, but now this has to be considered.)
>
>
>Special cases where I don't know whether they are treated correctly :
>
>1) I am not sure whether I have treated added Criterion's correctly
>(criteria.add(Criterion)). The tablenames for added Criterions are added
>like the colums from the select Columns (see 4), which is the treatment
>which they got in BasePeer.java in version 1.78 from the CVS. Therefore
>this should work with the old join type (join type null), but I have no
>idea if this treatment is ok for the join types LEFT_JOIN, RIGHT_JOIN and
>INNER_JOIN. This is mainly because I have no clue what
>criteria.add(Criterion) does.
>
Look at http://db.apache.org/torque/criteria-howto.html for an example
of Criterions in action. I would suggest that your patch should include
an update to this document (the underlying xdoc that is) to cover the
new functionality.
>
>If anybody can think of another special case which I did not consider,
>please tell me.
>
Can you please take a look at the coding standard found at
http://jakarta.apache.org/turbine/common/code-standards.html You need
to put your open braces on new lines and to keep an eye on line wrapping
(most common in torque would be to indent by 8 spaces for a broken line).
Other things you should include in your patch are updates to:
project.xml - add yourself as a contributor
changes.xml - mention the enhanced functionality
This is an extensive patch that is going to add significant new
functionality to torque. I would see HEAD as the appropriate place to
apply this patch (i.e. I do not see it going into 3.1.x).
I know it has taken an age for this response to appear. Please bear
with us. One or two patches on this scale and you will be welcomed with
open arms as a committer.
Scott
--
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: submitted patch which allows left joins, right joins and explicit
inner joins
Posted by T E Schmitz <ma...@numerixtechnology.de>.
Hello Thomas
>>It certainly is cleaner if SqlEnum remains non-public. However, if you
>>wanted to parameterize the join methods with Criteria.LEFT_JOIN, etc.
>>then the parameter would have to be an SqlEnum and hence have to be
> public.
>
> Sorry, I did not think thoroughly about it. Of course you are right,
> because what you want is not an instance of SqlEnum, but you want to
> declare a parameter of the type SQLEnum, which is quite a difference.
I also something else in the back of my mind - but I do not expect this
to be considered for Torque:
I decided to use the Joda-Time API (http://joda-time.sourceforge.net)
for DATE and TIME properties (this API is hugely more powerful than
java.util.Date and, most of all, it allows you to distinguish between
DateOnly and TimeOnly).
But as I am now using org.joda.time.DateOnly instead of java.util.Date
in the generated code I was forced to subclass Criteria and override all
methods which take a data object of some kind, check for DateOnly and
convert to Date. Some of the methods take an SqlEnum for a parameter and
if not for the joins I needed to make SqlEnum public for this exercise.
As I say, I don't expect this to be as a matter of general interest.
Joda-time is open source but not part of the Jakarta project anyway. But
the concept is certainly interesting.
> Forgive me ;-)
;-)
--
Regards/Gruß,
Tarlika
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: submitted patch which allows left joins, right joins and explicit inner
joins
Posted by Thomas Fischer <Fi...@seitenbau.net>.
Hi Tarlika,
T E Schmitz <ma...@numerixtechnology.de> schrieb am 10.08.2004 11:33:45:
> Hello Thomas,
>
> Thomas Fischer wrote:
>
> > T E Schmitz <ma...@numerixtechnology.de> schrieb am 06.08.2004
17:53:35:
> >
> >>Note that this requires SqlEnum to be public!
> >>
> >
> > Most of the SqlEnum constants are made available as public constants in
the
> > Criteria object (e.g. Criteria.LEFT_JOIN is a SqlEnum Object), so I do
not
> > see the reason why you had to make SqlEnum public ..... As one
> usually wants only one
> > access point to avoid confusion, the SqlEnum is not public. In my
opinion,
> > this is a correct decision.
>
> It certainly is cleaner if SqlEnum remains non-public. However, if you
> wanted to parameterize the join methods with Criteria.LEFT_JOIN, etc.
> then the parameter would have to be an SqlEnum and hence have to be
public.
Sorry, I did not think thoroughly about it. Of course you are right,
because what you want is not an instance of SqlEnum, but you want to
declare a parameter of the type SQLEnum, which is quite a difference.
Forgive me ;-)
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: submitted patch which allows left joins, right joins and explicit
inner joins
Posted by T E Schmitz <ma...@numerixtechnology.de>.
Hello Thomas,
Thomas Fischer wrote:
> T E Schmitz <ma...@numerixtechnology.de> schrieb am 06.08.2004 17:53:35:
>
>>>For backwards compatibility we would need to leave the existing methods
>>>as is. There are I guess two ways forward, either we generate a second
>>>set of methods that do left joins or we add an option to the generator
>>>to determine if the existing or a left join should be used (I think the
>>>latter might be the best option).
>>
>><code snippet>
>>
>
>
> I aggree with you that your suggestion is more "backwards compatible" than
> the generator option. However, in my opinion, the "standard" join for 99%
> of the cases will be a left join, so I would also prefer the generator
> option. If one really wants to have an inner join, one could emulate it via
> a Criteria object which restricts the FK column to "NOT NULL".
I agree - the left join is probably the standard option (that's why I
was so keen on it ;-) ) and doubling up all join methods would make the
templates a bit messy. As long as the user isn't restricted to one type
of join - now that inner and outer joins are possible! - that's the way
to go.
>
>>Note that this requires SqlEnum to be public!
>>
>
> Most of the SqlEnum constants are made available as public constants in the
> Criteria object (e.g. Criteria.LEFT_JOIN is a SqlEnum Object), so I do not
> see the reason why you had to make SqlEnum public ..... As one usually wants only one
> access point to avoid confusion, the SqlEnum is not public. In my opinion,
> this is a correct decision.
It certainly is cleaner if SqlEnum remains non-public. However, if you
wanted to parameterize the join methods with Criteria.LEFT_JOIN, etc.
then the parameter would have to be an SqlEnum and hence have to be public.
--
Regards/Gruß,
Tarlika Elisabeth Schmitz
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: submitted patch which allows left joins, right joins and explicit inner
joins
Posted by Thomas Fischer <Fi...@seitenbau.net>.
Hi,
T E Schmitz <ma...@numerixtechnology.de> schrieb am 06.08.2004 17:53:35:
> Hello,
>
>
>>> Note that I did NOT change the code in generator which generates code
for
>>> selets with joins, e.g. BookPeer.doSelectJoinAuthor(). In my opinion,
it
>>> would be nice to use a left join there to get also the books without an
>>> author, but I did not want to make too many changes at one time.
>>>
>> For backwards compatibility we would need to leave the existing methods
>> as is. There are I guess two ways forward, either we generate a second
>> set of methods that do left joins or we add an option to the generator
>> to determine if the existing or a left join should be used (I think the
>> latter might be the best option).
>
> If I could make a suggestion:
>
> I think the best thing would be to generate something like :
>
> /** Old method */
> protected static List doSelectJoin_XXX(
> Criteria criteria,
> Connection connection) throws TorqueException
> {
> return (doSelectJoin_BrandViaBrand1SkippedID(criteria,connection,null));
> }
>
> /** New method */
> protected static List doSelectJoin_XXX (
> Criteria criteria,
> Connection connection,
> SqlEnum joinType) throws TorqueException
>
I aggree with you that your suggestion is more "backwards compatible" than
the generator option. However, in my opinion, the "standard" join for 99%
of the cases will be a left join, so I would also prefer the generator
option. If one really wants to have an inner join, one could emulate it via
a Criteria object which restricts the FK column to "NOT NULL".
> Note that this requires SqlEnum to be public! Or is there any reason why
> you would not want SqlEnum to be public?
>
Most of the SqlEnum constants are made available as public constants in the
Criteria object (e.g. Criteria.LEFT_JOIN is a SqlEnum Object), so I do not
see the reason why you had to make SqlEnum public (I diod not have to make
this change). I guess the reason for not making SqlEnum public is the
following: People expect these constants in the Criteria object, so this is
the "default" access point to the constants. As one usually wants only one
access point to avoid confusion, the SqlEnum is not public. In my opinion,
this is a correct decision.
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: submitted patch which allows left joins, right joins and explicit
inner joins
Posted by T E Schmitz <ma...@numerixtechnology.de>.
Hello,
Scott Eade wrote:
>> I have added functionality to Torque that allows one to do left joins,
>> right joins (and explicit inner joins, in case anybody needs it). I
>> posted
> Wow - great stuff.
First of all, I am dead pleased, that you'll be taking Thomas' outer
join stuff on board. It'll be a huge boost to Torque's functionality.
>> Note that I did NOT change the code in generator which generates code for
>> selets with joins, e.g. BookPeer.doSelectJoinAuthor(). In my opinion, it
>> would be nice to use a left join there to get also the books without an
>> author, but I did not want to make too many changes at one time.
>>
> For backwards compatibility we would need to leave the existing methods
> as is. There are I guess two ways forward, either we generate a second
> set of methods that do left joins or we add an option to the generator
> to determine if the existing or a left join should be used (I think the
> latter might be the best option).
If I could make a suggestion:
I think the best thing would be to generate something like :
/** Old method */
protected static List doSelectJoin_XXX(
Criteria criteria,
Connection connection) throws TorqueException
{
return (doSelectJoin_BrandViaBrand1SkippedID(criteria,connection,null));
}
/** New method */
protected static List doSelectJoin_XXX (
Criteria criteria,
Connection connection,
SqlEnum joinType) throws TorqueException
Note that this requires SqlEnum to be public! Or is there any reason why
you would not want SqlEnum to be public?
As I am already using Thomas' patch, this was the one thing I had to
amend in Torque's runtime code in order to be able to generate
multi-purpose joins from the templates.
I have also changed my templates to cope with self-joins and joins with
multiple relationships to the same table. I have done this using aliases
for the join tables. The caller can query the alias name (if he wants to
build Criteria using columns from the FK tables).
Unfortunately, I am not sure whether my logic would work with multiple
column FKs (I don't use compound keys myself).
--
Regards/Gruß,
Tarlika Elisabeth Schmitz
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org
Re: submitted patch which allows left joins, right joins and explicit inner
joins
Posted by Thomas Fischer <Fi...@seitenbau.net>.
Scott,
thanks for looking through my lengthy mail. I will incorporate your
suggestions, but as I am really busy till the end of August at least, this
will probably take another month. I will then also adjust the patch to the
latest changes in the CVS HEAD.
More comments are inserted below.
Scott Eade <se...@backstagetech.com.au> schrieb am 06.08.2004 16:31:36:
> Thomas Fischer wrote:
>
> >Found that most code in BasePeer.createQuery(Criteria) was duplicated in
> >BasePeer.createPreparedStatement(Criteria, StringBuffer, List). As I had
to
> >make changes in both methods anyway, I decided to split the code and put
> >the parts which are (almost) equal in both methods into private the
methods
> >processSelectColumns(..), processAsColumns(..), processJoins(..),
> >processModifiers(..), processCriterions(..) and processOrderBy(..). I am
> >not sure why BasePeer.createQuery(..) processes the groupBy- and
> >Having-Lists of Criteria while BasePeer.createPreparedStatement(..)
> >doesn't, but I left it as it was, though I think it's a bit fishy. Also,
> >the criteria.getLimit() and criteria.getOffset() are processed
differently
> >by BasePeer.createQuery(..) and BasePeer.createPreparedStatement(..). I
> >could see no reason for this, but I left it also as it was.
> >
> I agree that it sounds fishy, I guess we need someone that uses prepared
> statements to chime in and indicate whether or not they have problems
> using group by or having clauses. In any case if you have retained the
> existing behaviour you should not have added to the problem.
>
>From the patch which you checked into the CVS two weks ago, I have an idea
why this is so. My guess is that most people don't use prepared statements,
and therefore the submitted patches also apply only for the "normal"
statements. I will check through the CVS history and see whether this is
true. Also, I will write some test cases for order by with prepared
statements and run them on oracle (unfortunalety, I do not have access to a
DB2 installation, so I cannot test it for DB2. If I remember correctly, the
biggest differences are for DB2). A problem with this is that some DB's do
not support prepared statements, so the tests should only be enabled for
the DB's which support prepared Statements. I know that Oracle and DB2 do
support prepared statements, and MySQL doesn't, but I do not know about
other DB's.
> >
> >Changed the treatment of select clauses in BasePeer.createQuery(..) and
> >basePeer.createPreparedStatement(..). Both treatments are quite similar,
> >except that the checks on column names differ: In normal statements, a
> >column does not have to contain a "." to be valid, if it contains a "*".
> >However, criteria.addSelectColumn("*") fails in Torque 3.1 with an
> >StringIndexOutOfBoundsException. Does anybody know why this is so ? I
did
> >not really know what to do, so I changed the behaviour in
> >basePeer.createPreparedStatement(..) so that it is like the behaviour in
> >BasePeer.createQuery(..). Should not "*" be a valid "column name" to
select
> >from ???
> >
> I guess it is better to either add the desired columns specifically or
> to let torque add all of the columns in the table - if you were to allow
> "*", would the information necessary to pull the data out of the result
> be available?
>
I agree that adding "*" to the desired columns could lead to a lot of
confusion, if it is done without thought. I never needed it, so I really
don't insist of enabling it. I also have no idea whether the column
information is needed at other places. I have no problem with leaving the
behaviour as it is.
> >
> >
> >Special cases where I don't know whether they are treated correctly :
> >
> >1) I am not sure whether I have treated added Criterion's correctly
> >(criteria.add(Criterion)). The tablenames for added Criterions are added
> >like the colums from the select Columns (see 4), which is the treatment
> >which they got in BasePeer.java in version 1.78 from the CVS. Therefore
> >this should work with the old join type (join type null), but I have no
> >idea if this treatment is ok for the join types LEFT_JOIN, RIGHT_JOIN
and
> >INNER_JOIN. This is mainly because I have no clue what
> >criteria.add(Criterion) does.
> >
> Look at http://db.apache.org/torque/criteria-howto.html for an example
> of Criterions in action. I would suggest that your patch should include
> an update to this document (the underlying xdoc that is) to cover the
> new functionality.
>
In the meantime, I have used a criterion with a left join, so this seems to
work ;-) . I will also update the docs illustrating how left joins are
used.
> >
> >If anybody can think of another special case which I did not consider,
> >please tell me.
> >
> Can you please take a look at the coding standard found at
> http://jakarta.apache.org/turbine/common/code-standards.html You need
> to put your open braces on new lines and to keep an eye on line wrapping
> (most common in torque would be to indent by 8 spaces for a broken line).
>
I didn't find any coding standards at the torque page, so i didn't know
which standards I should use. Perhaps a reference should be put into the
developer guide in the website. I will format my changes accordingly.
> Other things you should include in your patch are updates to:
> project.xml - add yourself as a contributor
> changes.xml - mention the enhanced functionality
Will do that as well.
>
> This is an extensive patch that is going to add significant new
> functionality to torque. I would see HEAD as the appropriate place to
> apply this patch (i.e. I do not see it going into 3.1.x).
I also do not see it as a small fix, so I agree to this.
>
> I know it has taken an age for this response to appear. Please bear
> with us. One or two patches on this scale and you will be welcomed with
> open arms as a committer.
>
As you already said, this was quite a large patch, so looking through it
will also have taken a lot of your time, so don't worry abouth the response
time.
> Scott
>
> --
> Scott Eade
> Backstage Technologies Pty. Ltd.
> http://www.backstagetech.com.au
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org