Slim-down continue [was Re: svn commit: r1340414 - ...]

[Adrian Crum-3]

On 5/30/2012 6:06 PM, Adam Heath wrote:

> On 05/30/2012 11:47 AM, Adrian Crum wrote:
>> On 5/30/2012 4:30 PM, Adam Heath wrote:
>>> On 05/30/2012 02:46 AM, Adrian Crum wrote:
>>>> On 5/29/2012 5:49 AM, Adam Heath wrote:
>>>>> On 05/25/2012 01:22 AM, Adam Heath wrote:
>>>>>> On 05/25/2012 12:26 AM, Jacques Le Roux wrote:
>>>>>>> From: "Jacques Le Roux"<[hidden email]>
>>>>>>>> From: "Adam Heath"<[hidden email]>
>>>>>>>>> On 05/24/2012 04:05 PM, Jacques Le Roux wrote:
>>>>>>>>>> From: "Adam Heath"<[hidden email]>
>>>>>>>>>>> On 05/24/2012 10:18 AM, Adam Heath wrote:
>>>>>>>>>>> The idea was that you would parse the sqlCondition once, in a
>>>>>>>>>>> static
>>>>>>>>>>> {} block somewhere, then at runtime, just build that map. This
>>>>>>>>>>> builds-upon the map/list idea inherent in ofbiz.
>>>>>>>>>>>
>>>>>>>>>>> I also had plans that you could store sql strings into a
>>>>>>>>>>> properties
>>>>>>>>>>> file somewhere, so that they could possibly be changed by the
>>>>>>>>>>> end-user
>>>>>>>>>>> without having to recompile.
>>>>>>>>>>>
>>>>>>>>>>> I need to revisit the "SELECT a.partyId, b.* EXCLUDE(partyId)
>>>>>>>>>>> FROM
>>>>>>>>>>> Party a LEFT JOIN PartyContactMech b USING (partyId)", now
>>>>>>>>>>> that ofbiz
>>>>>>>>>>> better supports conditions on the join clauses, esp. when
>>>>>>>>>>> combining
>>>>>>>>>>> views into other views.
>>>>>>>>>> Thanks for the explanation,
>>>>>>>>>>
>>>>>>>>>> So should we not rather create a Jira with all the needed in a
>>>>>>>>>> patch
>>>>>>>>>> until this is finished?
>>>>>>>>>> Or maybe a branch would be easier?
>>>>>>>>>>
>>>>>>>>>> Still with the slim-down idea in mind and as objective.
>>>>>>>>> I like the slimdown, but tbh, I would like to see the
>>>>>>>>> framework/sql
>>>>>>>>> stuff used more than it is(0 right now). Andrew Zeneski was an
>>>>>>>>> original requestor for something that parsed sql into
>>>>>>>>> EntityCondition. I took his suggestion, but went further, to
>>>>>>>>> allow
>>>>>>>>> CREATE VIEW AS SELECT to work.
>>>>>>>>>
>>>>>>>>> I've noticed that there aren't that many view definitions in
>>>>>>>>> ofbiz.
>>>>>>>>> As I've been deprecating all this code recently, I've noticed
>>>>>>>>> java
>>>>>>>>> code doing nested loop kinda-stuff, instead of just doing a
>>>>>>>>> view. I'm
>>>>>>>>> guessing because view-xml is verbose and not how people actually
>>>>>>>>> think.
>>>>>>>>>
>>>>>>>>> However, with what I committed, you can define the view using
>>>>>>>>> a SQL
>>>>>>>>> syntax, which is then backed by a DynamicViewEntity.
>>>>>>>>>
>>>>>>>>> I've seen rather impressive speedups just rewriting code to a
>>>>>>>>> single
>>>>>>>>> SQL query, instead of java loops; the database can be rather
>>>>>>>>> efficient. So making view writing simpler is a laudable goal.
>>>>>>>> Great, but still, why not a branch as long as it's not finished?
>>>>>>>>
>>>>>>>> Also something which I think is pretty neat in the principle (I
>>>>>>>> still
>>>>>>>> did not review the code) and would speed up views:
>>>>>>>> https://issues.apache.org/jira/browse/OFBIZ-4041
>>>>>>>>
>>>>>>>> Jacques
>>>>>>> BTW another stuff that could be part of this branch
>>>>>>> https://issues.apache.org/jira/browse/OFBIZ-3575
>>>>>> Ok, I suppose. This weekend I'll create such a branch to
>>>>>> fix/improve the view system. This will also attempt to fix the
>>>>>> reverse
>>>>>> cache clearing issues.
>>>>> branches/improved-entityengine-features-20120528
>>>>>
>>>>> Also see 1343540, which adds a README that has some things that we
>>>>> might want to implement in the branch.
>>>> Adam,
>>>>
>>>> I commented on the commit, but you didn't reply, so I will try again
>>>> in this thread.
>>> I saw, but didn't really think it needed a comment.  I guess I should
>>> have given an affirmative response, instead of no response.  I assumed
>>> that no response is not negative(yeah, that's a double negative).

Non-communication is not communication. ;)

>>>
>>>> If you don't mind, I would like to clean up the EntityConditionVisitor
>>>> interface so it looks more like a conventional visitor pattern.
>>> I'm down to whatever other people for, within reason.  Tbh, I'm
>>> currently tweaking EntityFieldValue, which is used by
>>> ModelViewEntity.ViewEntityCondition, in a hackish way; it means I'm
>>> having to tweak the visitor pattern, which sucks.
>>>
>>> I'm the one who added the visitor pattern all those ages ago(goes back
>>> to svn.ofbiz.org timeframe); I'd be fine with ripping it completely
>>> out, as we(brainfood) don't use it anywhere.
>> I need to reacquaint myself with the entity engine code. I was
>> thinking the visitor pattern could be used to construct the SQL string
>> instead of the complicated if-then-else code spread across a number of
>> classes. We could use different visitors for different databases.
> Yes, that was my original thought.  However, I don't think it's that
> simple anymore.  I've got a much better understanding of the entity
> system since I wrote the visitor stuff.
>
> I'd actually like to see my generic sql code be used to represent
> entitymodel, then add sql<->sql conversion code.  I have an xslt that
> can read *all* entitymodel.xml and convert it to entitymodel.sql, or
> entitymodel.java.  I used the former to verify that my sql parsing was
> featureful enough.

You lost me. How will this look when it is finished? Will the entity
model XML files be replaced with SQL files?

>
>>  From my perspective, the entity engine implementation seems a bit
>> tangled, and I was trying to come up with a strategy to simplify things.
> It's not too bad, imho; just has some issues with view abstraction
> that can't be currently represented.  It's getting much closer to not
> having to do raw sql anymore thru jdbc.
>
>>>> Also, I was wondering if we could add some timing metrics to the
>>>> entity engine. Maybe keep an average query time per entity, and throw
>>>> an exception when the average exceeds a configurable threshold. This
>>>> would facilitate server overload management.
>>> There is already code that does that, when a query takes a long time.
>> I don't see any code that does that.
> I've seen lines in the log file where queries take too long.
> GenericDAO.selectListIteratorByCondition() has an example for that,
> but it's not done in select().

Thanks. That code simply logs the time it took to execute a query. I'm
proposing code that will monitor queries and provide feedback to a
server management process.

[Adam Heath-2]

On 05/30/2012 12:22 PM, Adrian Crum wrote:

>
>>> I need to reacquaint myself with the entity engine code. I was
>>> thinking the visitor pattern could be used to construct the SQL string
>>> instead of the complicated if-then-else code spread across a number of
>>> classes. We could use different visitors for different databases.
>> Yes, that was my original thought.  However, I don't think it's that
>> simple anymore.  I've got a much better understanding of the entity
>> system since I wrote the visitor stuff.
>>
>> I'd actually like to see my generic sql code be used to represent
>> entitymodel, then add sql<->sql conversion code.  I have an xslt that
>> can read *all* entitymodel.xml and convert it to entitymodel.sql, or
>> entitymodel.java.  I used the former to verify that my sql parsing was
>> featureful enough.
>
>
> You lost me. How will this look when it is finished? Will the entity
> model XML files be replaced with SQL files?

No.  I just used the xslt to verify that the actual sql parsing code,
and the built-up object graph, were capable of representing everything
mentioned in entitymodel.xml.

>>>>> Also, I was wondering if we could add some timing metrics to the
>>>>> entity engine. Maybe keep an average query time per entity, and
>>>>> throw
>>>>> an exception when the average exceeds a configurable threshold. This
>>>>> would facilitate server overload management.
>>>> There is already code that does that, when a query takes a long time.
>>> I don't see any code that does that.
>> I've seen lines in the log file where queries take too long.
>> GenericDAO.selectListIteratorByCondition() has an example for that,
>> but it's not done in select().
>
>
> Thanks. That code simply logs the time it took to execute a query. I'm
> proposing code that will monitor queries and provide feedback to a
> server management process.

Sure.  The quickly typed example class I gave would help in that.

[Adrian Crum-3]

On 5/30/2012 6:29 PM, Adam Heath wrote:

> On 05/30/2012 12:22 PM, Adrian Crum wrote:
>>>> I need to reacquaint myself with the entity engine code. I was
>>>> thinking the visitor pattern could be used to construct the SQL string
>>>> instead of the complicated if-then-else code spread across a number of
>>>> classes. We could use different visitors for different databases.
>>> Yes, that was my original thought.  However, I don't think it's that
>>> simple anymore.  I've got a much better understanding of the entity
>>> system since I wrote the visitor stuff.
>>>
>>> I'd actually like to see my generic sql code be used to represent
>>> entitymodel, then add sql<->sql conversion code.  I have an xslt that
>>> can read *all* entitymodel.xml and convert it to entitymodel.sql, or
>>> entitymodel.java.  I used the former to verify that my sql parsing was
>>> featureful enough.
>>
>> You lost me. How will this look when it is finished? Will the entity
>> model XML files be replaced with SQL files?
> No.  I just used the xslt to verify that the actual sql parsing code,
> and the built-up object graph, were capable of representing everything
> mentioned in entitymodel.xml.
>
>>>>>> Also, I was wondering if we could add some timing metrics to the
>>>>>> entity engine. Maybe keep an average query time per entity, and
>>>>>> throw
>>>>>> an exception when the average exceeds a configurable threshold. This
>>>>>> would facilitate server overload management.
>>>>> There is already code that does that, when a query takes a long time.
>>>> I don't see any code that does that.
>>> I've seen lines in the log file where queries take too long.
>>> GenericDAO.selectListIteratorByCondition() has an example for that,
>>> but it's not done in select().
>>
>> Thanks. That code simply logs the time it took to execute a query. I'm
>> proposing code that will monitor queries and provide feedback to a
>> server management process.
> Sure.  The quickly typed example class I gave would help in that.

Yes, that will help. I'm also looking at the Sandstorm (SEDA)
implementation for ideas.

[Jacques Le Roux]

Adrian Crum wrote:

> On 5/30/2012 6:06 PM, Adam Heath wrote:
>> On 05/30/2012 11:47 AM, Adrian Crum wrote:
>>> On 5/30/2012 4:30 PM, Adam Heath wrote:
>>>> On 05/30/2012 02:46 AM, Adrian Crum wrote:
>>>>> On 5/29/2012 5:49 AM, Adam Heath wrote:
>>>>>> On 05/25/2012 01:22 AM, Adam Heath wrote:
>>>>>>> On 05/25/2012 12:26 AM, Jacques Le Roux wrote:
>>>>>>>> From: "Jacques Le Roux"<[hidden email]>
>>>>>>>>> From: "Adam Heath"<[hidden email]>
>>>>>>>>>> On 05/24/2012 04:05 PM, Jacques Le Roux wrote:
>>>>>>>>>>> From: "Adam Heath"<[hidden email]>
>>>>>>>>>>>> On 05/24/2012 10:18 AM, Adam Heath wrote:
>>>>>>>>>>>> The idea was that you would parse the sqlCondition once, in a
>>>>>>>>>>>> static
>>>>>>>>>>>> {} block somewhere, then at runtime, just build that map. This
>>>>>>>>>>>> builds-upon the map/list idea inherent in ofbiz.
>>>>>>>>>>>>
>>>>>>>>>>>> I also had plans that you could store sql strings into a
>>>>>>>>>>>> properties
>>>>>>>>>>>> file somewhere, so that they could possibly be changed by the
>>>>>>>>>>>> end-user
>>>>>>>>>>>> without having to recompile.
>>>>>>>>>>>>
>>>>>>>>>>>> I need to revisit the "SELECT a.partyId, b.* EXCLUDE(partyId)
>>>>>>>>>>>> FROM
>>>>>>>>>>>> Party a LEFT JOIN PartyContactMech b USING (partyId)", now
>>>>>>>>>>>> that ofbiz
>>>>>>>>>>>> better supports conditions on the join clauses, esp. when
>>>>>>>>>>>> combining
>>>>>>>>>>>> views into other views.
>>>>>>>>>>> Thanks for the explanation,
>>>>>>>>>>>
>>>>>>>>>>> So should we not rather create a Jira with all the needed in a
>>>>>>>>>>> patch
>>>>>>>>>>> until this is finished?
>>>>>>>>>>> Or maybe a branch would be easier?
>>>>>>>>>>>
>>>>>>>>>>> Still with the slim-down idea in mind and as objective.
>>>>>>>>>> I like the slimdown, but tbh, I would like to see the
>>>>>>>>>> framework/sql
>>>>>>>>>> stuff used more than it is(0 right now). Andrew Zeneski was an
>>>>>>>>>> original requestor for something that parsed sql into
>>>>>>>>>> EntityCondition. I took his suggestion, but went further, to
>>>>>>>>>> allow
>>>>>>>>>> CREATE VIEW AS SELECT to work.
>>>>>>>>>>
>>>>>>>>>> I've noticed that there aren't that many view definitions in
>>>>>>>>>> ofbiz.
>>>>>>>>>> As I've been deprecating all this code recently, I've noticed
>>>>>>>>>> java
>>>>>>>>>> code doing nested loop kinda-stuff, instead of just doing a
>>>>>>>>>> view. I'm
>>>>>>>>>> guessing because view-xml is verbose and not how people actually
>>>>>>>>>> think.
>>>>>>>>>>
>>>>>>>>>> However, with what I committed, you can define the view using
>>>>>>>>>> a SQL
>>>>>>>>>> syntax, which is then backed by a DynamicViewEntity.
>>>>>>>>>>
>>>>>>>>>> I've seen rather impressive speedups just rewriting code to a
>>>>>>>>>> single
>>>>>>>>>> SQL query, instead of java loops; the database can be rather
>>>>>>>>>> efficient. So making view writing simpler is a laudable goal.
>>>>>>>>> Great, but still, why not a branch as long as it's not finished?
>>>>>>>>>
>>>>>>>>> Also something which I think is pretty neat in the principle (I
>>>>>>>>> still
>>>>>>>>> did not review the code) and would speed up views:
>>>>>>>>> https://issues.apache.org/jira/browse/OFBIZ-4041
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>> BTW another stuff that could be part of this branch
>>>>>>>> https://issues.apache.org/jira/browse/OFBIZ-3575
>>>>>>> Ok, I suppose. This weekend I'll create such a branch to
>>>>>>> fix/improve the view system. This will also attempt to fix the
>>>>>>> reverse
>>>>>>> cache clearing issues.
>>>>>> branches/improved-entityengine-features-20120528
>>>>>>
>>>>>> Also see 1343540, which adds a README that has some things that we
>>>>>> might want to implement in the branch.
>>>>> Adam,
>>>>>
>>>>> I commented on the commit, but you didn't reply, so I will try again
>>>>> in this thread.
>>>> I saw, but didn't really think it needed a comment.  I guess I should
>>>> have given an affirmative response, instead of no response.  I assumed
>>>> that no response is not negative(yeah, that's a double negative).
>
>
> Non-communication is not communication. ;)

Especially when you ask

Jacques
 

>
>>>>
>>>>> If you don't mind, I would like to clean up the EntityConditionVisitor
>>>>> interface so it looks more like a conventional visitor pattern.
>>>> I'm down to whatever other people for, within reason.  Tbh, I'm
>>>> currently tweaking EntityFieldValue, which is used by
>>>> ModelViewEntity.ViewEntityCondition, in a hackish way; it means I'm
>>>> having to tweak the visitor pattern, which sucks.
>>>>
>>>> I'm the one who added the visitor pattern all those ages ago(goes back
>>>> to svn.ofbiz.org timeframe); I'd be fine with ripping it completely
>>>> out, as we(brainfood) don't use it anywhere.
>>> I need to reacquaint myself with the entity engine code. I was
>>> thinking the visitor pattern could be used to construct the SQL string
>>> instead of the complicated if-then-else code spread across a number of
>>> classes. We could use different visitors for different databases.
>> Yes, that was my original thought.  However, I don't think it's that
>> simple anymore.  I've got a much better understanding of the entity
>> system since I wrote the visitor stuff.
>>
>> I'd actually like to see my generic sql code be used to represent
>> entitymodel, then add sql<->sql conversion code.  I have an xslt that
>> can read *all* entitymodel.xml and convert it to entitymodel.sql, or
>> entitymodel.java.  I used the former to verify that my sql parsing was
>> featureful enough.
>
>
> You lost me. How will this look when it is finished? Will the entity
> model XML files be replaced with SQL files?
>
>
>>
>>>  From my perspective, the entity engine implementation seems a bit
>>> tangled, and I was trying to come up with a strategy to simplify things.
>> It's not too bad, imho; just has some issues with view abstraction
>> that can't be currently represented.  It's getting much closer to not
>> having to do raw sql anymore thru jdbc.
>>
>>>>> Also, I was wondering if we could add some timing metrics to the
>>>>> entity engine. Maybe keep an average query time per entity, and throw
>>>>> an exception when the average exceeds a configurable threshold. This
>>>>> would facilitate server overload management.
>>>> There is already code that does that, when a query takes a long time.
>>> I don't see any code that does that.
>> I've seen lines in the log file where queries take too long.
>> GenericDAO.selectListIteratorByCondition() has an example for that,
>> but it's not done in select().
>
>
> Thanks. That code simply logs the time it took to execute a query. I'm
> proposing code that will monitor queries and provide feedback to a
> server management process.