Pentaho Analysis - Mondrian
  1. Pentaho Analysis - Mondrian
  2. MONDRIAN-786

RolapMemberBase.setUniqueName generates incorrect values for children of hidden members in a ragged hierarchy

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Medium Medium
    • Resolution: Unresolved
    • Affects Version/s: 3.2.0 GA (3.6.0 GA Suite Release)
    • Fix Version/s: Mondrian Backlog
    • Component/s: None
    • Labels:
      None
    • Notice:
      When an issue is open, the "Fix Version/s" field conveys a target, not necessarily a commitment. When an issue is closed, the "Fix Version/s" field conveys the version that the issue was fixed in.

      Description

      When a hidden member's children are created, the constructor for mondrian.rolap.RolapMemberBase calls setUniqueName(Object) to create the unique name of the child member based on its parent member and its name. The current implementation generates the uniqueName by concatenating the uniqueName of the parent member with the value (via in internal call to Util.makeFqName). However, in the case of ragged hierarchies (in which some members are hidden), the uniqueName of the child member should be the concatenation of the uniqueName of the nearest visible ancestor (not necessarily the immediate parent) with the value.

      For example, in the "Sales Ragged" cube, the dimesion "Store" contains the hierarchy Country->State->City->Name, wherein the levels "State" and "City" may be hidden. In the case of "Store 17", the value of the level "State" is "Vatican", which is the same as the value for the level "Country", so it is hidden. Furthermore, the level "City" is hidden because the city name for "Store 17" is NULL. Thus, the correct uniqueName for "Store 17" would be [Store].[Vatican].[Store 17], but what gets generated is [Store].[Vatican].[Vatican].[#null].[Store 17]

      I propose the following modification of the method mondrian.rolap.RolapMemberBase.setUniqueName(Object) to implement the correct behavior of concatenating the uniqueName of the **nearest visible ancestor** with the value (as opposed to simply using **parentMember**). Note the insertion of 5 lines at the beginning of the method and the replacement of references to the member variable "parentMember" with references to the local variable "nearestVisibleAncestorMember'.

          protected void setUniqueName(Object key) {
              Member nearestVisibleAncestorMember = parentMember;
              // Walk up the hierarchy until we find a visible ancestor
              while(nearestVisibleAncestorMember != null && (nearestVisibleAncestorMember.isHidden())) {
                  nearestVisibleAncestorMember = nearestVisibleAncestorMember.getParentMember();
              }
              String name = keyToString(key);

              // Drop the '[All Xxxx]' segment in regular members.
              // Keep the '[All Xxxx]' segment in the 'all' member.
              // Keep the '[All Xxxx]' segment in calc members.
              // Drop it in visual-totals and parent-child members (which are flagged
              // as calculated, but do have a data member).
              if (nearestVisibleAncestorMember == null
                  || (nearestVisibleAncestorMember.isAll()
                      && (!isCalculated()
                          || this instanceof VisualTotalsFunDef.VisualTotalMember
                          || getDataMember() != null)))
              {
                  final RolapHierarchy hierarchy = getHierarchy();
                  final Dimension dimension = hierarchy.getDimension();
                  if (/* dimension.getHierarchies().length == 1
                      && */ hierarchy.getName().equals(dimension.getName()))
                  {
                      // Kludge to ensure that calc members are called
                      // [Measures].[Foo] not [Measures].[Measures].[Foo]. We can
                      // remove this code when we revisit the scheme to generate
                      // member unique names.
                      this.uniqueName = Util.makeFqName(dimension, name);
                  } else {
                      this.uniqueName = Util.makeFqName(hierarchy, name);
                  }
              } else {
                  this.uniqueName = Util.makeFqName(nearestVisibleAncestorMember, name);
              }
          }

        Activity

        Hide
        Will Gorman added a comment -
        Hi Julian, can you review these changes? Thanks! Will
        Show
        Will Gorman added a comment - Hi Julian, can you review these changes? Thanks! Will
        Hide
        Julian Hyde added a comment -
        I would challenge the statement

        "the correct uniqueName for "Store 17" would be [Store].[Vatican].[Store 17]"

        We've never made a decision about what the correct unique name for a member of a ragged hierarchy should be. We should make that decision as part of fixing the bug. The scheme suggested by Jason Edwards looks like it might create ambiguous unique names, and that would not be acceptable. A unique name like "[Store].[Vatican].[Vatican].[#null].[Store 17] " would be acceptable, provided that mondrian was able to parse it.
        Show
        Julian Hyde added a comment - I would challenge the statement "the correct uniqueName for "Store 17" would be [Store].[Vatican].[Store 17]" We've never made a decision about what the correct unique name for a member of a ragged hierarchy should be. We should make that decision as part of fixing the bug. The scheme suggested by Jason Edwards looks like it might create ambiguous unique names, and that would not be acceptable. A unique name like "[Store].[Vatican].[Vatican].[#null].[Store 17] " would be acceptable, provided that mondrian was able to parse it.
        Hide
        Jason Edwards added a comment -
        Agreed.

        Upon further testing, the change above should NOT be implemented, as the ambiguous unique names will indeed cause problems ([Dimension].[#null].[#null] will be changed to [Dimension].[#null], which is identical to its parent's unique name).

        The real issue I'm facing is not that the unique name is incorrect, but rather that the method mondrian.mdx.MemberExpr.toString() returns the "uniqueName" when an MDX query is being unparsed, resulting in MDX that cannot be re-parsed. This is where the portions of the "uniqueName" corresponding to hidden levels would need to be removed, not in RolapMemberBase as I proposed above.

        Alternatively, modifying mondrian.olap.Util.lookupCompound(SchemaReader, OlapElement, List<Segment>, boolean, int, MatchType) to be able to re-parse a unique name like "[Store].[Vatican].[Vatican].[#null].[Store 17]" would solve the problem I'm facing. It would take something like a "continue" statement in the for loop to skip over segments corresponding to hidden levels.
        Show
        Jason Edwards added a comment - Agreed. Upon further testing, the change above should NOT be implemented, as the ambiguous unique names will indeed cause problems ([Dimension].[#null].[#null] will be changed to [Dimension].[#null], which is identical to its parent's unique name). The real issue I'm facing is not that the unique name is incorrect, but rather that the method mondrian.mdx.MemberExpr.toString() returns the "uniqueName" when an MDX query is being unparsed, resulting in MDX that cannot be re-parsed. This is where the portions of the "uniqueName" corresponding to hidden levels would need to be removed, not in RolapMemberBase as I proposed above. Alternatively, modifying mondrian.olap.Util.lookupCompound(SchemaReader, OlapElement, List<Segment>, boolean, int, MatchType) to be able to re-parse a unique name like "[Store].[Vatican].[Vatican].[#null].[Store 17]" would solve the problem I'm facing. It would take something like a "continue" statement in the for loop to skip over segments corresponding to hidden levels.

          People

          • Assignee:
            Julian Hyde
            Reporter:
            Jason Edwards
          • Votes:
            2 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: