Skip to content

fix: improve sorting logic in renderSummary to handle NaN values#27

Closed
waynexia wants to merge 1 commit intoClickHouse:mainfrom
waynexia:fix-sort-algo
Closed

fix: improve sorting logic in renderSummary to handle NaN values#27
waynexia wants to merge 1 commit intoClickHouse:mainfrom
waynexia:fix-sort-algo

Conversation

@waynexia
Copy link
Copy Markdown

Fix render logic.

Some cases (like Elasticsearch (no source, best compression)) will bring NaN values. They may break the sorting logic and generate weird illustrations.

This patch enhances NaN handling in both comparison logic and relative scale rendering.

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
@waynexia
Copy link
Copy Markdown
Author

Now it can sort the NaN to the last, with the correct scale bar rendering

image

@rschu1ze
Copy link
Copy Markdown
Member

@waynexia Thanks for this PR, I think it can be merged.

One question I had is why we show "NaN" at all for "Elasticsearch (no source, best compression)". I could not find "NaN" in the corresponding result .json file (and in fact I could not find "NaN" in any result .json file of any database).

@sunng87
Copy link
Copy Markdown
Contributor

sunng87 commented Mar 18, 2025

it seems we missed total_size for "Elasticsearch (no source, best compression)"

@rschu1ze
Copy link
Copy Markdown
Member

It is better to correct the single incorrect JSON document (out of >60 JSON documents), than to add special handling in JavaScript. In this particular case, field total_size is affected and every benchmarked database is expected to provide some sort of total size information (i.e. NaN is not a viable value).

--> #28

@rschu1ze rschu1ze closed this Mar 19, 2025
@waynexia waynexia deleted the fix-sort-algo branch March 19, 2025 16:11
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