I was thinking that there may not be too many of these, but after
playing around with it I now know I was wrong!
There will be quite a few, many caused by the Double/BigDecimal issue,
but probably many others that are related to a String being passed in
where another object is needed (ie
/08, David E Jones [EMAIL PROTECTED] wrote:
From: David E Jones [EMAIL PROTECTED]
Subject: Re: Automatic Type Conversion for GenericEntity/Value.set (was Re:
Revision 703731)
To: dev@ofbiz.apache.org
Date: Sunday, October 19, 2008, 1:54 AM
I was thinking that there may not be too many
From: David E Jones [EMAIL PROTECTED]
I was thinking that there may not be too many of these, but after
playing around with it I now know I was wrong!
There will be quite a few, many caused by the Double/BigDecimal issue,
but probably many others that are related to a String being passed in
I spent some time looking through the entity condition code and I don't
think it is possible to set up a mismatched data type warning in the log.
Here's what I'm thinking:
1. Add a method to EntityUtil that will accept a Map, a
GenericDelegator, an entity name, a Locale, and a TimeZone. The
Maybe even easier, keep the specific log on a reasonnable period. Then remove duplicate log lines and fix in code. This will not
ensure no points are missing but it should help
Jacques
From: Jacques Le Roux [EMAIL PROTECTED]
Yes, I suggested to create a separate log for that (easier) and to
Yes, I suggested to create a separate log for that (easier) and to check it
daily on the demo server
Jacques
From: Adrian Crum [EMAIL PROTECTED]
We could add a warning message to the EntityCondition classes (like the
one on line 411 of GenericEntity.java), then start checking the logs for
On Oct 13, 2008, at 10:10 PM, Scott Gray wrote:
Inline:
2008/10/14 David E Jones [EMAIL PROTECTED]:
Scott, are you referring to the call to PreparedStatement.setString
on
SQLProcessor line 553? In other words, the difference in the
Postgres JDBC
driver is that they used to auto-convert
Ok I see your point, thanks for explaining.
#1 makes sense but it will be interesting to see how many bugs are
created, also doesn't the service engine currently perform automatic
conversion without any problems?
Regards
Scott
2008/10/14 David E Jones [EMAIL PROTECTED]:
On Oct 13, 2008, at
I wouldn't say that bugs would be created by this change, more than
existing bugs would be exposed (that are only visible on certain
databases, etc).
The service engine doesn't actually do automated mapping, is the
service event handler that does this for web requests before the
actual
David E Jones wrote:
What I'm proposing is to fix this further up the stack in the
GenericEntity.set method instead of lower down. I guess either way we
have the two options I mentioned earlier:
1. fail fast(er) by having OFBiz throw an exception
2. try to auto-convert
#1 will result, as
From: Adrian Crum [EMAIL PROTECTED]
David E Jones wrote:
What I'm proposing is to fix this further up the stack in the GenericEntity.set method instead of lower down. I guess either way
we have the two options I mentioned earlier:
1. fail fast(er) by having OFBiz throw an exception
2. try to
I mean in a specific log
Please let me know if this makes sense
From: Jacques Le Roux [EMAIL PROTECTED]
Maybe we could add an interrupting tool (at different code points if needed) on the demo server which would catch and put in log
all points needed to be changed before really changing them ?
Ok it makes more sense to me now, conversion needs to take place at a
level where the relevant information is still available so doing it on
a low level isn't really an option (even though that's been allowed to
happen up until now). I guess I was having trouble understanding why
we needed to do
Scott Gray wrote:
The service event handler, that's what I meant :-)
The guess the point I was trying to make was that with so many
services being called via the event handler aren't we making a pretty
huge exception to the rule that we're trying to enforce? The
performFind service would need
In addition, the service event handler has the information needed to
perform a proper conversion (locale and time zone). The entity engine
does not.
-Adrian
David E Jones wrote:
I wouldn't say that bugs would be created by this change, more than
existing bugs would be exposed (that are
Scott Gray wrote:
Ok it makes more sense to me now, conversion needs to take place at a
level where the relevant information is still available so doing it on
a low level isn't really an option (even though that's been allowed to
happen up until now). I guess I was having trouble understanding
Thanks Adrian, I understand the implications now.
So what's the next step, should we add type checking to the
EntityConditions and then perhaps only enforce the correct types on
some volunteer machines until we can fix a good portion of the problem
code and then move it to the trunk?
Regards
We could add a warning message to the EntityCondition classes (like the
one on line 411 of GenericEntity.java), then start checking the logs for
the warning messages.
-Adrian
Scott Gray wrote:
Thanks Adrian, I understand the implications now.
So what's the next step, should we add type
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 [EMAIL PROTECTED]
Hi Scott,
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
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 [EMAIL PROTECTED]:
Sorry forget it, I was wrong, I did the type=Timestamp removing in the
wrong screen !
So yes I was able to reproduce
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
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
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 [EMAIL PROTECTED]:
I suppose it was a problem with my Postgres installation since with a new
Postgres
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 [EMAIL PROTECTED]
Hi Jacques
I really don't think it
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
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
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
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,
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
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
Scott, are you referring to the call to PreparedStatement.setString on
SQLProcessor line 553? In other words, the difference in the Postgres
JDBC driver is that they used to auto-convert from a String to
whatever the database needs in the setString method, but now they don't?
What I'm
Inline:
2008/10/14 David E Jones [EMAIL PROTECTED]:
Scott, are you referring to the call to PreparedStatement.setString on
SQLProcessor line 553? In other words, the difference in the Postgres JDBC
driver is that they used to auto-convert from a String to whatever the
database needs in the
Adrian,
Yes good idea indeed, I will do that
Jacques
From: Adrian Crum [EMAIL PROTECTED]
Jacques,
Instead of modifying the groovy files, try specifying the data type in the
screen widget.
Example in ReportFinancialSummaryScreens.xml:
set field=fromDate from-field=parameters.fromDate
Done in revision: 703816
It was not possible for PackingSlip.groovy and FindInventoryEventPlan.groovy.
Because there the date string is build dynamically in the Groovy file
Jacques
From: Jacques Le Roux [EMAIL PROTECTED]
Adrian,
Yes good idea indeed, I will do that
Jacques
From: Adrian
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
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
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
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
39 matches
Mail list logo
