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 Fri, 07 Mar 2003 16:18:21 GMT
hi thomas,

i like the approach that SeqMan does NOT do any conversions.  imo 
returning Object (or String) should be sufficient to cover all requirements.

jakob

Thomas Mahler wrote:

> Hi Jakob,
>
> Jakob Braeuchi wrote:
>
>> hi thomas,
>>
>> this leads to the problem i described in an earlier post:
>>
>> the field is defined as Long (java-side) and has a FieldConversion 
>> sql-Int  to java-Long. due to type Long the sequenceManager wil 
>> return Long which is not accepted by FieldConversion looking for 
>> Integer.
>
>
> I we follow my approach we would say that the current solution of the 
> SeqMans to detect the needed type causes a problem.
> It's a mixture of concerns, as the SeqMan does type conversions that 
> are done by FieldConversions for all other attributes.
>
> We could decide that we only use SeqMan.getNextLong() and drop the 
> automatic detection of target types.
> Then the user would be responsible to choose the right field-conversion
> to map this Long Value on the target data type of the autoincrement 
> field.
>
> does this make sense?
> Thomas
>
>
>> btw. the patch contributed by dave marquard treats autoincremented 
>> values as sql but this leads to a failing testcase.
>>
>> jakob
>>
>> Mahler Thomas wrote:
>>
>>> Hi all,
>>>
>>> What about the following:
>>> Let us treat the output of the auoincrement methods as if they just 
>>> came
>>> from a ResultSet, that is as 'sql'.
>>> This will make it easy to work with database based sequence managers 
>>> too.
>>> It is then easy to apply the normal FieldConversion approach for
>>> autoincremented fields.
>>>
>>> IMO this would only have little impact on existing codebase this way.
>>>
>>> I don't like the idea to have extra functionality in the 
>>> FieldConversion
>>> classes for autoincremented fields.
>>>
>>> Did I miss something?
>>> Thomas
>>>
>>>  
>>>
>>>> -----Original Message-----
>>>> From: Armin Waibel [mailto:armin.waibel@code-au-lait.de]
>>>> Sent: Thursday, March 06, 2003 4:20 PM
>>>> To: OJB Developers List
>>>> Subject: Re: [OJB] Issue #OJB136 modified [VOTE]
>>>>
>>>>
>>>> Hi Jakob,
>>>>
>>>> ----- Original Message -----
>>>> From: "Jakob Braeuchi" <jbraeuchi@gmx.ch>
>>>> To: "OJB Developers List" <ojb-dev@db.apache.org>
>>>> Sent: Thursday, March 06, 2003 2:31 PM
>>>> Subject: Re: [OJB] Issue #OJB136 modified [VOTE]
>>>>
>>>>
>>>>  
>>>>
>>>>> 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 agree, but you will run in some hassle writing a FiledConversion
>>>> in a case descripted below.
>>>>
>>>> Say we will implement the method
>>>> public Object autoincrementToJava (Object ojb)
>>>> What type is object 'obj' if our pk-field is a user defined one?
>>>> To get the value BrokerHelper called
>>>> broker.serviceSequenceManager().getUniqueObject(fmd);
>>>> The returned object depends on the used sequence
>>>> manager implementation (think in most cases a String representation
>>>> of a long or int value was returned). This is not transparent for the
>>>> user and could cause trouble e.g.  when change the SM.
>>>>
>>>> If we adopt such a method these pitfalls should be well documented.
>>>>
>>>> just my 0.02 Euro ;-)
>>>>
>>>> Armin
>>>>
>>>>  
>>>>
>>>>> 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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>       
>>>>>
>>>>>
>>>>>
>>>>>     
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>>  
>>>>
>>>>> 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
>>
>>
>
>
>
> ---------------------------------------------------------------------
> 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