Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix for MONDRIAN-1197. RolapMemberBase was comparing members with nul…
…l keys by unique name, which only made sense for VisualTotalsMember but would create problems with other normal members. This fix makes it so that null keys are always equal, but VisualTotalsMember overrides the compareTo(Object) method so the delegate member is compared. This is in line with other parts of the code, like RolapCubeMember.
  • Loading branch information
lucboudreau committed Sep 4, 2012
1 parent 3c5d6b7 commit 2eb75a2
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 23 deletions.
15 changes: 15 additions & 0 deletions src/main/mondrian/olap/fun/VisualTotalsFunDef.java
Expand Up @@ -225,6 +225,18 @@ public boolean equals(Object o) {
&& this.member.equals(o);
}

@Override
public int compareTo(Object o) {
if (o instanceof VisualTotalMember) {
// VisualTotals members are a special case. We have
// to compare the delegate member.
return this.getMember().compareTo(
((VisualTotalMember) o).getMember());
} else {
return super.compareTo(o);
}
}

@Override
public int hashCode() {
return member.hashCode();
Expand All @@ -241,6 +253,9 @@ protected boolean computeCalculated(final MemberType memberType) {

public int getSolveOrder() {
// high solve order, so it is expanded after other calculations
// REVIEW: 99...really?? I've seen many queries with higher SO.
// I don't think we should be abusing arbitrary constants
// like this.
return 99;
}

Expand Down
16 changes: 6 additions & 10 deletions src/main/mondrian/rolap/RolapMemberBase.java
Expand Up @@ -512,20 +512,16 @@ public Object getKey() {
public int compareTo(Object o) {
RolapMemberBase other = (RolapMemberBase)o;
if (this.key != null && other.key == null) {
return 1; // not null is greater than null
// not null is greater than null
return 1;
}
if (this.key == null && other.key != null) {
return -1; // null is less than not null
// null is less than not null
return -1;
}
// compare by unique name, if both keys are null
if (this.key == null && other.key == null) {
return this.getUniqueName().compareTo(other.getUniqueName());
}
// compare by unique name, if one ore both members are null
if (this.key == RolapUtil.sqlNullValue
|| other.key == RolapUtil.sqlNullValue)
{
return this.getUniqueName().compareTo(other.getUniqueName());
// if both keys are null, they are equal.
return 0;
}
// as both keys are not null, compare by key
// String, Double, Integer should be possible
Expand Down
4 changes: 1 addition & 3 deletions src/main/mondrian/spi/SegmentCache.java
Expand Up @@ -69,9 +69,7 @@ public interface SegmentCache {
* segment corresponding to the header could be found.
*
* <p>Cache implementations are at liberty to 'forget' segments. Therefore
* it is allowable for this method to return null at any time, even if
* {@link #contains(SegmentHeader)} for this segment previously returned
* true.</p>
* it is allowable for this method to return null at any time</p>
*
* @param header The header of the segment to find.
* Consider this as a key.
Expand Down
34 changes: 24 additions & 10 deletions testsrc/main/mondrian/test/ParentChildHierarchyTest.java
Expand Up @@ -1327,6 +1327,20 @@ public void testBridgeTable() {
+ "Row #0: 28,435.38\n");
}

/**
* Fix for
* <a href="http://jira.pentaho.com/browse/MONDRIAN-1125">MONDRIAN-1225</a>
*
* <p>When nativizing a set which contained a PC
* hierarchy, the SqlTupleReader would ask the cache for the parent
* member of the member it was populating, but the members are only
* put in cache at the second phase of the tuple computation, once all
* the members have been populated from SQL. Now, the SqlTupleReader
* keeps an intermediate key->member map so it can construct PC
* hierarchies correctly. This map gets picked up by the GC as soon as
* the SQL result set reaches its end and the tuple reader is closed,
* so there are no added cost to this.
*/
public void testPCCacheKeyBug() throws Exception {
final String mdx =
"With\n"
Expand Down Expand Up @@ -1357,26 +1371,26 @@ public void testPCCacheKeyBug() throws Exception {
+ "Axis #1:\n"
+ "{[Measures].[*FORMATTED_MEASURE_0]}\n"
+ "Axis #2:\n"
+ "{[Employees].[Sheri Nowmer], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker].[Jacqueline Wyllie], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker].[Jacqueline Wyllie], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker].[Jacqueline Wyllie].[Ralph Mccoy], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker].[Jacqueline Wyllie], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker].[Jacqueline Wyllie].[Ralph Mccoy], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Pedro Castillo], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Beverly Baker].[Jacqueline Wyllie].[Ralph Mccoy], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Pedro Castillo], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Pedro Castillo].[Jose Bernard], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Pedro Castillo], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Pedro Castillo].[Jose Bernard], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Pedro Castillo].[Jose Bernard], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Mary Solimena], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Mary Solimena], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Mary Solimena].[Matthew Hunter], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Mary Solimena], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Mary Solimena].[Matthew Hunter], [Store].[USA], [Pay Type].[Hourly]}\n"
+ "{[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Mary Solimena].[Matthew Hunter], [Store].[USA], [Pay Type].[Monthly]}\n"
+ "Row #0: 3,996\n"
+ "Row #1: 3,396\n"
+ "Row #2: 3,840\n"
Expand Down
130 changes: 130 additions & 0 deletions testsrc/main/mondrian/test/SteelWheelsSchemaTest.java
Expand Up @@ -583,6 +583,136 @@ public void testEsr1587() {
+ "{[Time].[2005], [Measures].[*ZERO]}\n" + "Row #0: 0\n"
+ "Row #0: 0\n" + "Row #0: 0\n");
}

/**
* Fix for
* <a href="http://jira.pentaho.com/browse/MONDRIAN-1197">MONDRIAN-1197</a>
*
* RolapMemberBase was comparing members with null keys by unique name,
* which only made sense for VisualTotalsMember but would create problems
* with other normal members. This fix makes it so that null keys are
* always equal, but VisualTotalsMember overrides the compareTo(Object)
* method so the delegate member is compared. This is in line with other
* parts of the code, like RolapCubeMember.
*/
public void testMondrian1197() {
final TestContext testContext = getTestContext();
if (!testContext.databaseIsValid()) {
return;
}
final String mdx =
"With\n"
+ "Set [*NATIVE_CJ_SET] as '[*BASE_MEMBERS_Markets]'\n"
+ "Set [*SORTED_ROW_AXIS] as 'Order([*CJ_ROW_AXIS],[Markets].CurrentMember.OrderKey,DESC,Ancestor([Markets].CurrentMember,[Markets].[Country]).OrderKey,DESC)'\n"
+ "Set [*BASE_MEMBERS_Markets] as '[Markets].[State Province].Members'\n"
+ "Set [*BASE_MEMBERS_Measures] as '{[Measures].[*ZERO]}'\n"
+ "Set [*CJ_ROW_AXIS] as 'Generate([*NATIVE_CJ_SET], {([Markets].currentMember)})'\n"
+ "Set [*CJ_COL_AXIS] as '[*NATIVE_CJ_SET]'\n"
+ "Member [Measures].[*ZERO] as '0', SOLVE_ORDER=0\n"
+ "Select\n"
+ "[*BASE_MEMBERS_Measures] on columns,\n"
+ "[*SORTED_ROW_AXIS] on rows\n"
+ "From [SteelWheelsSales]\n";
testContext.assertQueryReturns(
mdx,
"Axis #0:\n"
+ "{}\n"
+ "Axis #1:\n"
+ "{[Measures].[*ZERO]}\n"
+ "Axis #2:\n"
+ "{[Markets].[NA].[USA].[PA]}\n"
+ "{[Markets].[NA].[USA].[NY]}\n"
+ "{[Markets].[NA].[USA].[NV]}\n"
+ "{[Markets].[NA].[USA].[NJ]}\n"
+ "{[Markets].[NA].[USA].[NH]}\n"
+ "{[Markets].[NA].[USA].[MA]}\n"
+ "{[Markets].[NA].[USA].[CT]}\n"
+ "{[Markets].[NA].[USA].[CA]}\n"
+ "{[Markets].[NA].[Canada].[Quu00e9bec]}\n"
+ "{[Markets].[NA].[Canada].[BC]}\n"
+ "{[Markets].[Japan].[Singapore].[#null]}\n"
+ "{[Markets].[Japan].[Philippines].[#null]}\n"
+ "{[Markets].[Japan].[Japan].[Tokyo]}\n"
+ "{[Markets].[Japan].[Japan].[Osaka]}\n"
+ "{[Markets].[Japan].[Hong Kong].[#null]}\n"
+ "{[Markets].[EMEA].[UK].[Isle of Wight]}\n"
+ "{[Markets].[EMEA].[UK].[#null]}\n"
+ "{[Markets].[EMEA].[Switzerland].[#null]}\n"
+ "{[Markets].[EMEA].[Sweden].[#null]}\n"
+ "{[Markets].[EMEA].[Spain].[#null]}\n"
+ "{[Markets].[EMEA].[Norway].[#null]}\n"
+ "{[Markets].[EMEA].[Italy].[#null]}\n"
+ "{[Markets].[EMEA].[Ireland].[#null]}\n"
+ "{[Markets].[EMEA].[Germany].[#null]}\n"
+ "{[Markets].[EMEA].[France].[#null]}\n"
+ "{[Markets].[EMEA].[Finland].[#null]}\n"
+ "{[Markets].[EMEA].[Denmark].[#null]}\n"
+ "{[Markets].[EMEA].[Belgium].[#null]}\n"
+ "{[Markets].[EMEA].[Austria].[#null]}\n"
+ "{[Markets].[APAC].[Singapore].[#null]}\n"
+ "{[Markets].[APAC].[New Zealand].[]}\n"
+ "{[Markets].[APAC].[New Zealand].[#null]}\n"
+ "{[Markets].[APAC].[Australia].[Victoria]}\n"
+ "{[Markets].[APAC].[Australia].[Queensland]}\n"
+ "{[Markets].[APAC].[Australia].[NSW]}\n"
+ "{[Markets].[#null].[Switzerland].[#null]}\n"
+ "{[Markets].[#null].[Spain].[#null]}\n"
+ "{[Markets].[#null].[South Africa].[#null]}\n"
+ "{[Markets].[#null].[Singapore].[#null]}\n"
+ "{[Markets].[#null].[Russia].[#null]}\n"
+ "{[Markets].[#null].[Portugal].[#null]}\n"
+ "{[Markets].[#null].[Poland].[#null]}\n"
+ "{[Markets].[#null].[Netherlands].[#null]}\n"
+ "{[Markets].[#null].[Israel].[#null]}\n"
+ "{[Markets].[#null].[Ireland].[Co. Cork]}\n"
+ "{[Markets].[#null].[Germany].[#null]}\n"
+ "Row #0: 0\n"
+ "Row #1: 0\n"
+ "Row #2: 0\n"
+ "Row #3: 0\n"
+ "Row #4: 0\n"
+ "Row #5: 0\n"
+ "Row #6: 0\n"
+ "Row #7: 0\n"
+ "Row #8: 0\n"
+ "Row #9: 0\n"
+ "Row #10: 0\n"
+ "Row #11: 0\n"
+ "Row #12: 0\n"
+ "Row #13: 0\n"
+ "Row #14: 0\n"
+ "Row #15: 0\n"
+ "Row #16: 0\n"
+ "Row #17: 0\n"
+ "Row #18: 0\n"
+ "Row #19: 0\n"
+ "Row #20: 0\n"
+ "Row #21: 0\n"
+ "Row #22: 0\n"
+ "Row #23: 0\n"
+ "Row #24: 0\n"
+ "Row #25: 0\n"
+ "Row #26: 0\n"
+ "Row #27: 0\n"
+ "Row #28: 0\n"
+ "Row #29: 0\n"
+ "Row #30: 0\n"
+ "Row #31: 0\n"
+ "Row #32: 0\n"
+ "Row #33: 0\n"
+ "Row #34: 0\n"
+ "Row #35: 0\n"
+ "Row #36: 0\n"
+ "Row #37: 0\n"
+ "Row #38: 0\n"
+ "Row #39: 0\n"
+ "Row #40: 0\n"
+ "Row #41: 0\n"
+ "Row #42: 0\n"
+ "Row #43: 0\n"
+ "Row #44: 0\n"
+ "Row #45: 0\n");
}
}

// End SteelWheelsSchemaTest.java

0 comments on commit 2eb75a2

Please sign in to comment.