[Mondrian] RE: Eigenbase perforce change 13092 for review

Kurtis.Walker at thomsonreuters.com Kurtis.Walker at thomsonreuters.com
Wed Nov 4 13:20:53 EST 2009


Hi Julian,
   Sorry to resurrect an old thread.  The workaround we came up with for
this issue has proven to be insufficient for a couple of our
implementations of MdxVisitor that we have outside of the Mondrian code
base.  So, we've come up with an alternative.
  We'd like to create a boolean method on MdxVisitor that indicates
whether or not the visitee should visit its children.  The current
default implementation, MdxVisitorImpl, would return true.  Then, we
would create a second base implementation that returns false and manages
the child visits within the visitor.  This allows for the least amount
of code change while supporting both variations on the visitor pattern.
Let us know if you have any concerns.  Thanks!

Kurt

-----Original Message-----
From: mondrian-bounces at pentaho.org [mailto:mondrian-bounces at pentaho.org]
On Behalf Of Walker, Kurtis (Healthcare USA)
Sent: Wednesday, October 14, 2009 8:55 AM
To: jhyde at pentaho.com; mondrian at pentaho.org; Campbell, Matthew
(Healthcare USA)
Subject: RE: [Mondrian] RE: Eigenbase perforce change 13092 for review

I'll back it out.  I think we can accomplish the same thing by keeping a
list of fun calls that we have visited, so that we only visit each one
once.

Kurt

-----Original Message-----
From: Julian Hyde [mailto:jhyde at pentaho.com] 
Sent: Tuesday, October 13, 2009 9:49 PM
To: Walker, Kurtis (Healthcare USA); mondrian at pentaho.org; Campbell,
Matthew (Healthcare USA)
Subject: RE: [Mondrian] RE: Eigenbase perforce change 13092 for review

Well, it's a big change moving the iteration from the visitee to the
visitor, and it is bound to introduce bugs. You did introduce one bug --
you
changed ResolvedFunCall but you didn't change UnresolvedFunCall. As I
said
before it's non-standard -- the convention is for the visitee to iterate
over children. So other people using the visitor are likely to be
confused.

What do you want to do about this? Is there a way to convert it back to
the
old style, and say throw an exception to abort the iteration. Or if we
leave
it this way, you should add more doc to the visitor and fix the bug you
introduced.

Julian







> -----Original Message-----
> From: Kurtis.Walker at thomsonreuters.com 
> [mailto:Kurtis.Walker at thomsonreuters.com] 
> Sent: Tuesday, October 13, 2009 12:25 PM
> To: jhyde at pentaho.com; mondrian at pentaho.org; 
> matthew.campbell at thomsonreuters.com
> Subject: RE: [Mondrian] RE: Eigenbase perforce change 13092 for review
> 
> Hi Julian,
>    Before we made the change of moving the iteration out of 
> the visitee
> and into the MdxVisitorImpl, we experienced the same performance issue
> that you have mentioned.  We override the visit of ResolvedFunCall in
> our new visitor and make decisions based on the result of 
> visiting each
> of the children.  So, the iteration in the visitee object was pure
> wasted processing when using our new visitor.  The change allowed the
> other MdxVisitors(except RolapCell.DrillThroughVisitor) to remain
> unchanged while keeping the same functionality.  
> 
> Kurt
> 
> -----Original Message-----
> From: mondrian-bounces at pentaho.org 
> [mailto:mondrian-bounces at pentaho.org]
> On Behalf Of Julian Hyde
> Sent: Tuesday, October 13, 2009 2:36 PM
> To: Campbell, Matthew (Healthcare USA)
> Cc: 'Mondrian developer mailing list'
> Subject: [Mondrian] RE: Eigenbase perforce change 13092 for review
> 
> I don't know the cause of the build breakage, but this change is of
> concern:
> 
> > ==== 
> > //open/mondrian/src/main/mondrian/mdx/MdxVisitorImpl.java#4 
> > (ktext) ====
> > 
> > 41a42,45
> > >         // visit the call's arguments
> > >         for (Exp arg : call.getArgs()) {
> > >             arg.accept(this);
> > >         }
> 
> In bug http://jira.pentaho.com/browse/MONDRIAN-608 both the 
> visitor and
> the
> visitee were iterating over children. Exponentially bad performance.
> This
> looks very similar.
> 
> Conventionally in the visitor pattern, it is the visitee that iterates
> over
> children (in the accept method) not the visitor (in the visit method).
> 
> Julian
> 
> _______________________________________________
> Mondrian mailing list
> Mondrian at pentaho.org
> http://lists.pentaho.org/mailman/listinfo/mondrian
> 
> 


_______________________________________________
Mondrian mailing list
Mondrian at pentaho.org
http://lists.pentaho.org/mailman/listinfo/mondrian




More information about the Mondrian mailing list