Here's the problem as I see it:
Postgresql used to allow you to specify parameter types which did not
match the column type, in the timestamp case if you passed in a
parameter specified as varchar it would automatically attempt to
convert it to a timestamp. Now Postgresql requires that you either
pass in the correct parameter type for the column or otherwise pass
the parameter type as unknown/unspecified. Postgresql still doesn't
care whether you pass in a string or a timestamp but you MUST specify
the correct sql type.
If letting the database take care of the type conversion has never
been a problem before, why do we need to worry about it now? Remember
the only problem here is that we are passing in a string specified as
varchar instead of a string specified as java.sql.Types.Timestamp.
Regards
Scott
2008/10/14 Adrian Crum <[EMAIL PROTECTED]>:
> I understood your alternatives. I apologize for being unclear.
>
> You said, "or to just throw an exception when the wrong data type is passed
> like the commented out code in GenericEntity referenced above does."
>
> And I'm saying we could do that, but it's going to get interesting because a
> lot of code passes Strings into the entity engine. In other words, if we
> throw a "hard" exception, there is a good chance that much of OFBiz won't
> run.
>
> Personally, I'd like to see better handling of object types in the higher
> level code. I fixed a problem over the weekend that was caused by this very
> thing (passing Strings into the entity engine).
>
> -Adrian
>
>
> David E Jones wrote:
>>
>> I'm referring to the exception I described in the GenericEntity.java file,
>> lines 410-415. Right now it is just a warning in the log (and has been for
>> years). The reason to do it there instead of letting the JDBC driver do it
>> is that not all developers will test on all possible databases, and this
>> will help avoid errors as people use different databases in
>> development/testing or deployment.
>>
>> I tried to describe two possible approaches to improve this situation in
>> my first message. Please let me know if those alternatives were not clear
>> and I'll try to explain them better.
>>
>> -David
>>
>>
>> On Oct 13, 2008, at 5:22 PM, Adrian Crum wrote:
>>
>>> Wasn't that being done already? Jacques started these changes based on
>>> exceptions being thrown by his JDBC driver. Other DBs seemed to be working
>>> okay.
>>>
>>> I guess we could throw our own exception to keep things consistent across
>>> databases. It will be interesting to see how that affects existing code.
>>>
>>> -Adrian
>>>
>>>
>>> David E Jones wrote:
>>>>
>>>> That is a very good point, and I agree.
>>>> To me that means we go with the fail-fast approach and have it throw an
>>>> exception if you pass in something that is not the correct object type.
>>>> Is that what you meant?
>>>> -David
>>>> On Oct 13, 2008, at 4:57 PM, Adrian Crum wrote:
>>>>>
>>>>> I would prefer handling object types in higher level code - due to
>>>>> ambiguity in how some types would be converted from a String (currency,
>>>>> date, time, etc).
>>>>>
>>>>> -Adrian
>>>>>
>>>>> David E Jones wrote:
>>>>>>
>>>>>> This has been a potential problem for a while, but the decision was
>>>>>> made quite a while back to not enforce the correct object type.
>>>>>> There is code in the GenericEntity.java file (lines 410-415) to check
>>>>>> to see if the value passed in matches the object type for the field it is
>>>>>> being set on.
>>>>>> The best way to solve this problem is a good question. It might be
>>>>>> good to introduce some automatic type conversion, or to just throw an
>>>>>> exception when the wrong data type is passed like the commented out code
>>>>>> in
>>>>>> GenericEntity referenced above does.
>>>>>> The most common form of automatic conversion is from String to
>>>>>> whatever the field's object type is, and that is done by the
>>>>>> GenericEntity.setString method.
>>>>>> So, what do people think about this? Should we do an automatic type
>>>>>> conversion to try to fix programming errors automatically, or do a
>>>>>> fail-fast
>>>>>> by throwing an exception when the object type is wrong? I suppose the
>>>>>> current fail-quiet (in the log) and fail loudly on certain databases
>>>>>> later
>>>>>> is not the best option...
>>>>>> -David
>>>>>> On Oct 12, 2008, at 2:20 PM, Scott Gray wrote:
>>>>>>>
>>>>>>> I've had a look into this and I can't see anything related to Groovy
>>>>>>> that is making this necessary, it appears to be entirely a postgresql
>>>>>>> issue.
>>>>>>>
>>>>>>> When executing a prepared statement postgresql seems to require that
>>>>>>> the parameter list sql types match the column types. So the problem
>>>>>>> isn't that we are passing in a string but that we are setting the sql
>>>>>>> type to character varying by using PreparedStatement.setString().
>>>>>>>
>>>>>>> Here's a patch that fixes the issue but I'm not really confident
>>>>>>> enough to commit it, it would be great to get some comments from
>>>>>>> people who know more about this kind of thing:
>>>>>>>
>>>>>>> Index: framework/entity/src/org/ofbiz/entity/jdbc/SQLProcessor.java
>>>>>>> ===================================================================
>>>>>>> --- framework/entity/src/org/ofbiz/entity/jdbc/SQLProcessor.java
>>>>>>> (revision
>>>>>>> 703572)
>>>>>>> +++ framework/entity/src/org/ofbiz/entity/jdbc/SQLProcessor.java
>>>>>>> (working copy)
>>>>>>> @@ -592,6 +592,22 @@
>>>>>>> *
>>>>>>> * @throws SQLException
>>>>>>> */
>>>>>>> + public void setValueTimestampString(String field) throws
>>>>>>> SQLException {
>>>>>>> + if (field != null) {
>>>>>>> + _ps.setObject(_ind, field, Types.TIMESTAMP);
>>>>>>> + } else {
>>>>>>> + _ps.setNull(_ind, Types.TIMESTAMP);
>>>>>>> + }
>>>>>>> + _ind++;
>>>>>>> + }
>>>>>>> +
>>>>>>> + /**
>>>>>>> + * Set the next binding variable of the currently active
>>>>>>> prepared
>>>>>>> statement.
>>>>>>> + *
>>>>>>> + * @param field
>>>>>>> + *
>>>>>>> + * @throws SQLException
>>>>>>> + */
>>>>>>> public void setValue(java.sql.Time field) throws SQLException {
>>>>>>> if (field != null) {
>>>>>>> _ps.setTime(_ind, field);
>>>>>>> Index: framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>>>>>>> ===================================================================
>>>>>>> --- framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>>>>>>> (revision
>>>>>>> 703572)
>>>>>>> +++ framework/entity/src/org/ofbiz/entity/jdbc/SqlJdbcUtil.java
>>>>>>> (working copy)
>>>>>>> @@ -731,6 +731,9 @@
>>>>>>> fieldClassName = "byte[]";
>>>>>>> }
>>>>>>>
>>>>>>> + if ("java.sql.Timestamp".equals(fieldType)) {
>>>>>>> + fieldClassName = fieldType;
>>>>>>> + }
>>>>>>> if (Debug.verboseOn()) Debug.logVerbose("type of
>>>>>>> field " + entityName + "." + modelField.getName() +
>>>>>>> " is " + fieldClassName + ", was expecting "
>>>>>>> + mft.getJavaType() + "; this may " +
>>>>>>> "indicate an error in the configuration or in
>>>>>>> the class, and may result " +
>>>>>>> @@ -749,7 +752,11 @@
>>>>>>> break;
>>>>>>>
>>>>>>> case 2:
>>>>>>> - sqlP.setValue((java.sql.Timestamp) fieldValue);
>>>>>>> + if (fieldValue instanceof String) {
>>>>>>> + sqlP.setValueTimestampString((String)
>>>>>>> fieldValue);
>>>>>>> + } else {
>>>>>>> + sqlP.setValue((java.sql.Timestamp) fieldValue);
>>>>>>> + }
>>>>>>> break;
>>>>>>>
>>>>>>> case 3:
>>>>>>>
>>>>>>> Regards
>>>>>>> Scott
>>>>>>>
>>>>>>>
>>>>>>> 2008/10/13 Jacques Le Roux <[EMAIL PROTECTED]>:
>>>>>>>>
>>>>>>>> Done in revision: 703816
>>>>>>>> It was not possible for PackingSlip.groovy and
>>>>>>>> FindInventoryEventPlan.groovy. Because there the date string is
>>>>>>>> build
>>>>>>>> dynamically in the Groovy file
>>>>>>>>
>>>>>>>> Jacques
>>>>>>>>
>>>>>>>> From: "Jacques Le Roux" <[EMAIL PROTECTED]>
>>>>>>>>>
>>>>>>>>> Adrian,
>>>>>>>>>
>>>>>>>>> Yes good idea indeed, I will do that
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>>>
>>>>>>>>> From: "Adrian Crum" <[EMAIL PROTECTED]>
>>>>>>>>>>
>>>>>>>>>> Jacques,
>>>>>>>>>>
>>>>>>>>>> Instead of modifying the groovy files, try specifying the data
>>>>>>>>>> type in
>>>>>>>>>> the screen widget.
>>>>>>>>>>
>>>>>>>>>> Example in ReportFinancialSummaryScreens.xml:
>>>>>>>>>>
>>>>>>>>>> <set field="fromDate" from-field="parameters.fromDate"
>>>>>>>>>> type="Timestamp"/>
>>>>>>>>>> <set field="thruDate" from-field="parameters.thruDate"
>>>>>>>>>> type="Timestamp"/>
>>>>>>>>>> <script
>>>>>>>>>>
>>>>>>>>>> location="component://accounting/webapp/accounting/WEB-INF/actions/reports/TransactionTotals.groovy"/>
>>>>>>>>>>
>>>>>>>>>> -Adrian
>>>>>>>>
>>>>>>>>
>>
>>
>
