db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Marquard <David_Marqu...@forgent.com>
Subject RE: [OJB] Issue #OJB136 modified [VOTE]
Date Thu, 06 Mar 2003 20:49:23 GMT
I like that solution the best, although I'd rather see only
SeqMan.getUniqueObject() used instead. An example of a generated key that
wouldn't fit in a long is the SQL Server "uniqueidentifier" type, which is a
GUID (represented as a String by the jdbc driver).

If we didn't want to change the way SequenceManager was used, we could only
apply FieldConversions to values returned from getUniqueObject(). That would
fix my problem, at least.

Dave

-----Original Message-----
From: Thomas Mahler [mailto:thma32@web.de]
Sent: Thursday, March 06, 2003 1:04 PM
To: OJB Developers List
Subject: Re: [OJB] Issue #OJB136 modified [VOTE]


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