-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
expression: implement vectorized evaluation for builtinTimestamp1ArgSig
#12339
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12339 +/- ##
===========================================
Coverage 80.8854% 80.8854%
===========================================
Files 454 454
Lines 99971 99971
===========================================
Hits 80862 80862
Misses 13323 13323
Partials 5786 5786 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The performance is really poor compared with other builtin functions 😂 |
if err = handleInvalidTimeError(b.ctx, err); err != nil { | ||
return err | ||
} | ||
result.SetNull(i, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about add an error rate option in generator to cover this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update. PTAL @SunRunAway
…Timestamp1ArgSig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
What problem does this PR solve?
implement vectorized evaluation for
builtinTimestamp1ArgSig
To: #12101
What is changed and how it works?
Check List
Tests