Skip to content

fix(partition): zero value sectors cause max stack call#2260

Merged
markov00 merged 3 commits intoelastic:mainfrom
markov00:2023_11_28-fix_division_by_zero
Nov 29, 2023
Merged

fix(partition): zero value sectors cause max stack call#2260
markov00 merged 3 commits intoelastic:mainfrom
markov00:2023_11_28-fix_division_by_zero

Conversation

@markov00
Copy link
Collaborator

@markov00 markov00 commented Nov 28, 2023 •
edited
Loading

Summary

After this change #1983 sectors with zero can appear in the data transformation. This could cause a max call stack error due to a uncached division by zero.

The PR fixes the division without touching the current recursive loop.

Issues

will fix elastic/kibana#164730 when merged into Kibana

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated

@markov00 markov00 added bug Something isn't working :data Data/series/scales related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related labels Nov 28, 2023
@markov00 markov00 force-pushed the 2023_11_28-fix_division_by_zero branch from 9131972 to e314c70 Compare November 28, 2023 11:34
@markov00 markov00 marked this pull request as ready for review November 28, 2023 14:30
dej611
dej611 approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Code review 👍

@markov00 markov00 merged commit 4b30db7 into elastic:main Nov 29, 2023
@markov00 markov00 deleted the 2023_11_28-fix_division_by_zero branch November 29, 2023 10:33
markov00 added a commit to markov00/elastic-charts that referenced this pull request Dec 7, 2023
@markov00
fix(partition): zero value sectors cause max stack call (elastic#2260)
fd7c27a
@markov00 markov00 mentioned this pull request Dec 7, 2023
Merged
markov00 added a commit that referenced this pull request Dec 7, 2023
@markov00
fix(partition): zero value sectors cause max stack call (#2260) [60.x] (
5034852 
#2274)

fix(partition): zero value sectors cause max stack call (#2260)
nickofthyme pushed a commit that referenced this pull request Dec 7, 2023
@semantic-release-bot
chore(release): 60.0.1 [skip ci]
aab63c7 
## [60.0.1](v60.0.0...v60.0.1) (2023-12-07)

### Bug Fixes

* clamp brushing min of last bucket ([#2227](#2227)) [60.x] ([#2276](#2276)) ([7582811](7582811))
* **partition:** zero value sectors cause max stack call ([#2260](#2260)) [60.x] ([#2274](#2274)) ([5034852](5034852))
nickofthyme pushed a commit that referenced this pull request Dec 19, 2023
@semantic-release-bot
chore(release): 61.2.0 [skip ci]
dc705ad 
# [61.2.0](v61.1.0...v61.2.0) (2023-12-19)

### Bug Fixes

* **deps:** update dependency @elastic/eui to ^91.1.0 ([#2267](#2267)) ([308e974](308e974))
* **deps:** update dependency @elastic/eui to ^91.2.0 ([#2268](#2268)) ([29cdcb3](29cdcb3))
* **metric:** background colors and sparkline rendering ([#2255](#2255)) ([5abddfc](5abddfc))
* **partition:** rendering with small radius ([#2273](#2273)) ([95a8537](95a8537))
* **partition:** zero value sectors cause max stack call ([#2260](#2260)) ([4b30db7](4b30db7))
* **theme:** legacy margins ([#2262](#2262)) ([299c869](299c869))

### Features

* increase tooltip width to 500px and truncate items to 2 lines ([#2261](#2261)) ([afdef1c](afdef1c))
@markov00 markov00 mentioned this pull request Dec 20, 2023
Merged
1 task
markov00 added a commit to elastic/kibana that referenced this pull request Dec 21, 2023
@renovate @markov00
Update dependency @elastic/charts to v61.2.0 (main) (#173329)
68883b3 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@elastic/charts](https://github.com/elastic/elastic-charts) |
[`61.0.3` ->
`61.2.0`](https://renovatebot.com/diffs/npm/@elastic%2fcharts/61.0.3/61.2.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@elastic%2fcharts/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@elastic%2fcharts/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@elastic%2fcharts/61.0.3/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@elastic%2fcharts/61.0.3/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>elastic/elastic-charts (@&#8203;elastic/charts)</summary>

###
[61.2.0](elastic/elastic-charts@v61.1.0...v61.2.0)
(2023-12-19)


##### Bug Fixes

* **deps:** update dependency @elastic/eui to ^91.1.0
([#2267](elastic/elastic-charts#2267))
([308e974](elastic/elastic-charts@308e974))
* **deps:** update dependency @elastic/eui to ^91.2.0
([#2268](elastic/elastic-charts#2268))
([29cdcb3](elastic/elastic-charts@29cdcb3))
* **metric:** background colors and sparkline rendering
([#2255](elastic/elastic-charts#2255))
([5abddfc](elastic/elastic-charts@5abddfc))
* **partition:** rendering with small radius
([#2273](elastic/elastic-charts#2273))
([95a8537](elastic/elastic-charts@95a8537))
* **partition:** zero value sectors cause max stack call
([#2260](elastic/elastic-charts#2260))
([4b30db7](elastic/elastic-charts@4b30db7))
* **theme:** legacy margins
([#2262](elastic/elastic-charts#2262))
([299c869](elastic/elastic-charts@299c869))


##### Features

* increase tooltip width to 500px and truncate items to 2 lines
([#2261](elastic/elastic-charts#2261))
([afdef1c](elastic/elastic-charts@afdef1c))

###
[`v61.1.0`](https://github.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6110-2023-11-20)

[Compare
Source](https://github.com/elastic/elastic-charts/compare/v61.0.3...v61.1.0)

##### Bug Fixes

- **deps:** update dependency
[@&#8203;elastic/eui](https://github.com/elastic/eui) to v91
([#&#8203;2233](https://github.com/elastic/elastic-charts/issues/2233))
([e89f623](https://github.com/elastic/elastic-charts/commit/e89f623792312c4f6b609ebb975de0800f3c297e))
- **metric:** add option to set empty cell background color
([#&#8203;2247](https://github.com/elastic/elastic-charts/issues/2247))
([6a9fb86](https://github.com/elastic/elastic-charts/commit/6a9fb86bee5212a47060c5070f260961097014b4))
- **metric:** background color for bar with interactions
([#&#8203;2248](https://github.com/elastic/elastic-charts/issues/2248))
([dcb56fa](https://github.com/elastic/elastic-charts/commit/dcb56fa08540631a9b3b0e588352ee6daf3d54a0))

##### Features

- **bullet:** improve header layout and positioning
([#&#8203;2243](https://github.com/elastic/elastic-charts/issues/2243))
([b3a95d2](https://github.com/elastic/elastic-charts/commit/b3a95d24fb02690ca6599622352c743c04624690))
- **bullet:** new design style and implementation
([#&#8203;2156](https://github.com/elastic/elastic-charts/issues/2156))
([fca6cdd](https://github.com/elastic/elastic-charts/commit/fca6cdd5bc34a65c0792dbab7d756404bf43501b))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/elastic/kibana).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44Ny4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTAzLjEiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
@nickofthyme nickofthyme mentioned this pull request Jan 30, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickofthyme nickofthyme Awaiting requested review from nickofthyme

1 more reviewer

@dej611 dej611 dej611 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working :data Data/series/scales related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Lens] Treemap error on sparse table

2 participants

@markov00 @dej611