created torque 4 branch with village removed

[Thomas Fox <Th...@seitenbau.net>]

As you might have noticed, there is a new branch in torque4 which has
village removed. This branch passes the Torque test project for mysql,
postgresql, oracle, derby and mssql, ecept one failuing tests with the
managers which I have not yet looked into.

The functionality which village provided was replaced by the following
strategy:

- The class which converts a result set into a java object is called
recordMapper. It is a new interface in a new package,
org.apache.torque.om.mapper, and has a few generic implementations in the
same package as well as one generated for each table as inner class in the
peer class. so what doSelect does is now construct the query, get the
result set, walk through it and applying the recordMapper to each row to
get the result list.

- The doInsert and doSelect methods convert a new class called
org.apache.torque.util.ColumnValues which is basically a map from column
values to an ew class called org.apache.torque.util.JdbcTypedValues shich
contains a value and a SQL type (ColumnValues additionally contains a table
map). ColumnValues replaces the Criteria used for inserts and updates; this
has become necessary because at least Derby needs the JDBC Types when
setting null values in prepared statements, and we do not want to query the
resultSetMetadata as village did. So what happens for inserts and updates,
the Peers construct a ColumnValues object from tha database object and pass
this to the BasePeer's doInsert and doUpdate methods, which then construct
a prepared statement from the information, insert the given values and
execute the prepared statement.

Of course this has led to quite a few changes in the APIs, in particular
all methods which return village values or village records are gone
(naturally).
The following  methods have changed signature:
- Base.Peer.getIdAsVillageValue in the AutoIncrementIdBroker and the
SequenceIdBroker
- Base.Peer.doInsert (Criteria -> ColumnValues)
- Base.Peer.doUpdate(Criteria -> ColumnValues)
- LargeSelect Constructor now aditiionally need a RecordMapper

The following methods were removed, renamed or moved
- doPsSelect (need to figure out hwether all selects can be done using
prepared statements)
- doSelectVillageRecords is now called doSelect and needs a RecordMapper
and a TableMap as additional inputs.
- BasePeer.getPrimaryKey was moved to TableMap
- BasePeer.executeQuery is now Called doSelect and needs a RecordMapper and
a TableMap as additional inputs.
- BasePeer.getSelectResults is removed without Replacement (one-step
mapping instead of two-step mapping)
- BasePeer.initTableColumns is removed (no village schema any more)
- BasePeer.insertOrUpdateRecord (no recordy any more)
- BasePeer.processTables (was needed for deletes and was handling village
records, not needed any more)
- LargeSelectonstructors which would have retuned a record list are removed
- generated Peer's row2object, populateObject and populateObjects method
(their job is now done by the RecordMappers)
- generated Peer's doSelectVillageRecords (replaced by doSelect methods)
- generated Peers getOmClass(record)
- DataTets's testNullConnection and testPreparedStatements are gone for now
(former needs to be rewritten)
- SqlToAppDataTest was removed (was useless before as Sql2AppData is now
tested in generator)

The return types of some Methods ofSummaryHelper now depend on the database
(but additional methodw were added to fix this across databases)
And finally, the ProcessCallback interface in BasePeer was removed (it was
handling village records)

I tried to keep focus on the remove village task, but while I was at it I
could not resist to do some additional changes
- methods which take a connection as argument now fail when null is given
as an argument. This was discussed to be teh expected behaviour on the dev
list some year ago, and the reason is that if you pass a connnection you
expect to be in a transaction (otherwise you would not bother). if you are
not in a transaction then you want to fail fast because this mioght be
difficult to track otherwise.
- renamed a few template veriables (because the table element now has an
attribute field, the column template attributes should have a different
name)
- db object instances are now created using a constructor

Although this list of changes may seem long, in my opinion most of these
things were not parts of torque which were used outside. As Proof to this
staement, check how few things needed to be changed in the test project.

Of course there are a few things left to do (which I will hopefully tackle
in the next fes weeks):
- check whether to revive the doPsSelect methods or whether all selects can
use prepared statements
- there was come code in doDelete which handled cascading deletes (at leats
theoretically, never saw it working) tris needs to be reimplemented
- Maybe one can clean up LargeSelect from the reflection stuff therein
- the builldCriteria method in generated peers does not make sense anymore
(replaced by buildColumnValues) and should be removed
- the null-failure if a connection is passed as null needs to be consistent
throughout Torque
- rewrite testNullConnection method (check passing a nul connection fails)

I'd appreciate any feedback. Of course my idea is to move this back into
trunk, but I wanted to give everybody opportunity to check this.

And, one last thing, as of my marriage two weeks ago my last name has
changed from Fischer to Fox, so do not be surprised by the new email
address.

    Cheers,

      Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

Re: created torque 4 branch with village removed

[Thomas Fox <Th...@seitenbau.net>]

> Of course there are a few things left to do (which I will hopefully
tackle
> in the next few weeks):
> ...
> - the builldCriteria method in generated peers does not make sense
anymore
> (replaced by buildColumnValues) and should be removed

I was wrong here, the method is needed by the managers. Restored as neeeded
by the managers.

> - the null-failure if a connection is passed as null needs to be
consistent
> throughout Torque

Checked that. Looks good. Also created a jira issue to document this.

> - rewrite testNullConnection method (check passing a nul connection
fails)

Done.

    Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

RE: created torque 4 branch with village removed

[Thomas Fox <Th...@seitenbau.net>]

> Question:  Can we still do generic record queries or is this limited
> to only Torque table records?  E.g., generate "Select t1.col1,
> t2.col1, ... from t1, t2 where..." using Criteria, then call
> something similar to BasePeer.doSelect(c) and process the results.
> I know this has been a common answer to "Can torque do this..."
> queries in the past.

Yes, one can. There is the org.apache.torque.om.mapper.ObjectListMapper
which will return a List of object lists. Either one can tell the Mapper
which classes should be used for the columns (database independent), or one
can let the db driver decide if one does not care about database
independence. See e.g. the
org.apache.torque.util.SummaryHelperTest.testSummarize() method in the test
project for an example.

   Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

RE: created torque 4 branch with village removed

[Greg Monroe <Gr...@dukece.com>]

Great work

Re: Prepared statements..

+1 on making all calls via prepared. As said, helps with SQL injection and probably improves performance for most use cases.  One possible headache might be the CUSTOM.SQL 

Question:  Can we still do generic record queries or is this limited to only Torque table records?  E.g., generate "Select t1.col1, t2.col1, ... from t1, t2 where..." using Criteria, then call something similar to BasePeer.doSelect(c) and process the results.  I know this has been a common answer to "Can torque do this..." queries in the past.

> -----Original Message-----
> From: Thomas Fox [mailto:Thomas.Fox@seitenbau.net]
> Sent: Sunday, October 31, 2010 5:22 PM
> To: Apache Torque Developers List
> Subject: Re: created torque 4 branch with village removed
> 
> 
> > >> - check whether to revive the doPsSelect methods or whether all
> selects
> > > can
> > >> use prepared statements
> > >
> > > Re-added BasePeer.doPSSelect() for now.
> >
> > This method has some weaknesses IIRC. It dos not work correctly with
> > selects containing joins. I'd prefer to use prepared statements for all
> > selects if at all possible and I don't think we need separate methods.
> 
> So would I. Much less room for SQL-injection attacks. Will take a bit of
> work, but if nobody objects, I'll have a go at it.
> 
> > >> - there was come code in doDelete which handled cascading deletes
> (at
> > > leats
> > >> theoretically, never saw it working) tris needs to be reimplemented
> > >
> > > Not re-added. The code can definitely not have worked for composite
> foreign
> > > keys because there is no way to tell a composite foreign key from the
> > > database map.
> >
> > Still I'm pretty sure that deletes containing joins do not work
> > correctly. So there should be another solution some day.
> 
> What do you mean by "deletes containing joins"? A delete which where
> clause
> contains a join or a delete which deletes from multiple tables? I'm
> pretty
> sure the former works now as the table name is now specified in the
> interface (no magic table-to-delete-from detection from the criteria any
> more); not sure whether the latter can work in SQL.
> 
> Would you mind to create a simple test case to show what you mean ?
> 
> > >> - Maybe one can clean up LargeSelect from the reflection stuff
> therein
> > >
> > > Not while the Peers have static methods.
> > >> ...
> >
> > I beg to differ. LargeSelect just tries to call the methods
> > addSelectColumns() and populateObject() on the given builder class. You
> > can provide any class you want here. I'd prefer to use some kind of
> > Builder-interface for this, however.
> 
> Agreed. But does one not still need to have non-static methods for
> implementing an interface.
> 
>    Thomas
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: torque-dev-help@db.apache.org

DukeCE Privacy Statement:
Please be advised that this e-mail and any files transmitted with
it are confidential communication or may otherwise be privileged or
confidential and are intended solely for the individual or entity
to whom they are addressed. If you are not the intended recipient
you may not rely on the contents of this email or any attachments,
and we ask that you please not read, copy or retransmit this
communication, but reply to the sender and destroy the email, its
contents, and all copies thereof immediately. Any unauthorized
dissemination, distribution or copying of this communication is
strictly prohibited.

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

Re: created torque 4 branch with village removed

[Thomas Fox <Th...@seitenbau.net>]

> >> - check whether to revive the doPsSelect methods or whether all
selects
> > can
> >> use prepared statements
> >
> > Re-added BasePeer.doPSSelect() for now.
>
> This method has some weaknesses IIRC. It dos not work correctly with
> selects containing joins. I'd prefer to use prepared statements for all
> selects if at all possible and I don't think we need separate methods.

So would I. Much less room for SQL-injection attacks. Will take a bit of
work, but if nobody objects, I'll have a go at it.

> >> - there was come code in doDelete which handled cascading deletes (at
> > leats
> >> theoretically, never saw it working) tris needs to be reimplemented
> >
> > Not re-added. The code can definitely not have worked for composite
foreign
> > keys because there is no way to tell a composite foreign key from the
> > database map.
>
> Still I'm pretty sure that deletes containing joins do not work
> correctly. So there should be another solution some day.

What do you mean by "deletes containing joins"? A delete which where clause
contains a join or a delete which deletes from multiple tables? I'm pretty
sure the former works now as the table name is now specified in the
interface (no magic table-to-delete-from detection from the criteria any
more); not sure whether the latter can work in SQL.

Would you mind to create a simple test case to show what you mean ?

> >> - Maybe one can clean up LargeSelect from the reflection stuff therein
> >
> > Not while the Peers have static methods.
> >> ...
>
> I beg to differ. LargeSelect just tries to call the methods
> addSelectColumns() and populateObject() on the given builder class. You
> can provide any class you want here. I'd prefer to use some kind of
> Builder-interface for this, however.

Agreed. But does one not still need to have non-static methods for
implementing an interface.

   Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

Re: created torque 4 branch with village removed

[Thomas Vandahl <tv...@apache.org>]

Hi Thomas,

On 29.10.10 22:24, Thomas Fox wrote:
>> Of course there are a few things left to do (which I will hopefully
> tackle
>> in the next few weeks):

Great stuff you did. I thought this would need much more time. Respect.

>> - check whether to revive the doPsSelect methods or whether all selects
> can
>> use prepared statements
> 
> Re-added BasePeer.doPSSelect() for now.

This method has some weaknesses IIRC. It dos not work correctly with
selects containing joins. I'd prefer to use prepared statements for all
selects if at all possible and I don't think we need separate methods.

>> - there was come code in doDelete which handled cascading deletes (at
> leats
>> theoretically, never saw it working) tris needs to be reimplemented
> 
> Not re-added. The code can definitely not have worked for composite foreign
> keys because there is no way to tell a composite foreign key from the
> database map.

Still I'm pretty sure that deletes containing joins do not work
correctly. So there should be another solution some day.

>> - Maybe one can clean up LargeSelect from the reflection stuff therein
> 
> Not while the Peers have static methods.
>> ...

I beg to differ. LargeSelect just tries to call the methods
addSelectColumns() and populateObject() on the given builder class. You
can provide any class you want here. I'd prefer to use some kind of
Builder-interface for this, however.

Bye, Thomas.

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

Re: created torque 4 branch with village removed

[Christoph Engelbert <c....@u-form.de>]

 Hi Thomas,

I'll try to test it against our softwaresystem on one of the
testserver next week. I'll give you the results as far as their are
available.

Christoph

Am 29.10.2010 22:24, schrieb Thomas Fox:
>> Of course there are a few things left to do (which I will hopefully
> tackle
>> in the next few weeks):
>> - check whether to revive the doPsSelect methods or whether all selects
> can
>> use prepared statements
> Re-added BasePeer.doPSSelect() for now.
>
>> - there was come code in doDelete which handled cascading deletes (at
> leats
>> theoretically, never saw it working) tris needs to be reimplemented
> Not re-added. The code can definitely not have worked for composite foreign
> keys because there is no way to tell a composite foreign key from the
> database map.
>
>> - Maybe one can clean up LargeSelect from the reflection stuff therein
> Not while the Peers have static methods.
>> ...
> So all the open points are closed now. Please give your opinions whether
> the solutioon outlined in the trunk-without-village branch is the way to go
> for removing village.
>
>    Thanks,
>
>       Thomas
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: torque-dev-help@db.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org

RE: created torque 4 branch with village removed

[Thomas Fox <Th...@seitenbau.net>]

> Of course there are a few things left to do (which I will hopefully
tackle
> in the next few weeks):
> - check whether to revive the doPsSelect methods or whether all selects
can
> use prepared statements

Re-added BasePeer.doPSSelect() for now.

> - there was come code in doDelete which handled cascading deletes (at
leats
> theoretically, never saw it working) tris needs to be reimplemented

Not re-added. The code can definitely not have worked for composite foreign
keys because there is no way to tell a composite foreign key from the
database map.

> - Maybe one can clean up LargeSelect from the reflection stuff therein

Not while the Peers have static methods.
> ...

So all the open points are closed now. Please give your opinions whether
the solutioon outlined in the trunk-without-village branch is the way to go
for removing village.

   Thanks,

      Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org