db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Mahler <thm...@web.de>
Subject Re: [OJB] Issue #OJB136 modified [VOTE]
Date Fri, 07 Mar 2003 17:49:14 GMT
Hi again Jakob,

Jakob Braeuchi wrote:
> 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.

Ok. Now we only have to solve one issue.
Say SeqMan only returns (say) String.
And we have defined a (say) MyType2Long conversion (so the column 
jdbc-type="BIGINT").

This would cause a problem, as the MyType2Long will fail if fed with a 
String value!
Unfortunately there is no way to see what input-type is expected by 
MyType2Long.sql2java(). The input parameter is just declared as Object.

But the SeqMan could use the jdbc-type="BIGINT" information to generate 
the required Long value.
Once SeqMan feeds a Long value into the MyType2Long conversion 
everything is fine.

SO my proposal is now:
1. The seqman must not try to do any field-conversion on its own.
2. It must produce values that comply with the fieldconversions defined 
for the attribute in question.
3. To be able to do so the generated values must match to the jdbc-type 
of the attribute in question.

I think going this way will provide maximum flexibility while strictly 
sticking to the normal OJB field-conversion mechanism.

cheers,
Thomas

> 
> 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
>>
>>
> 
> 
> ---------------------------------------------------------------------
> 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