cubewise-code / bedrock

Bedrock is TM1 Best Practice assets built from years of TM1 experience
Other
79 stars 74 forks source link

}bedrock.cube.data.import: Element checks fail with multi hierarchy dimensions #412

Open onefloid opened 5 months ago

onefloid commented 5 months ago

Description

If }bedrock.cube.data.import is used and the cube has at least one multi-hierarchy dimension, the element checks will fail (e.g. Data Line: 544) if the element is not in the standard hierarchy.

Solution

Maybe it's better to use the leaves hierarchy to check the elements in a dimension with multiple hierarchies. I'm not sure if there are any side effects that I haven't seen, though. This change would affect a lot of rows. Here is a small example.

Old

Data line: 505 and line 544

sElType = DType( sDim2, v3 );

...

ElseIf( sElType @= 'N' & DimIx( sDim2, v3 ) <> 0 );

New

  If(HierarchyExists(sDim2, 'Leaves') = 1);
      sElType = ElementType( sDim2, 'Leaves', v3 );
      nElIndex = ElementIndex( sDim2, 'Leaves', v3 );
  else;
      sElType = DType( sDim2, v3 );
      nElIndex = DimIx( sDim2, v3 );
  EndIf;

...

ElseIf( sElType @= 'N' & nElIndex <> 0 );

Alternatives

Pass a hierarchies list as a TI Parameter

Discussion

I look forward to your feedback

lotsaram commented 5 months ago

Hi @onefloid yes that's a good point. Possibly however, there's an easier and less effort and less obtrusive fix. In the Prolog at section ### Determine dimensions in target cube I think it might be possible to do the testing for the leaves hierarchy upfront and modify the sDimx variable, e.g.

sDim1 = TabDim( pCube, 1 );
If( HierarchyExists(sDim1, 'Leaves') = 1);
    sDim1 = Expand('%sDim1%:Leaves');
EndIf;

or 1 line variant thereof

sDim1 = TabDim( pCube, 1 ); If( HierarchyExists(sDim1, 'Leaves') = 1); sDim1 = Expand('%sDim1%:Leaves'); EndIf;

etc., etc. to sDim27

I believe (but could be wrong) that within the context of a function reference that TM1 will evaluate 'myDim':'Leaves' the same as 'myDim:Leaves' which is what the above would rely on.

I don't have a chance to test this myself this week but if you are able to and confirm if it works then this woudl be a quick and easy fix.

(Also IMO always best practice to make sure that all leaves are always present within the same named hierarchy.)

onefloid commented 5 months ago

Hi @lotsaram, Thanks. This is indeed an easier and less obtrusive solution. I can confirm that this solution works as expected for my use case.

I agree that it's best practice to have all leaves in the same named hierarchy. I know a use case where a dimension has for every year a hierarchy and the requirement is that only the current year is in the same named hierarchy.

lotsaram commented 5 months ago

Great! Are you able to do a pull request with the change?