types: remove param explicitTz from types.ParseTime
#48574
Conversation
ti-chi-bot
bot
added
release-note-none
lcwangchao
force-pushed
the
parse_time_remove_exp_tz
branch
from
713a9d3
to
d1487ab
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #48574 +/- ##
================================================
+ Coverage 71.4240% 72.8142% +1.3901%
================================================
Files 1404 1428 +24
Lines 407213 413827 +6614
================================================
+ Hits 290848 301325 +10477
+ Misses 96412 93536 -2876
+ Partials 19953 18966 -987
Flags with carried forward coverage won't be shown. Click here to find out more.
|
lcwangchao
force-pushed
the
parse_time_remove_exp_tz
branch
from
d1487ab
to
bfc4792
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
| @@ -89,8 +89,11 @@ func getTimeCurrentTimeStamp(ctx sessionctx.Context, tp byte, fsp int) (t types. | |||
| // GetTimeValue gets the time value with type tp. | |||
| func GetTimeValue(ctx sessionctx.Context, v interface{}, tp byte, fsp int, explicitTz *time.Location) (d types.Datum, err error) { | |||
| var value types.Time | |||
| tc := ctx.GetSessionVars().StmtCtx.TypeCtx() | |||
| if explicitTz != nil { | |||
| tc = tc.WithLocation(explicitTz) | |||
There was a problem hiding this comment.
tc.WithLocation(explicitTz) will create a new context with old one not modified. So there is no data race any more
| t1, err = tmp.GoTime(ctx.Location()) | ||
| } | ||
| if err != nil { | ||
| if t1, err = tmp.GoTime(ctx.Location()); err != nil { |
There was a problem hiding this comment.
We only need to care about ctx.Location() here after explicitTz removed
| } else { | ||
| t1 = t1.In(ctx.Location()) | ||
| } | ||
| t1 = t1.In(ctx.Location()) |
There was a problem hiding this comment.
We only need to care about ctx.Location() here after explicitTz removed
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3pointer, wjhuang2016, XuHuaiyu, YangKeao The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: close #48573
Problem Summary:
In #41146 a input parameter
explicitTzwas introduced to functiontypes.ParseTimeto avoid data race. Now after some refactoring works after47355, we usetypes.Contextwhich is immutable to replacestmtctx.StatementContext. So it is still thread-safe after we removeexplicitTz.What is changed and how it works?
Remove param
explicitTzfromtypes.ParseTimeto keep a clear semantic for functiontypes.ParseTime.We also remove
explicitTzinTime.checkand nowTime.checkandTime.Checkare all same. Then just removeTime.checkto useTime.CheckonlyCheck List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.