Re: svn commit: r952997 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java

[Adam Heath-2]

[hidden email] wrote:

> Author: jacopoc
> Date: Wed Jun  9 13:20:21 2010
> New Revision: 952997
>
> URL: http://svn.apache.org/viewvc?rev=952997&view=rev
> Log:
> Implemented sql date to time converter to avoid errors with Oracle.
>
>
> Modified:
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=952997&r1=952996&r2=952997&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java Wed Jun  9 13:20:21 2010
> @@ -305,7 +305,7 @@ public class DateTimeConverters implemen
>          }
>  
>          public java.sql.Time convert(java.sql.Date obj) throws ConversionException {
> -            throw new ConversionException("Conversion from Date to Time not supported");
> +            return new java.sql.Time(obj.getTime());
>         }
>      }

Huh?  java.sql.Date has no time component, only a date.  The hours,
minutes, seconds, etc are all set to 0.  java.sql.Time has no date,
the day is January 1, 1970.  So, doing this, will give you a Time of 0.

What is the real problem?  This isn't the correct solution.

[Adrian Crum]

On 6/9/2010 8:49 AM, Adam Heath wrote:

> [hidden email] wrote:
>> Author: jacopoc
>> Date: Wed Jun  9 13:20:21 2010
>> New Revision: 952997
>>
>> URL: http://svn.apache.org/viewvc?rev=952997&view=rev
>> Log:
>> Implemented sql date to time converter to avoid errors with Oracle.
>>
>>
>> Modified:
>>      ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=952997&r1=952996&r2=952997&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java Wed Jun  9 13:20:21 2010
>> @@ -305,7 +305,7 @@ public class DateTimeConverters implemen
>>           }
>>
>>           public java.sql.Time convert(java.sql.Date obj) throws ConversionException {
>> -            throw new ConversionException("Conversion from Date to Time not supported");
>> +            return new java.sql.Time(obj.getTime());
>>          }
>>       }
>
> Huh?  java.sql.Date has no time component, only a date.  The hours,
> minutes, seconds, etc are all set to 0.  java.sql.Time has no date,
> the day is January 1, 1970.  So, doing this, will give you a Time of 0.
>
> What is the real problem?  This isn't the correct solution.

It boils down to an incompatibility with Oracle and the conversion
framework.

I'm working on code to take the conversion framework out of the entity
engine - so that will fix this problem and others. I hope to have it
working this weekend.

-Adrian

[Adam Heath-2]

Adrian Crum wrote:

> On 6/9/2010 8:49 AM, Adam Heath wrote:
>> [hidden email] wrote:
>>> Author: jacopoc
>>> Date: Wed Jun  9 13:20:21 2010
>>> New Revision: 952997
>>>
>>> URL: http://svn.apache.org/viewvc?rev=952997&view=rev
>>> Log:
>>> Implemented sql date to time converter to avoid errors with Oracle.
>>>
>>>
>>> Modified:
>>>    
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>
>>>
>>> Modified:
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=952997&r1=952996&r2=952997&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>> (original)
>>> +++
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>> Wed Jun  9 13:20:21 2010
>>> @@ -305,7 +305,7 @@ public class DateTimeConverters implemen
>>>           }
>>>
>>>           public java.sql.Time convert(java.sql.Date obj) throws
>>> ConversionException {
>>> -            throw new ConversionException("Conversion from Date to
>>> Time not supported");
>>> +            return new java.sql.Time(obj.getTime());
>>>          }
>>>       }
>>
>> Huh?  java.sql.Date has no time component, only a date.  The hours,
>> minutes, seconds, etc are all set to 0.  java.sql.Time has no date,
>> the day is January 1, 1970.  So, doing this, will give you a Time of 0.
>>
>> What is the real problem?  This isn't the correct solution.
>
> It boils down to an incompatibility with Oracle and the conversion
> framework.

No.  The conversion framework is fine.  Don't break it, because some
use of it doesn't work.

If Oracle is broken, then it is the entityengine that must have the fix.

java.sql.Date to java.sql.Time is a disallowed conversion.  If higher
levels attempt that, then those higher levels should protect against that.

> I'm working on code to take the conversion framework out of the entity
> engine - so that will fix this problem and others. I hope to have it
> working this weekend.

Why?

[Adrian Crum]

On 6/9/2010 9:21 AM, Adam Heath wrote:

> Adrian Crum wrote:
>> On 6/9/2010 8:49 AM, Adam Heath wrote:
>>> [hidden email] wrote:
>>>> Author: jacopoc
>>>> Date: Wed Jun  9 13:20:21 2010
>>>> New Revision: 952997
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=952997&view=rev
>>>> Log:
>>>> Implemented sql date to time converter to avoid errors with Oracle.
>>>>
>>>>
>>>> Modified:
>>>>
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=952997&r1=952996&r2=952997&view=diff
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>> Wed Jun  9 13:20:21 2010
>>>> @@ -305,7 +305,7 @@ public class DateTimeConverters implemen
>>>>            }
>>>>
>>>>            public java.sql.Time convert(java.sql.Date obj) throws
>>>> ConversionException {
>>>> -            throw new ConversionException("Conversion from Date to
>>>> Time not supported");
>>>> +            return new java.sql.Time(obj.getTime());
>>>>           }
>>>>        }
>>>
>>> Huh?  java.sql.Date has no time component, only a date.  The hours,
>>> minutes, seconds, etc are all set to 0.  java.sql.Time has no date,
>>> the day is January 1, 1970.  So, doing this, will give you a Time of 0.
>>>
>>> What is the real problem?  This isn't the correct solution.
>>
>> It boils down to an incompatibility with Oracle and the conversion
>> framework.
>
> No.  The conversion framework is fine.  Don't break it, because some
> use of it doesn't work.

I agree this commit should be reverted. I'm just saying I have a fix in
mind that I will be committing later.

> If Oracle is broken, then it is the entityengine that must have the fix.
>
> java.sql.Date to java.sql.Time is a disallowed conversion.  If higher
> levels attempt that, then those higher levels should protect against that.
>
>> I'm working on code to take the conversion framework out of the entity
>> engine - so that will fix this problem and others. I hope to have it
>> working this weekend.
>
> Why?

In the original EE code, field values were simply cast to the Java data
type specified in the fieldtypes*.xml file. The cast was done inside a
long switch block (kind of like how ObjectType.simpleTypeConvert used to
be). The problem with that design was, you couldn't introduce new types
to the EE without modifying that switch block. So I switched that block
of code over to the conversion framework. Now field values are converted
instead of being cast to the target type.

But then problems started to appear due to incompatibilities with
certain JDBC drivers. After thinking about it for a few months, I came
to the conclusion that the change I made was a bad idea. I want to go
back to the original method of casting field values to the target data
type, but still use polymorphism instead of the long switch block.

If a field value needs conversion to another type, it can be done higher
up in the stack.

-Adrian

[Adrian Crum]

On 6/9/2010 9:35 AM, Adrian Crum wrote:

> On 6/9/2010 9:21 AM, Adam Heath wrote:
>> Adrian Crum wrote:
>>> On 6/9/2010 8:49 AM, Adam Heath wrote:
>>>> [hidden email] wrote:
>>>>> Author: jacopoc
>>>>> Date: Wed Jun 9 13:20:21 2010
>>>>> New Revision: 952997
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=952997&view=rev
>>>>> Log:
>>>>> Implemented sql date to time converter to avoid errors with Oracle.
>>>>>
>>>>>
>>>>> Modified:
>>>>>
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>>>
>>>>>
>>>>>
>>>>> Modified:
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>>>
>>>>>
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java?rev=952997&r1=952996&r2=952997&view=diff
>>>>>
>>>>>
>>>>> ==============================================================================
>>>>>
>>>>>
>>>>> ---
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>>>
>>>>> (original)
>>>>> +++
>>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
>>>>>
>>>>> Wed Jun 9 13:20:21 2010
>>>>> @@ -305,7 +305,7 @@ public class DateTimeConverters implemen
>>>>> }
>>>>>
>>>>> public java.sql.Time convert(java.sql.Date obj) throws
>>>>> ConversionException {
>>>>> - throw new ConversionException("Conversion from Date to
>>>>> Time not supported");
>>>>> + return new java.sql.Time(obj.getTime());
>>>>> }
>>>>> }
>>>>
>>>> Huh? java.sql.Date has no time component, only a date. The hours,
>>>> minutes, seconds, etc are all set to 0. java.sql.Time has no date,
>>>> the day is January 1, 1970. So, doing this, will give you a Time of 0.
>>>>
>>>> What is the real problem? This isn't the correct solution.
>>>
>>> It boils down to an incompatibility with Oracle and the conversion
>>> framework.
>>
>> No. The conversion framework is fine. Don't break it, because some
>> use of it doesn't work.
>
> I agree this commit should be reverted. I'm just saying I have a fix in
> mind that I will be committing later.
>
>> If Oracle is broken, then it is the entityengine that must have the fix.
>>
>> java.sql.Date to java.sql.Time is a disallowed conversion. If higher
>> levels attempt that, then those higher levels should protect against
>> that.
>>
>>> I'm working on code to take the conversion framework out of the entity
>>> engine - so that will fix this problem and others. I hope to have it
>>> working this weekend.
>>
>> Why?
>
> In the original EE code, field values were simply cast to the Java data
> type specified in the fieldtypes*.xml file. The cast was done inside a
> long switch block (kind of like how ObjectType.simpleTypeConvert used to
> be). The problem with that design was, you couldn't introduce new types
> to the EE without modifying that switch block. So I switched that block
> of code over to the conversion framework. Now field values are converted
> instead of being cast to the target type.
>
> But then problems started to appear due to incompatibilities with
> certain JDBC drivers. After thinking about it for a few months, I came
> to the conclusion that the change I made was a bad idea. I want to go
> back to the original method of casting field values to the target data
> type, but still use polymorphism instead of the long switch block.
>
> If a field value needs conversion to another type, it can be done higher
> up in the stack.

I think I found out the real reason there is a problem with Oracle. In
JDBC there are three date/time SQL types: Date, Time, and Timestamp.
Oracle has only two date/time SQL types: DATE and TIMESTAMP. Oracle does
not have a TIME SQL type.

The fieldtypeoracle.xml file contains this:

<field-type-def type="date-time" sql-type="TIMESTAMP"
sql-type-alias="TIMESTAMP(6)"
java-type="java.sql.Timestamp"></field-type-def>
<field-type-def type="date" sql-type="DATE"
java-type="java.sql.Date"></field-type-def>
<field-type-def type="time" sql-type="DATE"
java-type="java.sql.Time"></field-type-def>

So, I assume the Oracle JDBC driver is returning a java.sql.Date type
for the OFBiz "time" data type.

-Adrian

[Jacopo Cappellato-4]

On Jun 29, 2010, at 6:45 PM, Adrian Crum wrote:

>
> I think I found out the real reason there is a problem with Oracle. In JDBC there are three date/time SQL types: Date, Time, and Timestamp. Oracle has only two date/time SQL types: DATE and TIMESTAMP. Oracle does not have a TIME SQL type.
>
> The fieldtypeoracle.xml file contains this:
>
> <field-type-def type="date-time" sql-type="TIMESTAMP" sql-type-alias="TIMESTAMP(6)" java-type="java.sql.Timestamp"></field-type-def>
> <field-type-def type="date" sql-type="DATE" java-type="java.sql.Date"></field-type-def>
> <field-type-def type="time" sql-type="DATE" java-type="java.sql.Time"></field-type-def>
>
> So, I assume the Oracle JDBC driver is returning a java.sql.Date type for the OFBiz "time" data type.
>
> -Adrian

Should I try with the following setting:

> <field-type-def type="time" sql-type="DATE" java-type="java.sql.Date"></field-type-def>

?

I think I will be able to d this test if you want me to try it.

Kind regards,

Jacopo

[Adam Heath-2]

Jacopo Cappellato wrote:
> Should I try with the following setting:
>
>> <field-type-def type="time" sql-type="DATE" java-type="java.sql.Date"></field-type-def>

This won't work; java.sql.Date doesn't store time fields; its hours,
minutes, and seconds are all set to 0.

[Adrian Crum-2]

Jacopo,

I have a fix prepared that I'm testing right now. It's the change I mentioned last week - I'm taking the conversion framework out of the entity engine, so this problem will go away.

I've tested it on a few databases, but not Oracle. So when you see the commit, please test it with Oracle and let me know how it works.

-Adrian

--- On Tue, 6/29/10, Jacopo Cappellato <[hidden email]> wrote:

> From: Jacopo Cappellato <[hidden email]>
> Subject: Re: svn commit: r952997 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/DateTimeConverters.java
> To: [hidden email]
> Date: Tuesday, June 29, 2010, 10:54 PM
>
> On Jun 29, 2010, at 6:45 PM, Adrian Crum wrote:
> >
> > I think I found out the real reason there is a problem
> with Oracle. In JDBC there are three date/time SQL types:
> Date, Time, and Timestamp. Oracle has only two date/time SQL
> types: DATE and TIMESTAMP. Oracle does not have a TIME SQL
> type.
> >
> > The fieldtypeoracle.xml file contains this:
> >
> > <field-type-def type="date-time"
> sql-type="TIMESTAMP" sql-type-alias="TIMESTAMP(6)"
> java-type="java.sql.Timestamp"></field-type-def>
> > <field-type-def type="date" sql-type="DATE"
> java-type="java.sql.Date"></field-type-def>
> > <field-type-def type="time" sql-type="DATE"
> java-type="java.sql.Time"></field-type-def>
> >
> > So, I assume the Oracle JDBC driver is returning a
> java.sql.Date type for the OFBiz "time" data type.
> >
> > -Adrian
>
> Should I try with the following setting:
>
> > <field-type-def type="time" sql-type="DATE"
> java-type="java.sql.Date"></field-type-def>
>
> ?
>
> I think I will be able to d this test if you want me to try
> it.
>
> Kind regards,
>
> Jacopo
>
>