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

sql: float cast to integer is incorrect #112515

Closed
DrewKimball opened this issue Oct 17, 2023 · 1 comment · Fixed by #117798
Closed

sql: float cast to integer is incorrect #112515

DrewKimball opened this issue Oct 17, 2023 · 1 comment · Fixed by #117798
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team

Comments

@DrewKimball
Copy link
Collaborator

DrewKimball commented Oct 17, 2023

Casting a float value to an integer is meant to round to the nearest integer, but currently, CRDB just truncates the float. See a comparison against postgres:

root@localhost:26257/system/defaultdb> select 0.9090909090909091::float8::int8;
  int8
--------
     0
(1 row)

vs

postgres=# select 0.9090909090909091::float8::int8;
 int8
------
    1
(1 row)

Jira issue: CRDB-32461

@DrewKimball DrewKimball added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Oct 17, 2023
@DrewKimball
Copy link
Collaborator Author

This reproduces at least as far back as v22.2.

@yuzefovich yuzefovich added the T-sql-queries SQL Queries Team label Oct 17, 2023
@yuzefovich yuzefovich added the E-quick-win Likely to be a quick win for someone experienced. label Oct 24, 2023
da-ket added a commit to da-ket/cockroach that referenced this issue Jan 16, 2024
Previously, casting a float value to an integer simply truncates the float.
This commit fixes that by rounding the float value to the nearest integer,
rounding ties to even, as in postgres.

Epic: None
Fixes: cockroachdb#112515
da-ket added a commit to da-ket/cockroach that referenced this issue Jan 16, 2024
Previously, casting a float value to an integer simply truncates the float.
This commit fixes that by rounding the float value to the nearest integer,
rounding ties to even, as in postgres.

Epic: None
Fixes: cockroachdb#112515

Release note (bug fix): A bug has been fixed where casts of floats to integers
truncated the value. These casts now round a float to the nearest integer,
rounding ties to even integers.
craig bot pushed a commit that referenced this issue Feb 2, 2024
117798: sql: fix float to integer casting r=mgartner a=da-ket

Previously, casting a float value to an integer simply truncates the float. This commit fixes that by rounding the float value to the nearest integer, rounding ties to even, as in postgres.

Epic: None
Fixes: #112515

Release note (bug fix): A bug has been fixed where casts of floats to integers
truncated the value. These casts now round a float to the nearest integer,
rounding ties to even integers.

118489: concurrency: add basic deadlock tests involving shared locks r=arulajmani a=arulajmani

Informs #109634

Release note: None

118640: sql: fix usages of internal executor to delegate from user transaction r=rafiss a=rafiss

These commits fix a few usages of the internal executor to use the new interface which delegates to an internal executor that shares the outer txn's state.

For ease of review, the testing for this is in a separate PR (where the bugs were discovered): #107990

informs #100178

118653: sql: keep track of per-stmt read committed retries r=rafiss a=rafiss

This change makes it possible to view automatic retries performed under read committed txns in the DB Console.

fixes #113986
Release note: None

118657: server/status: silence missing CPU cgroup error r=nvanbenschoten a=nvanbenschoten

Fixes #111648.

This commit eliminates log spam when a CPU cgroup is not configured for the cockroach process. This is a supported deployment mode, but since `v22.2.11`/`v23.1.3`/`v23.2.0` (df55958), we've been spamming the logs with "unable to get CPU capacity" errors every 10 seconds when running outside of a CPU cgroup.

I considered reducing the severity of the log message from ERROR to INFO, but even that seemed loud for a log message every 10s in a supported deployment mode. For now, we remove the log message.

Release note (bug fix): Cockroach will no longer spam the logs with "unable to get CPU capacity" errors every 10 seconds when running outside of a CPU cgroup.

118660: server: disambiguate duplicated column names in /v2/sql endpoint r=rafiss a=rafiss

Previously, results that had the same column name would clobber each other in the response of this endpoint, since JSON does not allow duplicate key names. This is fixed by adding a suffix when column names are duplicated.

No release note since this endpoint is for internal use.

fixes #116851
Release note: None

Co-authored-by: da-ket <daket.duet@gmail.com>
Co-authored-by: Arul Ajmani <arulajmani@gmail.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@craig craig bot closed this as completed in 66be939 Feb 2, 2024
wenyihu6 pushed a commit to wenyihu6/cockroach that referenced this issue Feb 21, 2024
Previously, casting a float value to an integer simply truncates the float.
This commit fixes that by rounding the float value to the nearest integer,
rounding ties to even, as in postgres.

Epic: None
Fixes: cockroachdb#112515

Release note (bug fix): A bug has been fixed where casts of floats to integers
truncated the value. These casts now round a float to the nearest integer,
rounding ties to even integers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants