-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
evalengine: Implement LOCATE and friends #15195
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
to = len(src) | ||
} | ||
return src[from:to] | ||
} |
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.
Added a number of these optimized / simplified implementations where possible as well (we check for this interface when slicing in the helper).
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #15195 +/- ##
===========================================
+ Coverage 47.29% 67.36% +20.06%
===========================================
Files 1137 1560 +423
Lines 238684 192576 -46108
===========================================
+ Hits 112895 129733 +16838
+ Misses 117168 62843 -54325
+ Partials 8621 0 -8621 ☔ View full report in Codecov by Sentry. |
@@ -719,7 +725,7 @@ func (ast *astCompiler) translateCallable(call sqlparser.Callable) (IR, error) { | |||
|
|||
case *sqlparser.CurTimeFuncExpr: | |||
if call.Fsp > 6 { | |||
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Too-big precision 12 specified for '%s'. Maximum is 6.", call.Name.String()) | |||
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Too-big precision %d specified for '%s'. Maximum is 6.", call.Fsp, call.Name.String()) |
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.
This was incorrectly hardcoding a value instead of using the one provided by the user.
a18d001
to
93316a1
Compare
@@ -282,8 +282,8 @@ func (b *builtinDateFormat) eval(env *ExpressionEnv) (eval, error) { | |||
case *evalTemporal: | |||
t = e.toDateTime(datetime.DefaultPrecision, env.now) | |||
default: | |||
t = evalToDateTime(date, datetime.DefaultPrecision, env.now, env.sqlmode.AllowZeroDate()) | |||
if t == nil || t.isZero() { |
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.
Did a little cleanup of the zero date handling as well, since it's equivalent to convert not allowing zero dates if we check against nil
and the zero date anyway.
13a3d83
to
4416195
Compare
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.
Looks great. I love the zero-date cleanups. 🙇
This implements `LOCATE`, `POSITION` and `INSTR` to find substrings inside another string. It diverges in behavior for multibyte characters because of the bugs in MySQL identified in https://bugs.mysql.com/bug.php?id=113933. Signed-off-by: Dirkjan Bussink <[email protected]>
We have a bunch of cases where we convert to zero date with the runtime flag, but then check both nil and zero date. It's equivalent to convert without allowing zero dates to simplify the code here. Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Dirkjan Bussink <[email protected]>
4416195
to
789bbad
Compare
This implements
LOCATE
,POSITION
andINSTR
to find substrings inside another string.It diverges in behavior for multibyte characters because of the bugs in MySQL identified in https://bugs.mysql.com/bug.php?id=113933.
Related Issue(s)
Part of #9647
Checklist