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

Add rule for excessive number of measures in model #236

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

DBojsen
Copy link

@DBojsen DBojsen commented Oct 25, 2023

I believe a model should not have more than a few hundred measures.
More are typically a sign of poor model design or not using modern features such as calculation groups.
This rule highlights if a model has more than 500 measures.
If there is a good reason for this, if prompts the user to disable all BPA rules that iterate over the Model.AllMeasures collection as we have seen users TE becoming unresponsive if using these rules with a really big quantity measures.

@m-kovalsky

@DBojsen
Copy link
Author

DBojsen commented Oct 25, 2023

@microsoft-github-policy-service agree company="Tabular Editor ApS"

@otykier
Copy link

otykier commented Oct 25, 2023

Case in point: TabularEditor/TabularEditor#1148 (comment)

@m-kovalsky
Copy link
Collaborator

Thanks David, I completely agree with this and have experienced this myself (which is why I generally recommend not having the background BPA scan enabled). Would you point me to where Tabular Editor notifies the user to disable all BPA rules which use the Model.AllMeasures?

@DBojsen
Copy link
Author

DBojsen commented Oct 25, 2023

Would you point me to where Tabular Editor notifies the user to disable all BPA rules which use the Model.AllMeasures?

It's just in the rule description - we don't have a mechanism for prompting or the likes from a BPA rule.
Perhaps my PR description should have mentioned that.

@marcosqlbi
Copy link

I would say that 500 could be a too low limit.
While it is true that there is a performance penalty with thousands of measures in the model, it is also true that calculation groups for time intelligence are not the best practice from a performance standpoint.
A limit at 1500 would better IMHO. If you use Bravo for Power BI it's easy to get many measures generated automatically.
500 seems a low number IMHO.

@m-kovalsky
Copy link
Collaborator

Ah I see, thanks David. Yes, I had a feeling that the number chosen here (500, 1500 etc.) would be contentious. The primary part of the problem is rules which use measure x measure comparison (i.e. [DAX Expressions] No two measures should have the same definition). Then it's the square of the number of measures in the model (1,000 measures -> 1,000,000 checks).

@DBojsen
Copy link
Author

DBojsen commented Nov 6, 2023

@m-kovalsky & @marcosqlbi

I don't feel strongly if the limit should be 500, 1500 or something else - but I feel it would be a good check to add to the list.
I can change the limit, if you like. To 1500 ?

@m-kovalsky
Copy link
Collaborator

It's fine with me. Also remember that folks can always modify this number for their own use case if they so desire.

@DBojsen
Copy link
Author

DBojsen commented Nov 8, 2023

Perfect, I updated the rule definition

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.

4 participants