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

builtins: add width_bucket builtin function #38855

Closed
jordanlewis opened this issue Jul 12, 2019 · 7 comments · Fixed by #39263
Closed

builtins: add width_bucket builtin function #38855

jordanlewis opened this issue Jul 12, 2019 · 7 comments · Fixed by #39263
Labels
A-sql-builtins SQL built-in functions and semantics thereof. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience good first issue

Comments

@jordanlewis
Copy link
Member

The width_bucket builtin is a super useful function that figures out where a number lies inside of a histogram given certain parameters.

We should implement these. It's a pretty simple task that will boost compatibility (and help make ALTER TABLE SPLIT BY a lot easier to run).

This is a great issue for new contributors. The implementation of this builtin (well, each of its 3 overloads defined in the Postgres docs below) should live in pkg/sql/sem/builtins/builtins.go.

https://www.postgresql.org/docs/11/functions-math.html

@jordanlewis jordanlewis added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience good first issue A-sql-builtins SQL built-in functions and semantics thereof. labels Jul 12, 2019
@Inconnu08
Copy link
Contributor

I'll take on this

@jordanlewis
Copy link
Member Author

@Inconnu08 great! Do let me know if you need pointers.

@Russiancold
Copy link

@jordanlewis Hi! What is the actual status of this issue?

@Inconnu08
Copy link
Contributor

@Russiancold Hey, I'm still working on this. But I'll let you know if I can't. :)

@Russiancold
Copy link

@Inconnu08 Alright

@kevinbarbour
Copy link
Contributor

Hope you don't mind, but I took a stab at this since there wasn't much movement on it. #39263

@jordanlewis One thing I was unsure of is if it is necessary to have the overload for Numeric and Decimal? I created an overload for decimal and have been unable to find a situation where it doesn't work.

kevinbarbour added a commit to kevinbarbour/cockroach that referenced this issue Aug 2, 2019
Implements the width_bucket() builtin function

Details on the Postgres implementation can be found here:
https://www.postgresql.org/docs/11/functions-math.html

Resolves cockroachdb#38855

Release note (sql change): add the width_bucket builtin function.
@Inconnu08
Copy link
Contributor

Ahh, I was almost halfway there. Stuck with the tests. Hence I didn't PR until completely done. But it's completely fine. :)

kevinbarbour added a commit to kevinbarbour/cockroach that referenced this issue Aug 2, 2019
Implements the width_bucket() builtin function

Details on the Postgres implementation can be found here:
https://www.postgresql.org/docs/11/functions-math.html

Resolves cockroachdb#38855

Release note (sql change): add the width_bucket builtin function.
kevinbarbour added a commit to kevinbarbour/cockroach that referenced this issue Aug 2, 2019
Implements the width_bucket() builtin function

Details on the Postgres implementation can be found here:
https://www.postgresql.org/docs/11/functions-math.html

Resolves cockroachdb#38855

Release note (sql change): add the width_bucket builtin function.
craig bot pushed a commit that referenced this issue Aug 30, 2019
39263: sql: add width_bucket builtin r=jordanlewis a=barbourkd

Implements the width_bucket() builtin function

Details on the Postgres implementation can be found here:
https://www.postgresql.org/docs/11/functions-math.html

Resolves #38855

Release note (sql change): add the width_bucket builtin function.

Co-authored-by: Kevin Barbour <barbourkd@vcu.edu>
Co-authored-by: Kevin <kevinbarbourd@gmail.com>
@craig craig bot closed this as completed in 48599ad Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-builtins SQL built-in functions and semantics thereof. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-easy Easy issue to tackle, requires little or no CockroachDB experience good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants