db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Armin Waibel" <armin.wai...@code-au-lait.de>
Subject Re: [OJB] Issue #OJB136 modified [VOTE]
Date Thu, 06 Mar 2003 15:20:11 GMT
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
>
>
>


Mime
View raw message