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

Disallow axis ranges with the same min/max, which causes divide by zero error #27518

Merged
merged 4 commits into from
Jan 5, 2023

Conversation

adam-james-v
Copy link
Contributor

Fixes #27427

The group-axes private fn is used to try group series axes together if the ranges of the series are 'close enough', which is calculated by determining the percentage overlap the ranges share.

As the comparison is made, the first series in the list will always be compared with itself, and should have 1.0 (100%) overlap, as the ranges are going to be identical. The divide by zero issue arises when this first series, and potentially any other series, has a 'range' where the min and max are the same. This happens if the series has the same value in every row.

With this change, these ranges are considered invalid, and we avoid the divide by zero.

image

…ro error

The group-axes private fn is used to try group series axes together if the ranges of the series are 'close enough',
which is calculated by determining the percentage overlap the ranges share.

As the comparison is made, the first series in the list will always be compared with itself, and should have
1.0 (100%) overlap, as the ranges are going to be identical. The divide by zero issue arises when this first series,
and potentially any other series, has a 'range' where the min and max are the same. This happens if the series has the
same value in every row.

With this change, these ranges are considered invalid, and we avoid the divide by zero.
@codecov
Copy link

codecov bot commented Jan 5, 2023

Codecov Report

Base: 64.97% // Head: 64.97% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (791520b) compared to base (22c19cb).
Patch coverage: 66.66% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #27518   +/-   ##
=======================================
  Coverage   64.97%   64.97%           
=======================================
  Files        3184     3184           
  Lines       92586    92588    +2     
  Branches    11767    11766    -1     
=======================================
+ Hits        60154    60157    +3     
  Misses      27581    27581           
+ Partials     4851     4850    -1     
Flag Coverage Δ
back-end 85.40% <66.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/metabase/pulse/render/body.clj 72.45% <66.66%> (+0.25%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adam-james-v adam-james-v added the backport Automatically create PR on current release branch on merge label Jan 5, 2023
@adam-james-v adam-james-v enabled auto-merge (squash) January 5, 2023 20:23
@adam-james-v adam-james-v merged commit 716b87b into master Jan 5, 2023
@adam-james-v adam-james-v deleted the bugfix-27427-axis-grouping-divide-by-zero branch January 5, 2023 20:24
github-actions bot pushed a commit that referenced this pull request Jan 5, 2023
…ro error (#27518)

* Disallow axis ranges with the same min/max, which causes divide by zero error

The group-axes private fn is used to try group series axes together if the ranges of the series are 'close enough',
which is calculated by determining the percentage overlap the ranges share.

As the comparison is made, the first series in the list will always be compared with itself, and should have
1.0 (100%) overlap, as the ranges are going to be identical. The divide by zero issue arises when this first series,
and potentially any other series, has a 'range' where the min and max are the same. This happens if the series has the
same value in every row.

With this change, these ranges are considered invalid, and we avoid the divide by zero.

* Unskip e2e repro
metabase-bot bot added a commit that referenced this pull request Jan 5, 2023
…ro error (#27518) (#27535)

* Disallow axis ranges with the same min/max, which causes divide by zero error

The group-axes private fn is used to try group series axes together if the ranges of the series are 'close enough',
which is calculated by determining the percentage overlap the ranges share.

As the comparison is made, the first series in the list will always be compared with itself, and should have
1.0 (100%) overlap, as the ranges are going to be identical. The divide by zero issue arises when this first series,
and potentially any other series, has a 'range' where the min and max are the same. This happens if the series has the
same value in every row.

With this change, these ranges are considered invalid, and we avoid the divide by zero.

* Unskip e2e repro

Co-authored-by: adam-james <21064735+adam-james-v@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Static viz fails when there's an unused column returned
2 participants