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: fix multiplication incorrect of float and interval #37582

Merged
merged 1 commit into from May 21, 2019

Conversation

Projects
None yet
3 participants
@hueypark
Copy link
Contributor

commented May 19, 2019

Fixes #37540

Release note (bug fix): fix multiplication incorrect of float and interval

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

commented May 19, 2019

This change is Reviewable

@@ -30,6 +30,8 @@ const (
daysInMonth = 30
nanosInDay = 24 * int64(time.Hour) // Try as I might, couldn't do this without the cast.
nanosInMonth = daysInMonth * nanosInDay
nanosInHour = nanosInMinute * 60

This comment has been minimized.

Copy link
@mjibson

mjibson May 20, 2019

Member

These should be moved to somewhere in duration_test.go since they are only used in tests.

This comment has been minimized.

Copy link
@hueypark

hueypark May 21, 2019

Author Contributor

Thank you. I've fixed this problem.

sql: fix multiplication incorrect of float and interval
Fixes #37540

Release note (bug fix): fix multiplication incorrect of float and interval

@hueypark hueypark force-pushed the hueypark:fix-multiplication-incorrect branch from 8971a20 to c2979a0 May 21, 2019

@mjibson

This comment has been minimized.

Copy link
Member

commented May 21, 2019

bors r+

craig bot pushed a commit that referenced this pull request May 21, 2019

Merge #37582 #37705
37582: sql: fix multiplication incorrect of float and interval r=mjibson a=hueypark

Fixes #37540

Release note (bug fix): fix multiplication incorrect of float and interval

37705: server: handle new behavior of setrlimit on macOS r=bdarnell a=benesch

macOS's getrlimit can return a garbage hard limit for the number of open
files--that is, a hard limit which is higher than what setrlimit(2) will
accept. Since Go 1.12's syscall.Setrlimit actually delegates to the C
library's setrlimit, rather than making a system call directly,
Cockroach can fail to start because its call to syscall.Setrlimit now
fails.

This commit fixes the problem in two ways:

  1. It prevents a failed setrlimit call from stopping the entire
     server startup process, unless the original rlimit is really
     beneath the minimum allowable rlimit.

  2. It provides a macOS wrapper for syscall.Getrlimit that returns the
     actual hard limit, thus preventing the setrlimit call from failing
     in the first place.

Fix #37685.

Release note: None

Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
@craig

This comment has been minimized.

Copy link

commented May 21, 2019

Build succeeded

@craig craig bot merged commit c2979a0 into cockroachdb:master May 21, 2019

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

mjibson added a commit to mjibson/cockroach that referenced this pull request May 22, 2019

sql: improve interval math
In cockroachdb#37582 we added correct `interval * float` semantics. Here we extend that
to division.

In addition, remove the int variants because they need to share the same
semantics as float, so just force external users to cast their ints to
floats. None of our tests thus far have demonstrated a problem with the
possible precision loss for this operation. Since it was definitely incorrect
before this is an improvement. If we find some edge cases where the int ->
float conversion causes rounding or other problems we can revisit this
decision.

Release note (sql change): Correct interval math when multiplying or dividing
by floats or ints.

mjibson added a commit to mjibson/cockroach that referenced this pull request May 22, 2019

sql: improve interval math for div and mul
In cockroachdb#37582 we added correct `interval * float` semantics. Here we extend that to
float division.

In addition, change int division to be a wrapper around float division so that
remainders are correctly handled. Int multiply has no need to change since it's
lossless.

Release note (sql change): Correct interval math when multiplying or dividing
by floats or ints.

craig bot pushed a commit that referenced this pull request May 23, 2019

Merge #37744
37744: sql: improve interval math for div and mul r=mjibson a=mjibson

In #37582 we added correct `interval * float` semantics. Here we extend that to
float division.

In addition, change int division to be a wrapper around float division so that
remainders are correctly handled. Int multiply has no need to change since it's
lossless.

Release note (sql change): Correct interval math when multiplying or dividing
by floats or ints.

Co-authored-by: Matt Jibson <matt.jibson@gmail.com>

craig bot pushed a commit that referenced this pull request May 23, 2019

Merge #37744
37744: sql: improve interval math for div and mul r=mjibson a=mjibson

In #37582 we added correct `interval * float` semantics. Here we extend that to
float division.

In addition, change int division to be a wrapper around float division so that
remainders are correctly handled. Int multiply has no need to change since it's
lossless.

Release note (sql change): Correct interval math when multiplying or dividing
by floats or ints.

Co-authored-by: Matt Jibson <matt.jibson@gmail.com>

craig bot pushed a commit that referenced this pull request May 23, 2019

Merge #37744 #37745
37744: sql: improve interval math for div and mul r=mjibson a=mjibson

In #37582 we added correct `interval * float` semantics. Here we extend that to
float division.

In addition, change int division to be a wrapper around float division so that
remainders are correctly handled. Int multiply has no need to change since it's
lossless.

Release note (sql change): Correct interval math when multiplying or dividing
by floats or ints.

37745: col{data,serde}: various fixes related to TestArrowBatchConverterRandom r=solongordon a=asubiotto

See individual commits for details

Fixes #37458
Fixes #37692

Co-authored-by: Matt Jibson <matt.jibson@gmail.com>
Co-authored-by: Alfonso Subiotto Marqués <alfonso@cockroachlabs.com>

danhhz pushed a commit to danhhz/cockroach that referenced this pull request May 28, 2019

sql: improve interval math for div and mul
In cockroachdb#37582 we added correct `interval * float` semantics. Here we extend that to
float division.

In addition, change int division to be a wrapper around float division so that
remainders are correctly handled. Int multiply has no need to change since it's
lossless.

Release note (sql change): Correct interval math when multiplying or dividing
by floats or ints.

(sapling split of 567a5fe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.