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

Add width_bucket function with bins array #63

Closed
wants to merge 1 commit into from
Closed

Add width_bucket function with bins array #63

wants to merge 1 commit into from

Conversation

shixuan-fan
Copy link
Contributor

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 18, 2021
@facebook-github-bot
Copy link
Contributor

@shixuan-fan has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mbasmanova mbasmanova requested a review from kgpai August 18, 2021 19:06
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shixuan-fan Would you document the new function?

Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
@shixuan-fan
Copy link
Contributor Author

@shixuan-fan Would you document the new function?

Will do. Sorry missed that :)

Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shixuan-fan Looks great. Just a few nits and one comment on error reporting.

velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
@shixuan-fan
Copy link
Contributor Author

There are some major changes after changing it to be a stateful vector, so another round of review would be appreciated :) Also, the TODO I have in test might not be legit. It might just be due to my lack of knowledge on how to implement them.

Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shixuan-fan Looks great. Some comments below.

velox/docs/functions/math.rst Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/WidthBucketArray.cpp Outdated Show resolved Hide resolved
velox/functions/common/tests/WidthBucketArrayTest.cpp Outdated Show resolved Hide resolved
velox/functions/common/tests/WidthBucketArrayTest.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shixuan-fan Looks good to me.

@facebook-github-bot
Copy link
Contributor

@shixuan-fan has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Pull Request resolved: #63

Reviewed By: mbasmanova

Differential Revision: D30402055

Pulled By: shixuan-fan

fbshipit-source-id: dff5b88a0b022cc8371b5fb26634c0046755b812
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D30402055

@facebook-github-bot
Copy link
Contributor

@shixuan-fan merged this pull request in d1708eb.

rui-mo pushed a commit to rui-mo/velox that referenced this pull request Nov 14, 2022
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Nov 22, 2022
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Dec 15, 2022
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Jan 6, 2023
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Jan 12, 2023
PHILO-HE pushed a commit to PHILO-HE/velox that referenced this pull request Feb 3, 2023
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Feb 24, 2023
liujiayi771 pushed a commit to liujiayi771/velox that referenced this pull request Mar 3, 2023
liujiayi771 pushed a commit to liujiayi771/velox that referenced this pull request Mar 9, 2023
liujiayi771 pushed a commit to liujiayi771/velox that referenced this pull request Apr 1, 2023
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Apr 23, 2023
zhouyuan pushed a commit to zhouyuan/velox that referenced this pull request Jun 7, 2023
relative pr:

Fix replace SparkSQL function facebookincubator#277
Support kPreceeding & kFollowing for window range frame type facebookincubator#287
support timestamp hash facebookincubator#269
Spark sum can overflow facebookincubator#101
Support float & double types in pmod function facebookincubator#157
Implement datetime functions in velox/sparksql. facebookincubator#81
Fix type check in MapFunction facebookincubator#273
Let function validation fail for lookaround pattern in RE2-based implementation facebookincubator#124
Register lpad/rpad functions for Spark SQL. facebookincubator#63
Support substring_index sql function facebookincubator#189
Fix First/Last aggregate functions intermediate type and support decimal facebookincubator#245
Support date_add spark sql function facebookincubator#144
Yohahaha pushed a commit to Yohahaha/velox that referenced this pull request Jul 4, 2023
relative pr:

Fix replace SparkSQL function facebookincubator#277
Support kPreceeding & kFollowing for window range frame type facebookincubator#287
support timestamp hash facebookincubator#269
Spark sum can overflow facebookincubator#101
Support float & double types in pmod function facebookincubator#157
Implement datetime functions in velox/sparksql. facebookincubator#81
Fix type check in MapFunction facebookincubator#273
Let function validation fail for lookaround pattern in RE2-based implementation facebookincubator#124
Register lpad/rpad functions for Spark SQL. facebookincubator#63
Support substring_index sql function facebookincubator#189
Fix First/Last aggregate functions intermediate type and support decimal facebookincubator#245
Support date_add spark sql function facebookincubator#144
chenxu14 pushed a commit to chenxu14/velox that referenced this pull request Jul 5, 2023
relative pr:

Fix replace SparkSQL function facebookincubator#277
Support kPreceeding & kFollowing for window range frame type facebookincubator#287
support timestamp hash facebookincubator#269
Spark sum can overflow facebookincubator#101
Support float & double types in pmod function facebookincubator#157
Implement datetime functions in velox/sparksql. facebookincubator#81
Fix type check in MapFunction facebookincubator#273
Let function validation fail for lookaround pattern in RE2-based implementation facebookincubator#124
Register lpad/rpad functions for Spark SQL. facebookincubator#63
Support substring_index sql function facebookincubator#189
Fix First/Last aggregate functions intermediate type and support decimal facebookincubator#245
Support date_add spark sql function facebookincubator#144
PHILO-HE pushed a commit to PHILO-HE/velox that referenced this pull request Jul 17, 2023
relative pr:

Fix replace SparkSQL function facebookincubator#277
Support kPreceeding & kFollowing for window range frame type facebookincubator#287
support timestamp hash facebookincubator#269
Spark sum can overflow facebookincubator#101
Support float & double types in pmod function facebookincubator#157
Implement datetime functions in velox/sparksql. facebookincubator#81
Fix type check in MapFunction facebookincubator#273
Let function validation fail for lookaround pattern in RE2-based implementation facebookincubator#124
Register lpad/rpad functions for Spark SQL. facebookincubator#63
Support substring_index sql function facebookincubator#189
Fix First/Last aggregate functions intermediate type and support decimal facebookincubator#245
Support date_add spark sql function facebookincubator#144
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Jul 21, 2023
relative pr:

Fix replace SparkSQL function facebookincubator#277
Support kPreceeding & kFollowing for window range frame type facebookincubator#287
support timestamp hash facebookincubator#269
Spark sum can overflow facebookincubator#101
Support float & double types in pmod function facebookincubator#157
Implement datetime functions in velox/sparksql. facebookincubator#81
Fix type check in MapFunction facebookincubator#273
Let function validation fail for lookaround pattern in RE2-based implementation facebookincubator#124
Register lpad/rpad functions for Spark SQL. facebookincubator#63
Support substring_index sql function facebookincubator#189
Fix First/Last aggregate functions intermediate type and support decimal facebookincubator#245
Support date_add spark sql function facebookincubator#144
rui-mo pushed a commit to rui-mo/velox that referenced this pull request Jul 24, 2023
relative pr:

Fix replace SparkSQL function facebookincubator#277
Support kPreceeding & kFollowing for window range frame type facebookincubator#287
support timestamp hash facebookincubator#269
Spark sum can overflow facebookincubator#101
Support float & double types in pmod function facebookincubator#157
Implement datetime functions in velox/sparksql. facebookincubator#81
Fix type check in MapFunction facebookincubator#273
Let function validation fail for lookaround pattern in RE2-based implementation facebookincubator#124
Register lpad/rpad functions for Spark SQL. facebookincubator#63
Support substring_index sql function facebookincubator#189
Fix First/Last aggregate functions intermediate type and support decimal facebookincubator#245
Support date_add spark sql function facebookincubator#144
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants