Revision 703731
Locked
123
123
 
|
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 |
Administrator
|
Adrian,
Yes good idea indeed, I will do that Jacques From: "Adrian Crum" <[hidden email]> > 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 > > > > > |
|
Administrator
|
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" <[hidden email]> > Adrian, > > Yes good idea indeed, I will do that > > Jacques > > From: "Adrian Crum" <[hidden email]> >> 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 |
|
|
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 <[hidden email]>: > 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" <[hidden email]> >> >> Adrian, >> >> Yes good idea indeed, I will do that >> >> Jacques >> >> From: "Adrian Crum" <[hidden email]> >>> >>> 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 > > |
|
Administrator
|
Hi Scott,
No time to look into details tonight. Just to let you know that I considered this a Groovy issue because it seems that the same code used in Beanshell did not have this problem. Maybe it's no related to Groovy itself, but the way we interface it ? BTW we have the same kind of issue (not timestamp related though) at https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails There it's between "double precision" and "character varying". I guess, the same kinf of fix may be applied as well Jacques From: "Scott Gray" <[hidden email]> > 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 <[hidden email]>: >> 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" <[hidden email]> >>> >>> Adrian, >>> >>> Yes good idea indeed, I will do that >>> >>> Jacques >>> >>> From: "Adrian Crum" <[hidden email]> >>>> >>>> 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 >> >> > |
|
|
Hi Jacques
I really don't think it has anything to do with groovy, it is just passing a simple string into the entity condition which is no more or less than beanshell would have done. I think it just seems that way because we noticed the problem after the beanshell->groovy switch, the change probably came from postgresql in some recent version. I'll have a look at the double problem as soon as I get a chance. Thanks Scott 2008/10/13 Jacques Le Roux <[hidden email]>: > Hi Scott, > > No time to look into details tonight. Just to let you know that I considered > this a Groovy issue because it seems that the same code used in Beanshell > did not have this problem. Maybe it's no related to Groovy itself, but the > way we interface it ? > > BTW we have the same kind of issue (not timestamp related though) at > https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails > There it's between "double precision" and "character varying". I guess, the > same kinf of fix may be applied as well > > Jacques > > From: "Scott Gray" <[hidden email]> >> >> 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 <[hidden email]>: >>> >>> 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" <[hidden email]> >>>> >>>> Adrian, >>>> >>>> Yes good idea indeed, I will do that >>>> >>>> Jacques >>>> >>>> From: "Adrian Crum" <[hidden email]> >>>>> >>>>> 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 >>> >>> >> > > |
|
Administrator
|
Yes, you are right.
https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails works with postgres 8.2.9 Maybe it even comes from my own Postgres 8.3 installation. I will try with a new one. Thanks Jacques From: "Scott Gray" <[hidden email]> > Hi Jacques > > I really don't think it has anything to do with groovy, it is just > passing a simple string into the entity condition which is no more or > less than beanshell would have done. I think it just seems that way > because we noticed the problem after the beanshell->groovy switch, the > change probably came from postgresql in some recent version. > > I'll have a look at the double problem as soon as I get a chance. > > Thanks > Scott > > 2008/10/13 Jacques Le Roux <[hidden email]>: >> Hi Scott, >> >> No time to look into details tonight. Just to let you know that I considered >> this a Groovy issue because it seems that the same code used in Beanshell >> did not have this problem. Maybe it's no related to Groovy itself, but the >> way we interface it ? >> >> BTW we have the same kind of issue (not timestamp related though) at >> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >> There it's between "double precision" and "character varying". I guess, the >> same kinf of fix may be applied as well >> >> Jacques >> >> From: "Scott Gray" <[hidden email]> >>> >>> 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 <[hidden email]>: >>>> >>>> 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" <[hidden email]> >>>>> >>>>> Adrian, >>>>> >>>>> Yes good idea indeed, I will do that >>>>> >>>>> Jacques >>>>> >>>>> From: "Adrian Crum" <[hidden email]> >>>>>> >>>>>> 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 >>>> >>>> >>> >> >> > |
|
Administrator
|
I suppose it was a problem with my Postgres installation since with a new Postgres 8.3 installation
ViewFacilityInventoryItemsDetails works. I also wondered why nobody was complaining about this issue but me. I think we can forget it, as the changes I done (suggested by Adrian) are harmless anyway. Hopefully I did not introduce a bug. Sorry for the bothering Jacques From: "Jacques Le Roux" <[hidden email]> > Yes, you are right. https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails works with postgres 8.2.9 Maybe it > even comes from my own Postgres 8.3 installation. I will try with a new one. > > Thanks > > Jacques > > From: "Scott Gray" <[hidden email]> >> Hi Jacques >> >> I really don't think it has anything to do with groovy, it is just >> passing a simple string into the entity condition which is no more or >> less than beanshell would have done. I think it just seems that way >> because we noticed the problem after the beanshell->groovy switch, the >> change probably came from postgresql in some recent version. >> >> I'll have a look at the double problem as soon as I get a chance. >> >> Thanks >> Scott >> >> 2008/10/13 Jacques Le Roux <[hidden email]>: >>> Hi Scott, >>> >>> No time to look into details tonight. Just to let you know that I considered >>> this a Groovy issue because it seems that the same code used in Beanshell >>> did not have this problem. Maybe it's no related to Groovy itself, but the >>> way we interface it ? >>> >>> BTW we have the same kind of issue (not timestamp related though) at >>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>> There it's between "double precision" and "character varying". I guess, the >>> same kinf of fix may be applied as well >>> >>> Jacques >>> >>> From: "Scott Gray" <[hidden email]> >>>> >>>> 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 <[hidden email]>: >>>>> >>>>> 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" <[hidden email]> >>>>>> >>>>>> Adrian, >>>>>> >>>>>> Yes good idea indeed, I will do that >>>>>> >>>>>> Jacques >>>>>> >>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>> >>>>>>> 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 >>>>> >>>>> >>>> >>> >>> >> > |
|
|
I was able to reproduce the problem here (with timestamps I haven't
tried doubles) on a fresh install, could you please double check that it's working? Thanks Scott 2008/10/13 Jacques Le Roux <[hidden email]>: > I suppose it was a problem with my Postgres installation since with a new > Postgres 8.3 installation ViewFacilityInventoryItemsDetails works. > I also wondered why nobody was complaining about this issue but me. > > I think we can forget it, as the changes I done (suggested by Adrian) are > harmless anyway. Hopefully I did not introduce a bug. > > Sorry for the bothering > > Jacques > > From: "Jacques Le Roux" <[hidden email]> >> >> Yes, you are right. >> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >> works with postgres 8.2.9 Maybe it even comes from my own Postgres 8.3 >> installation. I will try with a new one. >> >> Thanks >> >> Jacques >> >> From: "Scott Gray" <[hidden email]> >>> >>> Hi Jacques >>> >>> I really don't think it has anything to do with groovy, it is just >>> passing a simple string into the entity condition which is no more or >>> less than beanshell would have done. I think it just seems that way >>> because we noticed the problem after the beanshell->groovy switch, the >>> change probably came from postgresql in some recent version. >>> >>> I'll have a look at the double problem as soon as I get a chance. >>> >>> Thanks >>> Scott >>> >>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>> >>>> Hi Scott, >>>> >>>> No time to look into details tonight. Just to let you know that I >>>> considered >>>> this a Groovy issue because it seems that the same code used in >>>> Beanshell >>>> did not have this problem. Maybe it's no related to Groovy itself, but >>>> the >>>> way we interface it ? >>>> >>>> BTW we have the same kind of issue (not timestamp related though) at >>>> >>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>> There it's between "double precision" and "character varying". I guess, >>>> the >>>> same kinf of fix may be applied as well >>>> >>>> Jacques >>>> >>>> From: "Scott Gray" <[hidden email]> >>>>> >>>>> 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 <[hidden email]>: >>>>>> >>>>>> 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" <[hidden email]> >>>>>>> >>>>>>> Adrian, >>>>>>> >>>>>>> Yes good idea indeed, I will do that >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>> >>>>>>>> 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 >>>>>> >>>>>> >>>>> >>>> >>>> >>> >> > > |
|
Administrator
|
Hi Scott,
I just commited a bug fix (typo) for MarketingCampaignReport.groovy in r704013. It's an easy way to try with Timestamp. If you remove <<type="Timestamp">> from lines <set field="fromDate" from-field="requestParameters.fromDate" type="Timestamp"/> <set field="thruDate" from-field="requestParameters.thruDate" type="Timestamp"/> in MarketingCampaignReport (in MarketingReportScreens.xml) You will see that (at least) with Postgres 8.3.4 it's still working. FYI it's not working with my other installation which uses Postgres 8.3.1. There I get "timestamp with time zone >= character varying". I use the same postgresql-8.3-603.jdbc3.jar driver (447 301 bytes each). So it could be a Postgres bug or something related to my installation (I remember having tried some not default settings on this installation) Please, let me know what you get HTH Jacques From: "Scott Gray" <[hidden email]> >I was able to reproduce the problem here (with timestamps I haven't > tried doubles) on a fresh install, could you please double check that > it's working? > > Thanks > Scott > > 2008/10/13 Jacques Le Roux <[hidden email]>: >> I suppose it was a problem with my Postgres installation since with a new >> Postgres 8.3 installation ViewFacilityInventoryItemsDetails works. >> I also wondered why nobody was complaining about this issue but me. >> >> I think we can forget it, as the changes I done (suggested by Adrian) are >> harmless anyway. Hopefully I did not introduce a bug. >> >> Sorry for the bothering >> >> Jacques >> >> From: "Jacques Le Roux" <[hidden email]> >>> >>> Yes, you are right. >>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>> works with postgres 8.2.9 Maybe it even comes from my own Postgres 8.3 >>> installation. I will try with a new one. >>> >>> Thanks >>> >>> Jacques >>> >>> From: "Scott Gray" <[hidden email]> >>>> >>>> Hi Jacques >>>> >>>> I really don't think it has anything to do with groovy, it is just >>>> passing a simple string into the entity condition which is no more or >>>> less than beanshell would have done. I think it just seems that way >>>> because we noticed the problem after the beanshell->groovy switch, the >>>> change probably came from postgresql in some recent version. >>>> >>>> I'll have a look at the double problem as soon as I get a chance. >>>> >>>> Thanks >>>> Scott >>>> >>>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>>> >>>>> Hi Scott, >>>>> >>>>> No time to look into details tonight. Just to let you know that I >>>>> considered >>>>> this a Groovy issue because it seems that the same code used in >>>>> Beanshell >>>>> did not have this problem. Maybe it's no related to Groovy itself, but >>>>> the >>>>> way we interface it ? >>>>> >>>>> BTW we have the same kind of issue (not timestamp related though) at >>>>> >>>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>>> There it's between "double precision" and "character varying". I guess, >>>>> the >>>>> same kinf of fix may be applied as well >>>>> >>>>> Jacques >>>>> >>>>> From: "Scott Gray" <[hidden email]> >>>>>> >>>>>> 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 <[hidden email]>: >>>>>>> >>>>>>> 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" <[hidden email]> >>>>>>>> >>>>>>>> Adrian, >>>>>>>> >>>>>>>> Yes good idea indeed, I will do that >>>>>>>> >>>>>>>> Jacques >>>>>>>> >>>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>>> >>>>>>>>> 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 >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >>> >> >> > |
|
Administrator
|
Sorry forget it, I was wrong, I did the <<type="Timestamp">> removing in the wrong screen !
So yes I was able to reproduce the problem with Timestamp. Not sure why ViewFacilityInventoryItemsDetails works on this new installation though :/ Jacques From: "Jacques Le Roux" <[hidden email]> > Hi Scott, > > I just commited a bug fix (typo) for MarketingCampaignReport.groovy in r704013. It's an easy way to try with Timestamp. > > If you remove <<type="Timestamp">> from lines > <set field="fromDate" from-field="requestParameters.fromDate" type="Timestamp"/> > <set field="thruDate" from-field="requestParameters.thruDate" type="Timestamp"/> > in MarketingCampaignReport (in MarketingReportScreens.xml) > You will see that (at least) with Postgres 8.3.4 it's still working. > FYI it's not working with my other installation which uses Postgres 8.3.1. > There I get "timestamp with time zone >= character varying". I use the same postgresql-8.3-603.jdbc3.jar driver (447 301 bytes > each). > So it could be a Postgres bug or something related to my installation (I remember having tried some not default settings on this > installation) > > Please, let me know what you get > > HTH > > Jacques > > From: "Scott Gray" <[hidden email]> >>I was able to reproduce the problem here (with timestamps I haven't >> tried doubles) on a fresh install, could you please double check that >> it's working? >> >> Thanks >> Scott >> >> 2008/10/13 Jacques Le Roux <[hidden email]>: >>> I suppose it was a problem with my Postgres installation since with a new >>> Postgres 8.3 installation ViewFacilityInventoryItemsDetails works. >>> I also wondered why nobody was complaining about this issue but me. >>> >>> I think we can forget it, as the changes I done (suggested by Adrian) are >>> harmless anyway. Hopefully I did not introduce a bug. >>> >>> Sorry for the bothering >>> >>> Jacques >>> >>> From: "Jacques Le Roux" <[hidden email]> >>>> >>>> Yes, you are right. >>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>> works with postgres 8.2.9 Maybe it even comes from my own Postgres 8.3 >>>> installation. I will try with a new one. >>>> >>>> Thanks >>>> >>>> Jacques >>>> >>>> From: "Scott Gray" <[hidden email]> >>>>> >>>>> Hi Jacques >>>>> >>>>> I really don't think it has anything to do with groovy, it is just >>>>> passing a simple string into the entity condition which is no more or >>>>> less than beanshell would have done. I think it just seems that way >>>>> because we noticed the problem after the beanshell->groovy switch, the >>>>> change probably came from postgresql in some recent version. >>>>> >>>>> I'll have a look at the double problem as soon as I get a chance. >>>>> >>>>> Thanks >>>>> Scott >>>>> >>>>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>>>> >>>>>> Hi Scott, >>>>>> >>>>>> No time to look into details tonight. Just to let you know that I >>>>>> considered >>>>>> this a Groovy issue because it seems that the same code used in >>>>>> Beanshell >>>>>> did not have this problem. Maybe it's no related to Groovy itself, but >>>>>> the >>>>>> way we interface it ? >>>>>> >>>>>> BTW we have the same kind of issue (not timestamp related though) at >>>>>> >>>>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>>>> There it's between "double precision" and "character varying". I guess, >>>>>> the >>>>>> same kinf of fix may be applied as well >>>>>> >>>>>> Jacques >>>>>> >>>>>> From: "Scott Gray" <[hidden email]> >>>>>>> >>>>>>> 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 <[hidden email]>: >>>>>>>> >>>>>>>> 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" <[hidden email]> >>>>>>>>> >>>>>>>>> Adrian, >>>>>>>>> >>>>>>>>> Yes good idea indeed, I will do that >>>>>>>>> >>>>>>>>> Jacques >>>>>>>>> >>>>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>>>> >>>>>>>>>> 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 >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >>> >> > |
|
|
Are you actually getting any results displayed? I doesn't work on
mine but the error only shows up in the logs... Thanks Scott 2008/10/14 Jacques Le Roux <[hidden email]>: > Sorry forget it, I was wrong, I did the <<type="Timestamp">> removing in the > wrong screen ! > So yes I was able to reproduce the problem with Timestamp. > Not sure why ViewFacilityInventoryItemsDetails works on this new > installation though :/ > > Jacques > > From: "Jacques Le Roux" <[hidden email]> >> >> Hi Scott, >> >> I just commited a bug fix (typo) for MarketingCampaignReport.groovy in >> r704013. It's an easy way to try with Timestamp. >> >> If you remove <<type="Timestamp">> from lines >> <set field="fromDate" from-field="requestParameters.fromDate" >> type="Timestamp"/> >> <set field="thruDate" from-field="requestParameters.thruDate" >> type="Timestamp"/> >> in MarketingCampaignReport (in MarketingReportScreens.xml) >> You will see that (at least) with Postgres 8.3.4 it's still working. >> FYI it's not working with my other installation which uses Postgres 8.3.1. >> There I get "timestamp with time zone >= character varying". I use the >> same postgresql-8.3-603.jdbc3.jar driver (447 301 bytes each). >> So it could be a Postgres bug or something related to my installation (I >> remember having tried some not default settings on this installation) >> >> Please, let me know what you get >> >> HTH >> >> Jacques >> >> From: "Scott Gray" <[hidden email]> >>> >>> I was able to reproduce the problem here (with timestamps I haven't >>> tried doubles) on a fresh install, could you please double check that >>> it's working? >>> >>> Thanks >>> Scott >>> >>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>> >>>> I suppose it was a problem with my Postgres installation since with a >>>> new >>>> Postgres 8.3 installation ViewFacilityInventoryItemsDetails works. >>>> I also wondered why nobody was complaining about this issue but me. >>>> >>>> I think we can forget it, as the changes I done (suggested by Adrian) >>>> are >>>> harmless anyway. Hopefully I did not introduce a bug. >>>> >>>> Sorry for the bothering >>>> >>>> Jacques >>>> >>>> From: "Jacques Le Roux" <[hidden email]> >>>>> >>>>> Yes, you are right. >>>>> >>>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>>> works with postgres 8.2.9 Maybe it even comes from my own Postgres 8.3 >>>>> installation. I will try with a new one. >>>>> >>>>> Thanks >>>>> >>>>> Jacques >>>>> >>>>> From: "Scott Gray" <[hidden email]> >>>>>> >>>>>> Hi Jacques >>>>>> >>>>>> I really don't think it has anything to do with groovy, it is just >>>>>> passing a simple string into the entity condition which is no more or >>>>>> less than beanshell would have done. I think it just seems that way >>>>>> because we noticed the problem after the beanshell->groovy switch, the >>>>>> change probably came from postgresql in some recent version. >>>>>> >>>>>> I'll have a look at the double problem as soon as I get a chance. >>>>>> >>>>>> Thanks >>>>>> Scott >>>>>> >>>>>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>>>>> >>>>>>> Hi Scott, >>>>>>> >>>>>>> No time to look into details tonight. Just to let you know that I >>>>>>> considered >>>>>>> this a Groovy issue because it seems that the same code used in >>>>>>> Beanshell >>>>>>> did not have this problem. Maybe it's no related to Groovy itself, >>>>>>> but >>>>>>> the >>>>>>> way we interface it ? >>>>>>> >>>>>>> BTW we have the same kind of issue (not timestamp related though) at >>>>>>> >>>>>>> >>>>>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>>>>> There it's between "double precision" and "character varying". I >>>>>>> guess, >>>>>>> the >>>>>>> same kinf of fix may be applied as well >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> From: "Scott Gray" <[hidden email]> >>>>>>>> >>>>>>>> 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 <[hidden email]>: >>>>>>>>> >>>>>>>>> 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" <[hidden email]> >>>>>>>>>> >>>>>>>>>> Adrian, >>>>>>>>>> >>>>>>>>>> Yes good idea indeed, I will do that >>>>>>>>>> >>>>>>>>>> Jacques >>>>>>>>>> >>>>>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>>>>> >>>>>>>>>>> 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 >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>>> >>> >> > |
|
Administrator
|
I get this on screen
org.ofbiz.widget.screen.ScreenRenderException: Error rendering screen [component://marketing/widget/MarketingReportScreens.xml#MarketingCampaignReport]: org.ofbiz.base.util.GeneralException: Error running Groovy script at location [component://marketing/webapp/marketing/WEB-INF/actions/reports/MarketingCampaignReport.groovy] (SQL Exception while executing the following:SELECT TC.MARKETING_CAMPAIGN_ID, COUNT(TCV.VISIT_ID) FROM public.TRACKING_CODE TC LEFT OUTER JOIN public.TRACKING_CODE_VISIT TCV ON TC.TRACKING_CODE_ID = TCV.TRACKING_CODE_ID WHERE (TCV.FROM_DATE >= ? AND TCV.FROM_DATE <= ?) GROUP BY TC.MARKETING_CAMPAIGN_ID ORDER BY TC.MARKETING_CAMPAIGN_ID ASC (ERREUR: l'op�rateur n'existe pas : timestamp with time zone >= character varying)) (Error running Groovy script at location [component://marketing/webapp/marketing/WEB-INF/actions/reports/MarketingCampaignReport.groovy] (SQL Exception while executing the following:SELECT TC.MARKETING_CAMPAIGN_ID, COUNT(TCV.VISIT_ID) FROM public.TRACKING_CODE TC LEFT OUTER JOIN public.TRACKING_CODE_VISIT TCV ON TC.TRACKING_CODE_ID = TCV.TRACKING_CODE_ID WHERE (TCV.FROM_DATE >= ? AND TCV.FROM_DATE <= ?) GROUP BY TC.MARKETING_CAMPAIGN_ID ORDER BY TC.MARKETING_CAMPAIGN_ID ASC (ERREUR: l'op�rateur n'existe pas : timestamp with time zone >= character varying))) The weird part is that ViewFacilityInventoryItemsDetails works now also on Postgres 8.3.1 (always on Win XP) :/ If I remember well it's not the 1st time I see it re-working. I guess there is certainly something in OFBIz under all that. Jacques From: "Scott Gray" <[hidden email]> > Are you actually getting any results displayed? I doesn't work on > mine but the error only shows up in the logs... > > Thanks > Scott > > 2008/10/14 Jacques Le Roux <[hidden email]>: >> Sorry forget it, I was wrong, I did the <<type="Timestamp">> removing in the >> wrong screen ! >> So yes I was able to reproduce the problem with Timestamp. >> Not sure why ViewFacilityInventoryItemsDetails works on this new >> installation though :/ >> >> Jacques >> >> From: "Jacques Le Roux" <[hidden email]> >>> >>> Hi Scott, >>> >>> I just commited a bug fix (typo) for MarketingCampaignReport.groovy in >>> r704013. It's an easy way to try with Timestamp. >>> >>> If you remove <<type="Timestamp">> from lines >>> <set field="fromDate" from-field="requestParameters.fromDate" >>> type="Timestamp"/> >>> <set field="thruDate" from-field="requestParameters.thruDate" >>> type="Timestamp"/> >>> in MarketingCampaignReport (in MarketingReportScreens.xml) >>> You will see that (at least) with Postgres 8.3.4 it's still working. >>> FYI it's not working with my other installation which uses Postgres 8.3.1. >>> There I get "timestamp with time zone >= character varying". I use the >>> same postgresql-8.3-603.jdbc3.jar driver (447 301 bytes each). >>> So it could be a Postgres bug or something related to my installation (I >>> remember having tried some not default settings on this installation) >>> >>> Please, let me know what you get >>> >>> HTH >>> >>> Jacques >>> >>> From: "Scott Gray" <[hidden email]> >>>> >>>> I was able to reproduce the problem here (with timestamps I haven't >>>> tried doubles) on a fresh install, could you please double check that >>>> it's working? >>>> >>>> Thanks >>>> Scott >>>> >>>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>>> >>>>> I suppose it was a problem with my Postgres installation since with a >>>>> new >>>>> Postgres 8.3 installation ViewFacilityInventoryItemsDetails works. >>>>> I also wondered why nobody was complaining about this issue but me. >>>>> >>>>> I think we can forget it, as the changes I done (suggested by Adrian) >>>>> are >>>>> harmless anyway. Hopefully I did not introduce a bug. >>>>> >>>>> Sorry for the bothering >>>>> >>>>> Jacques >>>>> >>>>> From: "Jacques Le Roux" <[hidden email]> >>>>>> >>>>>> Yes, you are right. >>>>>> >>>>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>>>> works with postgres 8.2.9 Maybe it even comes from my own Postgres 8.3 >>>>>> installation. I will try with a new one. >>>>>> >>>>>> Thanks >>>>>> >>>>>> Jacques >>>>>> >>>>>> From: "Scott Gray" <[hidden email]> >>>>>>> >>>>>>> Hi Jacques >>>>>>> >>>>>>> I really don't think it has anything to do with groovy, it is just >>>>>>> passing a simple string into the entity condition which is no more or >>>>>>> less than beanshell would have done. I think it just seems that way >>>>>>> because we noticed the problem after the beanshell->groovy switch, the >>>>>>> change probably came from postgresql in some recent version. >>>>>>> >>>>>>> I'll have a look at the double problem as soon as I get a chance. >>>>>>> >>>>>>> Thanks >>>>>>> Scott >>>>>>> >>>>>>> 2008/10/13 Jacques Le Roux <[hidden email]>: >>>>>>>> >>>>>>>> Hi Scott, >>>>>>>> >>>>>>>> No time to look into details tonight. Just to let you know that I >>>>>>>> considered >>>>>>>> this a Groovy issue because it seems that the same code used in >>>>>>>> Beanshell >>>>>>>> did not have this problem. Maybe it's no related to Groovy itself, >>>>>>>> but >>>>>>>> the >>>>>>>> way we interface it ? >>>>>>>> >>>>>>>> BTW we have the same kind of issue (not timestamp related though) at >>>>>>>> >>>>>>>> >>>>>>>> https://localhost:8443/facility/control/ViewFacilityInventoryItemsDetails >>>>>>>> There it's between "double precision" and "character varying". I >>>>>>>> guess, >>>>>>>> the >>>>>>>> same kinf of fix may be applied as well >>>>>>>> >>>>>>>> Jacques >>>>>>>> >>>>>>>> From: "Scott Gray" <[hidden email]> >>>>>>>>> >>>>>>>>> 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 <[hidden email]>: >>>>>>>>>> >>>>>>>>>> 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" <[hidden email]> >>>>>>>>>>> >>>>>>>>>>> Adrian, >>>>>>>>>>> >>>>>>>>>>> Yes good idea indeed, I will do that >>>>>>>>>>> >>>>>>>>>>> Jacques >>>>>>>>>>> >>>>>>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>>>>>> >>>>>>>>>>>> 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 >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >>> >> > |
Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
In reply to this post by Scott Gray
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 <[hidden email]>: >> 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" <[hidden email]> >>> >>> Adrian, >>> >>> Yes good idea indeed, I will do that >>> >>> Jacques >>> >>> From: "Adrian Crum" <[hidden email]> >>>> >>>> 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 >> >> |
Re: Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
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 <[hidden email]>: >>> 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" <[hidden email]> >>>> >>>> Adrian, >>>> >>>> Yes good idea indeed, I will do that >>>> >>>> Jacques >>>> >>>> From: "Adrian Crum" <[hidden email]> >>>>> >>>>> 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 >>> >>> > > |
Re: Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
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 <[hidden email]>: >>>> 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" <[hidden email]> >>>>> >>>>> Adrian, >>>>> >>>>> Yes good idea indeed, I will do that >>>>> >>>>> Jacques >>>>> >>>>> From: "Adrian Crum" <[hidden email]> >>>>>> >>>>>> 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 >>>> >>>> |
Re: Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
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 <[hidden email]>: >>>>> 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" <[hidden email]> >>>>>> >>>>>> Adrian, >>>>>> >>>>>> Yes good idea indeed, I will do that >>>>>> >>>>>> Jacques >>>>>> >>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>> >>>>>>> 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 >>>>> >>>>> > > |
Re: Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
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 <[hidden email]>: >>>>>> 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" <[hidden email]> >>>>>>> >>>>>>> Adrian, >>>>>>> >>>>>>> Yes good idea indeed, I will do that >>>>>>> >>>>>>> Jacques >>>>>>> >>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>> >>>>>>>> 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 >>>>>> >>>>>> |
Re: Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
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 <[hidden email]>: >>>>>>> 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" <[hidden email]> >>>>>>>> >>>>>>>> Adrian, >>>>>>>> >>>>>>>> Yes good idea indeed, I will do that >>>>>>>> >>>>>>>> Jacques >>>>>>>> >>>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>>> >>>>>>>>> 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 >>>>>>> >>>>>>> > > |
Re: Automatic Type Conversion for GenericEntity/Value.set (was Re: Revision 703731)
|
|
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 <[hidden email]>: > 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 <[hidden email]>: >>>>>>>> >>>>>>>> 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" <[hidden email]> >>>>>>>>> >>>>>>>>> Adrian, >>>>>>>>> >>>>>>>>> Yes good idea indeed, I will do that >>>>>>>>> >>>>>>>>> Jacques >>>>>>>>> >>>>>>>>> From: "Adrian Crum" <[hidden email]> >>>>>>>>>> >>>>>>>>>> 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 >>>>>>>> >>>>>>>> >> >> > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |