-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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 builtinCastStringAsDurationSig
#13807
Conversation
…urationSig` expression: implement vectorized evaluation for `builtinCastStringAsDurationSig`
/rebuild |
Codecov Report
@@ Coverage Diff @@
## master #13807 +/- ##
===========================================
Coverage ? 80.1389%
===========================================
Files ? 474
Lines ? 117048
Branches ? 0
===========================================
Hits ? 93801
Misses ? 15844
Partials ? 7403 |
/rebuild |
@SilvaXiang, please update your pull request. |
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
Hi @SilvaXiang, please run |
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.
Rest LGTM.
expression/builtin_cast_vec_test.go
Outdated
@@ -51,7 +51,6 @@ var vecBuiltinCastCases = map[string][]vecExprBenchCase{ | |||
{retEvalType: types.ETDuration, childrenTypes: []types.EvalType{types.ETInt}, geners: []dataGenerator{new(randDurInt)}}, | |||
{retEvalType: types.ETDuration, childrenTypes: []types.EvalType{types.ETReal}, geners: []dataGenerator{new(randDurReal)}}, | |||
{retEvalType: types.ETDuration, childrenTypes: []types.EvalType{types.ETDecimal}, geners: []dataGenerator{new(randDurDecimal)}}, | |||
{retEvalType: types.ETDuration, childrenTypes: []types.EvalType{types.ETString}, geners: []dataGenerator{new(randDurString)}}, |
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.
Why remove this? We need this test case actually.
@SilvaXiang, please update your pull request. |
1 similar comment
@SilvaXiang, please update your pull request. |
/run-all-tests |
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
What problem does this PR solve?
expression: implement vectorized evaluation for
builtinCastStringAsDurationSig
from #12176What is changed and how it works?
2.7% faster than before:
Check List
Tests