Skip to content

Conversation

@carlopi
Copy link
Contributor

@carlopi carlopi commented Nov 20, 2023

Some reference implementations:

Before this PR, 10000 would be returned as 10.0 KB, after as 9.76 KiB (node 9.76 * 1024 = 10000).

Note that input side both styles are now supported, so both 100.0 KB, 97.6 KiB, 100000 bytes, 0.1 MB or 0.095367432 MiB are supported and the resulting settings would be roughly equivalent.

This PR makes a few opinionated choices:

  • MiB instead of MB
  • space between number and unit
  • relevant digits are in the range 1.0-1023.9, similar to previous behaviour (it was 1.0 - 1000). We could consider going for a preferred range starting at 10.0, as postgress does, where for example 7649 kB is rendered instead of 7.6 MB

An overload for format_bytes taking the binary base was considered, but given there are other possible overloads (precision, preferred unit), I think it's safer to avoid it, also given there are the alternative methods formatReadableSize and formatReadableDecimalSize.

@github-actions github-actions bot marked this pull request as draft November 20, 2023 16:45
@carlopi carlopi marked this pull request as ready for review November 20, 2023 16:46
@github-actions github-actions bot marked this pull request as draft November 20, 2023 19:33
@carlopi carlopi marked this pull request as ready for review November 20, 2023 19:33
@github-actions github-actions bot marked this pull request as draft November 20, 2023 19:53
@carlopi carlopi marked this pull request as ready for review November 20, 2023 21:42
@Mytherin Mytherin merged commit 18f614f into duckdb:main Nov 24, 2023
@Mytherin
Copy link
Collaborator

Thanks! LGTM

@carlopi carlopi added the Needs Documentation Use for issues or PRs that require changes in the documentation label Nov 24, 2023
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Dec 14, 2023
Merge pull request duckdb/duckdb#9736 from carlopi/format_bytes
Merge pull request duckdb/duckdb#9659 from pdet/arrow_jdbc_segfault
@carlopi carlopi deleted the format_bytes branch January 10, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Documentation Use for issues or PRs that require changes in the documentation Ready For Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants