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




Reply via email to