Skip to content

Commit

Permalink
MONDRIAN - Fixed LER-3097. When setting the measures member context for
Browse files Browse the repository at this point in the history 
       a nonempty crossjoin, exclude calculated measures that reference the
       cross join.  Otherwise, an infinite loop results.

[git-p4: depot-paths = "//open/mondrian/": change = 8257]
  • Loading branch information
Zelaine Fong committed Dec 5, 2006
1 parent 7edf268 commit 1ea633f
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 21 deletions.
4 changes: 3 additions & 1 deletion src/main/mondrian/mdx/NamedSetExpr.java
View file
Expand Up @@ -87,7 +87,9 @@ public boolean dependsOn(Dimension dimension) {
}

public Object accept(MdxVisitor visitor) {
return visitor.visit(this);
Object o = visitor.visit(this);
namedSet.getExp().accept(visitor);
return o;
}

public Type getType() {
Expand Down
126 changes: 110 additions & 16 deletions src/main/mondrian/olap/fun/CrossJoinFunDef.java
View file
Expand Up @@ -17,7 +17,7 @@
import mondrian.resource.MondrianResource;
import mondrian.calc.*;
import mondrian.calc.impl.AbstractListCalc;
import mondrian.mdx.ResolvedFunCall;
import mondrian.mdx.*;
import mondrian.util.Bug;

import java.util.ArrayList;
Expand Down Expand Up @@ -108,7 +108,7 @@ public List evaluateList(Evaluator evaluator) {
}
final List list2 = listCalc2.evaluateList(evaluator.push());
assert oldEval.equals(evaluator) : "listCalc2 changed context";
return crossJoin(list1, list2, evaluator);
return crossJoin(list1, list2, evaluator, call);
}
};
}
Expand All @@ -125,7 +125,12 @@ private ListCalc toList(ExpCompiler compiler, final Exp exp) {
}
}

List crossJoin(List list1, List list2, Evaluator evaluator) {
List crossJoin(
List list1,
List list2,
Evaluator evaluator,
ResolvedFunCall call)
{
if (list1.isEmpty() || list2.isEmpty()) {
return Collections.EMPTY_LIST;
}
Expand All @@ -150,8 +155,8 @@ List crossJoin(List list1, List list2, Evaluator evaluator) {
list1 = nonEmptyListOld(evaluator, list1);
list2 = nonEmptyListOld(evaluator, list2);
} else {
list1 = nonEmptyList(evaluator, list1);
list2 = nonEmptyList(evaluator, list2);
list1 = nonEmptyList(evaluator, list1, call);
list2 = nonEmptyList(evaluator, list2, call);
}
size = (long)list1.size() * (long)list2.size();
// both list1 and list2 may be empty after nonEmpty optimization
Expand Down Expand Up @@ -279,15 +284,27 @@ protected static List nonEmptyListOld(Evaluator evaluator, List list) {


/**
* Visitor which builds a list of all measures referenced in a query.
* Visitor which builds a list of all measures referenced in a query,
* provided the measures don't reference the function call we're trying
* to evaluate for non-emptiness.
*/
static class MeasureVisitor implements mondrian.mdx.MdxVisitor {

// This set is null unless a measure is found.
Set<Member> measureSet;
Set<Member> queryMeasureSet;
MeasureVisitor(Set<Member> queryMeasureSet) {
// measures referencing this call should be excluded from the list
// of measures found
ResolvedFunCall crossJoinCall;

MeasureVisitor(
Set<Member> queryMeasureSet,
ResolvedFunCall crossJoinCall)
{
this.queryMeasureSet = queryMeasureSet;
this.crossJoinCall = crossJoinCall;
}

public Object visit(mondrian.olap.Query query) {
return null;
}
Expand Down Expand Up @@ -322,12 +339,14 @@ public Object visit(mondrian.mdx.MemberExpr memberExpr) {
Member member = memberExpr.getMember();
for (Member measure : queryMeasureSet) {
if (measure.equals(member)) {
if (measureSet == null) {
measureSet = new HashSet<Member>();
}
if (validMeasure(measure)) {
if (measureSet == null) {
measureSet = new HashSet<Member>();
}
//System.out.println("CrossJoinFunDef.MeasureVisitor.visit: measure=" +measure.getUniqueName());
measureSet.add(measure);
break;
measureSet.add(measure);
break;
}
}
}
return null;
Expand All @@ -338,8 +357,77 @@ public Object visit(mondrian.mdx.NamedSetExpr namedSetExpr) {
public Object visit(mondrian.olap.Literal literal) {
return null;
}

/**
* Determines if a measure should be added to the set of measures
* that make up the evaluation context for the nonempty cross join.
* It should not be if it is a calculated measure that references
* the cross join, unless the cross join itself also references
* that calculated measure, in which case, we have a recursive call,
* an an exception is thrown.
*
* @param measure measure being examined
*
* @return true if the measure should be added
*/
private boolean validMeasure(Member measure)
{
if (measure.isCalculated()) {
// check if the measure references the crossjoin
Exp measureExp = measure.getExpression();
ResolvedFunCallFinder finder =
new ResolvedFunCallFinder(crossJoinCall);
measureExp.accept(finder);
if (finder.found) {
// check if the arguments to the cross join reference
// the measure
Exp [] args = crossJoinCall.getArgs();
for (int i = 0; i < args.length; i++) {
Set<Member> measureSet = new HashSet<Member>();
measureSet.add(measure);
MeasureVisitor measureFinder =
new MeasureVisitor(measureSet, null);
args[i].accept(measureFinder);
measureSet = measureFinder.measureSet;
if (measureSet != null && measureSet.size() > 0) {
// recursive condition
throw FunUtil.newEvalException(null,
"Infinite loop detected in " +
crossJoinCall.toString());
}
}
return false;
}
}
return true;
}
}

/**
* Visitor class used to locate a resolved function call within an
* expression
*/
private static class ResolvedFunCallFinder
extends MdxVisitorImpl
{
private ResolvedFunCall call;
public boolean found;

public ResolvedFunCallFinder(ResolvedFunCall call)
{
this.call = call;
found = false;
}

public Object visit(ResolvedFunCall funCall)
{
if (funCall == call) {
found = true;
}
return null;
}
}

/**
* What one wants to determine is for each individual Members of the input
* parameter list whether across a slice there is any data. But what data.
Expand All @@ -353,10 +441,15 @@ public Object visit(mondrian.olap.Literal literal) {
* data iterating across all Measures until non-null data is found or the
* end of the Measures is reached.
*
* @param evaluator
* @param list
* @param evaluator evaluator
* @param list list of members being checked for non-emptiness
* @param call the cross join function call
*/
protected static List nonEmptyList(Evaluator evaluator, List list) {
protected static List nonEmptyList(
Evaluator evaluator,
List list,
ResolvedFunCall call)
{
if (list.isEmpty()) {
return list;
}
Expand Down Expand Up @@ -388,7 +481,8 @@ protected static List nonEmptyList(Evaluator evaluator, List list) {
// if the slicer contains a Measure, then the other axes can not
// contain a Measure, so look at slicer axis first
if (queryMeasureSet.size() > 0) {
MeasureVisitor visitor = new MeasureVisitor(queryMeasureSet);
MeasureVisitor visitor =
new MeasureVisitor(queryMeasureSet, call);
QueryAxis[] axes = query.getAxes();
QueryAxis slicerAxis = query.getSlicerAxis();
if (slicerAxis != null) {
Expand Down
4 changes: 2 additions & 2 deletions src/main/mondrian/olap/fun/NonEmptyCrossJoinFunDef.java
View file
Expand Up @@ -61,10 +61,10 @@ public List evaluateList(Evaluator evaluator) {
// evaluate the arguments in non empty mode
evaluator = evaluator.push();
evaluator.setNonEmpty(true);
List result = crossJoin(list1, list2, evaluator);
List result = crossJoin(list1, list2, evaluator, call);

// remove any remaining empty crossings from the result
result = nonEmptyList(evaluator, result);
result = nonEmptyList(evaluator, result, call);
return result;
}

Expand Down
59 changes: 59 additions & 0 deletions testsrc/main/mondrian/rolap/NonEmptyTest.java
View file
Expand Up @@ -1286,6 +1286,65 @@ public void testCrossJoinSetWithUniqueLevel()
"on rows from Sales");
}

public void testCrossJoinEvaluatorContext()
{
// This test ensures that the proper measure members context is
// set when evaluating a non-empty cross join. The context should
// not include the calculated measure [*TOP_BOTTOM_SET]. If it
// does, the query will result in an infinite loop because the cross
// join will try evaluating the calculated member (when it shouldn't)
// and the calculated member references the cross join, resulting
// in the loop
assertQueryReturns(
"With " +
"Set [*NATIVE_CJ_SET] as " +
"'NonEmptyCrossJoin([*BASE_MEMBERS_Store], [*BASE_MEMBERS_Products])' " +
"Set [*TOP_BOTTOM_SET] as " +
"'Order([*GENERATED_MEMBERS_Store], ([Measures].[Unit Sales], " +
"[Product].[All Products].[*TOP_BOTTOM_MEMBER]), BDESC)' " +
"Set [*BASE_MEMBERS_Store] as '[Store].members' " +
"Set [*GENERATED_MEMBERS_Store] as 'Generate([*NATIVE_CJ_SET], {[Store].CurrentMember})' " +
"Set [*BASE_MEMBERS_Products] as " +
"'{[Product].[All Products].[Food], [Product].[All Products].[Drink], " +
"[Product].[All Products].[Non-Consumable]}' " +
"Set [*GENERATED_MEMBERS_Products] as " +
"'Generate([*NATIVE_CJ_SET], {[Product].CurrentMember})' " +
"Member [Product].[All Products].[*TOP_BOTTOM_MEMBER] as " +
"'Aggregate([*GENERATED_MEMBERS_Products])'" +
"Member [Measures].[*TOP_BOTTOM_MEMBER] as 'Rank([Store].CurrentMember,[*TOP_BOTTOM_SET])' " +
"Member [Store].[All Stores].[*SUBTOTAL_MEMBER_SEL~SUM] as " +
"'sum(Filter([*GENERATED_MEMBERS_Store], [Measures].[*TOP_BOTTOM_MEMBER] <= 10))'" +
"Select {[Measures].[Store Cost]} on columns, " +
"Non Empty Filter(Generate([*NATIVE_CJ_SET], {([Store].CurrentMember)}), " +
"[Measures].[*TOP_BOTTOM_MEMBER] <= 10) on rows From [Sales]",

fold(
"Axis #0:\n" +
"{}\n" +
"Axis #1:\n" +
"{[Measures].[Store Cost]}\n" +
"Axis #2:\n" +
"{[Store].[All Stores]}\n" +
"{[Store].[All Stores].[USA]}\n" +
"{[Store].[All Stores].[USA].[CA]}\n" +
"{[Store].[All Stores].[USA].[OR]}\n" +
"{[Store].[All Stores].[USA].[OR].[Portland]}\n" +
"{[Store].[All Stores].[USA].[OR].[Salem]}\n" +
"{[Store].[All Stores].[USA].[OR].[Salem].[Store 13]}\n" +
"{[Store].[All Stores].[USA].[WA]}\n" +
"{[Store].[All Stores].[USA].[WA].[Tacoma]}\n" +
"{[Store].[All Stores].[USA].[WA].[Tacoma].[Store 17]}\n" +
"Row #0: 225,627.23\n" +
"Row #1: 225,627.23\n" +
"Row #2: 63,530.43\n" +
"Row #3: 56,772.50\n" +
"Row #4: 21,948.94\n" +
"Row #5: 34,823.56\n" +
"Row #6: 34,823.56\n" +
"Row #7: 105,324.31\n" +
"Row #8: 29,959.28\n" +
"Row #9: 29,959.28\n"));
}

/**
* make sure the following is not run natively
Expand Down
2 changes: 0 additions & 2 deletions testsrc/main/mondrian/rolap/VirtualCubeTest.java
View file
Expand Up @@ -451,8 +451,6 @@ public void testAllMeasureMembers()
{
// result should exclude measures that are not explicitly defined
// in the virtual cube (e.g., [Profit last Period])
executeQuery(
"select {[Measures].allMembers} on columns from [Warehouse and Sales");
assertQueryReturns(
"select\n" +
"{[Measures].allMembers} on columns\n" +
Expand Down

0 comments on commit 1ea633f

Please sign in to comment.