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: Add support for floor division #6642

Merged

Conversation

nvanbenschoten
Copy link
Member

@nvanbenschoten nvanbenschoten commented May 11, 2016

See #6369.

This change adds the syntax // for floor division.


This change is Reviewable

@knz
Copy link
Contributor

knz commented May 12, 2016

LGTM, but I was imagining more an overloaded operator. x // y where x and y are decimals should produce decimal, just one there the decimal part is all zeros. Same for floats.

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

sql: Add support for integer division

See #6369.

This change adds the syntax // for integer division.


Reviewed 10 of 10 files at r1.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@bdarnell
Copy link
Contributor

Yeah, in python // is called "floor division" instead of "integer division" because it's equivalent to floor(x/y) cast to the type of the inputs.

Previously, knz (kena) wrote…

LGTM, but I was imagining more an overloaded operator. x // y where x and y are decimals should produce decimal, just one there the decimal part is all zeros. Same for floats.


Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


sql/parser/scan.go, line 330 [r1] (raw file):

  case '/':
      switch s.peek() {
      case '/': // //

Nice comment :)


Comments from Reviewable

@nvanbenschoten nvanbenschoten changed the title sql: Add support for integer division sql: Add support for floor division May 19, 2016
@nvanbenschoten
Copy link
Member Author

Updated to support floor division with all three numeric data types. I also had to change a few tests now that DECIMAL is the default numeric data type. PTAL

Previously, bdarnell (Ben Darnell) wrote…

Yeah, in python // is called "floor division" instead of "integer division" because it's equivalent to floor(x/y) cast to the type of the inputs.


Review status: 0 of 10 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.


sql/parser/scan.go, line 330 [r1] (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Nice comment :)

I'm pretty proud of that one.

Comments from Reviewable

@nvanbenschoten nvanbenschoten force-pushed the nvanbenschoten/intDiv branch 2 times, most recently from a52e6f0 to c6ed8ea Compare May 24, 2016 16:44
@nvanbenschoten
Copy link
Member Author

Does this still look good after the support for FLOAT and DECIMAL?

See cockroachdb#6369.

This change adds the syntax `//` for floor division.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants