Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-expand transformation #293

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Auto-expand transformation #293

wants to merge 35 commits into from

Conversation

HeikoTheissen
Copy link
Contributor

@HeikoTheissen HeikoTheissen commented Nov 10, 2023

See wiki.

"@Core.OptionalParameter": { "DefaultValue": "false" }
}
],
"$ReturnType": { "$Collection": true, "$Type": "Edm.EntityType" }
Copy link
Member

@uhlmannm uhlmannm Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the EntityType is not enough. We will need some of the information that we also get from a hierarchical query. Most importantly, the LimitedDescendantCount but also the DrillState and the DistanceFromRoot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need an instance annotation whose type contains a subset of the properties of Hierarchy.RecursiveHierarchyType. Which subset exactly?

Copy link
Member

@uhlmannm uhlmannm Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasChadzelek I had found the LimitedDescendantCount, the DrillState and the DistanceFromRoot. Is that it? Or already too much?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it sufficient if these instance annotations are returned by the auto-expand function? If one node is expanded, they must be computed by the OData model itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes. In the other cases, the model is creating requests to manually expand the visual groups. The backend would not even be aware that we are creating a visual grouping on the screen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasChadzelek I had found the LimitedDescendantCount, the DrillState and the DistanceFromRoot. Is that it? Or already too much?

What about LimitedRank? Our new favorite ;-)

DistanceFromRoot might be redundant because the hierarchy is leveled, not recursive?

It is needed if `AggregatedValuesLeafFilter` is specified or properties
from `Aggregation` occur in `SiblingOrder`. Especially the latter is
typical.
@HeikoTheissen HeikoTheissen marked this pull request as ready for review January 19, 2024 15:33
<Annotation Term="Core.Description" String="Sort specification to apply to all direct descendants of a given entry in the resulting leveled hierarchy" />
</Parameter>
<Parameter Name="BeforeAggregationFilter" Type="Edm.String" Nullable="false">
<Annotation Term="Core.Description" String="Expression valid for `filter` transformation to restrict the input set before any further procressing" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you achieve the same thing with a filter() transformation beforehand, in the same $apply? For TopLevels, we do likewise with orderby() or search() or filter().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but then the backend would have to take all these transformations together and process them as one "auto-expand" transformation. (At least with the Analytical Engine, the filter is part of the auto-expand operation.)

If the backend agrees, we can change this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the backend disagrees, do we need support for search as well? Or is that too fuzzy for analytics?

@HeikoTheissen
Copy link
Contributor Author

HeikoTheissen commented Apr 26, 2024

Open issue: Whether and how to instruct the transformation to create one row for an expanded group header above its descendants and another row for the subtotals below its descendants.

"$IsBound": true,
"@Common.Experimental": true,
"@Core.Description": "`$apply` transformation that expands an unnamed leveled hierarchy with custom aggregation of certain properties",
"@Core.LongDescription": "Example transformation sequence:\n```\n$apply=filter(Industry eq 'IT')\n/groupby((Country,CountryName,Region,RegionName),\n filter($these/aggregate(Amount) gt 0 and\n $these/aggregate(Currency) ne null))\n/concat(\n Analytics.AutoExpand(\n Levels=[{\"P\":[\"Country\",\"CountryName\"]},\n {\"P\":[\"Region\",\"RegionName\"]}],\n LeafLevel=[\"Segment\",\"Industry\"],\n Aggregation=[\"Amount\",\"Currency\"],\n SiblingOrder=[{\"Property\":\"Amount\",\n \"Order\":\"desc\"}],\n ExpandLevels=[{\"Entry\":[\"US\",\"USA\"],\"Levels\":0},\n {\"Entry\":[\"DE\",\"Germany\",\"BW\",\"Baden-Württemberg\"],\"Levels\":2}])\n /concat(aggregate($count) as ResultEntriesCount,\n skip(20)/top(10)),\n aggregate(Amount,Currency))\n```\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In UI5 we start with the leaves count that is missing in the example. It would be a term groupby((Country,CountryName,Region,RegionName))/aggregate($count as LeavesCount) in the outer concat followed by the grand total term aggregate(Amount,Currency) that is currently last in the example. The third term in the outer concat would be Analytics.AutoExpand(...)/concat(aggregate($count) as ResultEntriesCount,skip(20)/top(10)).

{
"$Name": "LeafLevel",
"$Collection": true,
"@Core.Description": "A possibly empty set of property names that constitute, together with the property names from `Levels`, the leaf level of the leveled hierarchy",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possibly empty set of property names

I wonder how that would work. If LeafLevel is empty then the last level in Levels is not expanded, cannot be expanded and could as well be written into LeafLevel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't LeafLevel simply the last entry in Levels, then?

Copy link
Member

@uhlmannm uhlmannm Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be several properties used for grouping on the leaf level that are not used for visual grouping / group headers.
The interesting part is that we have three things:

  1. properties used for visual grouping
  2. properties used additionally for grouping on the leaf level
  3. the number of visual groups initially expanded.

I think we had decided somewhen in the past that 3. is not necessary for the AutoExpand function as one can simply as collapsed visual groups to 2. But I wonder whether that decision is still good when introducing ExpandLevels. Should the backend then not know which visual groups are introduced, how grouping is done on leaf level and how deep to expand initially?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The leaf level is just the deepest grouping level, therefore we only need the parameter Levels, not LeafLevel.

In the current definition, nodes not mentioned in ExpandLevels are completely expanded. If you want the default to be "expand N levels", we need an additional parameter for that N.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can do it like that. But the question is how to denote the leaf level. Is it always the last of the Levels? And hence this last level is not and cannot be expanded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be stated explicitly in the Levels description?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at the FE examples, then on the leaf level there are individual (unaggregated) records. If that is the target state, one could also argue that the last level can still be expanded and below this last level are the unaggregated records.

Copy link
Contributor Author

@HeikoTheissen HeikoTheissen Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

entries on the finest-grained level cannot be expanded further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants