db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jakob Braeuchi <jbraeu...@gmx.ch>
Subject Re: [OJB] Issue #OJB136 modified [VOTE]
Date Thu, 06 Mar 2003 13:31:00 GMT
hi armin,

you're right the problem occurs when 'autoincrement' and 
fieldconversions collide.
imo a new method (be it autoincrementToJava or whatever) would allow 
conversion of autoincremented values for those who need it.
i'm not sure whether we should ban field conversion for autoincrement 
values, because autoincrement only supports a limited range of types. 
 for maximum flexibility we should allow it by providing a new method in 
FieldConversion and leave the implementation to those who need it ;)

just my 0.02 CHF.

jakob

Armin Waibel wrote:

>Hi,
>
>  
>
>>i'm really not sure about conversion of autoIncrement values.
>>
>>cons:
>>- the autoincrement value is calculated based on the type of the field
>>and thus conversion shouldn't be necessary
>>
>>pros:
>>- i'm sure there's at least one case where conversion makes sense...
>>
>>    
>>
>Say we use a class PrimaryKey
>
>class PrimaryKey
>{
>    private Long key;
>    public PrimaryKey()
>    {
>    ......
>}
>as the primary key field of a persistent object and we want
>to use autoincrement for that field too.
>
>  
>
>>if we do convert autoIncrement values we'll need an additional method
>>    
>>
>in
>  
>
>>FieledConversion because sqlToJava might fail.
>>sqlJava is the mehod called when reading from a ResultSet,  so i
>>    
>>
>propose
>  
>
>>to name the new method:
>>
>>autoIncrementToJava
>>
>>what do you think ?
>>    
>>
>
>The problem only occurs when someone use 'autoincrement'
>and need fieldConversion too, right?
>And only in the sql-->java direction?
>Seems in that case the SequenceManager/BrokerHelper
>be an 'anti-pattern' ;-)
>
>A compromise that should work in 90% of all cases without
>adopt new methods could be to do the field conversion only
>in the 'object case'.
>...
>                // Object
>                else if (!f.getType().isPrimitive())
>                {
>                    // only assign a value if attribute == null
>                    if (f.get(obj) == null)
>                    {
>                        result =
>broker.serviceSequenceManager().getUniqueObject(fmd);
>                        // ### here we do field conversion to support
>user defined fields
>                        result =
>fmd.getFieldConversion().sqlToJava(result);
>                        if (result != null)
>The pitfall is the
>broker.serviceSequenceManager().getUniqueObject(fmd)
>method, because we don't know what type of object the
>sequence manager returns.
>This depends on the SM implementation
>and not on the jdbc-type of the persistent-class field.
>
>Thus it could be difficult to write a adequate FieldConversion
>AND the #sqlToJava(...) method in fact does a 'javaToJava'
>conversion, exactly a 'sequenceManagerToJava' conversion.
>The same problem arise when we add a new method.
>
>Conclusion, I think the only transparent way is to document that
>autoincrement fields do not support field conversion. Or do I
>overlook somthing?
>
>regards,
>Armin
>
>  
>
>>jakob
>>
>>Jakob Braeuchi wrote:
>>
>>    
>>
>>>hi david,
>>>
>>>sorry, you're right there's _no_ double conversion. but we have a
>>>      
>>>
>test
>  
>
>>>case failing after applying this patch.
>>>
>>>FieldConversionTest fails because of  conversion :
>>>
>>>in BrokerHelper  the autoIncr value in result is _Long_ because of
>>>      
>>>
>the
>  
>
>>>typ of the field
>>>
>>>      result = new
>>>Long(broker.serviceSequenceManager().getUniqueLong(fmd));
>>>      // reflect autoincrement value back into object
>>>      f.set(obj, fmd.getFieldConversion().sqlToJava(result));
>>>
>>>the fieldconversion (inner class FieldConversionLongToInteger)
>>>      
>>>
>defined
>  
>
>>>for this field checks for _Integer_ which fails:
>>>
>>>       public Object sqlToJava(Object source) throws
>>>      
>>>
>ConversionException
>  
>
>>>       {
>>>           if (!(source instanceof Integer))
>>>           {
>>>               throw new ConversionException(
>>>                       "Wrong java field type when java-->sql,
>>>expected java.lang.Integer, found "
>>>                       + source.getClass());
>>>           }
>>>           return new Long(((Integer) source).longValue());
>>>       }
>>>
>>>maybe we should make field conversion more tolerant ???
>>>
>>>jakob
>>>
>>>David Marquard wrote:
>>>
>>>      
>>>
>>>>That's not right! The patch doesn't result in double conversion.
>>>>        
>>>>
>The
>  
>
>>>>current
>>>>(unpatched) code does something like this:
>>>>
>>>>result = broker.serviceSequenceManager().getUniqueObject(fmd);
>>>>// reflect autoincrement value back into object
>>>>f.set(obj, result);
>>>>          ^^^^^^
>>>>As you can see, an *unconverted* object is being retrieved from the
>>>>sequence
>>>>manager and set directly into a persistent field without a field
>>>>conversion
>>>>being run on it. The patch changes that second line to:
>>>>
>>>>f.set(obj, fmd.getFieldConversion().sqlToJava(result));
>>>>
>>>>which properly converts the object before setting it in the
>>>>        
>>>>
>persistent
>  
>
>>>>field. The converted object *is not* returned from
>>>>getAutoIncrement(), and
>>>>thus does not cause double conversions. In fact, callers of
>>>>getAutoIncrement() won't see any difference at all.
>>>>
>>>>Dave
>>>>
>>>>-----Original Message-----
>>>>From: Jakob Braeuchi [mailto:jbraeuchi@hotmail.com]
>>>>Sent: Wednesday, March 05, 2003 8:33 AM
>>>>To: Dave Marquard
>>>>Cc: ojb-dev@db.apache.org
>>>>Subject: [OJB] Issue #OJB136 modified
>>>>
>>>>
>>>>
>>>>Issue OJB136 has just been modified by user brj
>>>>
>>>>You can view the issue detail at the following URL:
>>>>    <http://scarab.werken.com/scarab/issues/id/OJB136>
>>>>
>>>>The following modifications were made to this issue:
>>>>
>>>>getAutoIncrement is called be other mehods of BrokerHelper which DO
>>>>field
>>>>conversion.
>>>>applying this patch results in double conversion.
>>>>
>>>>
>>>>        
>>>>
>>>---------------------------------------------------------------------
>>>      
>>>
>>>>To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>>>For additional commands, e-mail: ojb-dev-help@db.apache.org
>>>>
>>>>
>>>>
>>>>
>>>>        
>>>>
>>>      
>>>
>>---------------------------------------------------------------------
>>    
>>
>>>To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>>For additional commands, e-mail: ojb-dev-help@db.apache.org
>>>
>>>
>>>      
>>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>>For additional commands, e-mail: ojb-dev-help@db.apache.org
>>
>>
>>
>>    
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
>For additional commands, e-mail: ojb-dev-help@db.apache.org
>
>
>  
>


Mime
View raw message