expression: wrong result of timestampadd(month,1,date '2024-01-31') #53101

xzhangxian1008 · 2024-05-08T06:18:29Z

What problem does this PR solve?

Issue Number: close #41052

Problem Summary:

What changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

xzhangxian1008 · 2024-05-08T06:19:01Z

/cc @XuHuaiyu @yibin87 @SeaRise

tiprow · 2024-05-08T06:19:25Z

Hi @xzhangxian1008. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

codecov · 2024-05-08T06:37:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.0256%. Comparing base (2b754c7) to head (4e080f5).
Report is 29 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #53101        +/-   ##
================================================
+ Coverage   72.4215%   75.0256%   +2.6040%     
================================================
  Files          1490       1493         +3     
  Lines        428984     438057      +9073     
================================================
+ Hits         310677     328655     +17978     
+ Misses        99041      89396      -9645     
- Partials      19266      20006       +740

Flag	Coverage Δ
integration	`49.4659% <0.0000%> (?)`
unit	`71.6518% <100.0000%> (+0.3504%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`53.9957% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`50.3521% <ø> (+9.0100%)`	⬆️

yibin87 · 2024-05-08T07:52:26Z

pkg/expression/builtin_time.go

@@ -6232,6 +6232,11 @@ func addUnitToTime(unit string, t time.Time, v float64) (time.Time, bool, error)
 			return tb, true, nil
 		}
 		tb = t.AddDate(0, int(v), 0)
+
+		// For corner case: timestampadd(month,1,date '2024-01-31') = "2024-02-29", timestampadd(month,1,date '2024-01-30') = "2024-02-29"
+		for tb.Month() != t.Month()+1 {


What happen if adding more than 1 month?

What happen if adding more than 1 month?

This is mysql: timestampadd(month,1,date '2024-01-31') = "2024-02-29"

This is tidb: timestampadd(month,1,date '2024-01-31') = "2024-03-02"

The month field of the result of timestampadd(month,1,date '2024-01-31') is always 1 more than the month field of input. Adding more than 1 month is unexpected.

xzhangxian1008 · 2024-05-09T01:07:41Z

/cc @SeaRise

pkg/expression/builtin_time.go

yibin87

Others LGTM

ti-chi-bot · 2024-05-09T09:53:53Z

@yibin87: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

Others LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

SeaRise · 2024-05-09T10:11:27Z

/cc @XuHuaiyu @windtalker
Need a LGTM from the expression owners.

windtalker

LGTM

ti-chi-bot · 2024-05-10T03:03:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: windtalker, XuHuaiyu, yibin87

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:

~~pkg/expression/OWNERS~~ [XuHuaiyu,windtalker]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-05-11T05:49:30Z

In response to a cherrypick label: new pull request created to branch release-8.1: #53194.

…53101) (#53194) close #41052

…ingcap#53101) close pingcap#41052

ti-chi-bot · 2024-05-21T01:39:36Z

In response to a cherrypick label: new pull request created to branch release-7.5: #53419.

ti-chi-bot · 2024-05-21T01:39:42Z

In response to a cherrypick label: new pull request created to branch release-7.1: #53420.

ti-chi-bot · 2024-05-21T01:39:46Z

In response to a cherrypick label: new pull request created to branch release-6.5: #53421.

ti-chi-bot · 2024-05-21T01:39:47Z

In response to a cherrypick label: new pull request created to branch release-6.1: #53422.

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2024-05-21T01:39:50Z

In response to a cherrypick label: new pull request created to branch release-5.4: #53423.

Signed-off-by: ti-chi-bot <[email protected]>

…53101) (#53423) close #41052

…53101) (#53420) close #41052

…53101) (#53422) close #41052

…53101) (#53421) close #41052

…53101) (#53419) close #41052

…ingcap#53101) close pingcap#41052

zanmato1984 · 2024-07-25T09:53:23Z

pkg/expression/builtin_time_test.go

@@ -2580,6 +2580,18 @@ func TestTimestampAdd(t *testing.T) {
 		{"MINUTE", 1.5, "1995-05-01 00:00:00", "1995-05-01 00:02:00"},
 		{"MINUTE", 1.5, "1995-05-01 00:00:00.000000", "1995-05-01 00:02:00"},
 		{"MICROSECOND", -100, "1995-05-01 00:00:00.0001", "1995-05-01 00:00:00"},
+
+		// issue41052
+		{"MONTH", 1, "2024-01-31", "2024-02-29 00:00:00"},


We should've at least had one case for negative month.

We should've at least had one case for negative month.

fixed in #54916

fix and add tests

cbed6e7

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 8, 2024

ti-chi-bot bot requested review from SeaRise, XuHuaiyu and yibin87 May 8, 2024 06:19

yibin87 reviewed May 8, 2024

View reviewed changes

xzhangxian1008 requested a review from yibin87 May 9, 2024 01:07

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label May 9, 2024

xzhangxian1008 added 2 commits May 9, 2024 14:07

more fix

8b6b410

fix

3b3d253

yibin87 reviewed May 9, 2024

View reviewed changes

pkg/expression/builtin_time.go Outdated Show resolved Hide resolved

yibin87 approved these changes May 9, 2024

View reviewed changes

ti-chi-bot bot requested a review from windtalker May 9, 2024 10:11

remove useless codes

4c5ebb2

windtalker approved these changes May 10, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 10, 2024

add comment

4e080f5

XuHuaiyu approved these changes May 10, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 10, 2024

xzhangxian1008 deleted the fix-41052 branch May 11, 2024 05:47

ti-chi-bot added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label May 11, 2024

ti-chi-bot mentioned this pull request May 11, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#53101) #53194

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request May 11, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#…

870c1fd

…53101) (#53194) close #41052

terry1purcell pushed a commit to terry1purcell/tidb that referenced this pull request May 17, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (p…

8434198

…ingcap#53101) close pingcap#41052

ti-chi-bot mentioned this pull request May 21, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#53101) #53419

Merged

13 tasks

This was referenced May 21, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#53101) #53420

Merged

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#53101) #53421

Merged

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request May 21, 2024

This is an automated cherry-pick of pingcap#53101

852fcf9

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request May 21, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#53101) #53422

Merged

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request May 21, 2024

This is an automated cherry-pick of pingcap#53101

392e5d9

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request May 21, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#53101) #53423

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request May 22, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#…

dce26c2

…53101) (#53423) close #41052

ti-chi-bot bot pushed a commit that referenced this pull request May 22, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#…

b511a45

…53101) (#53420) close #41052

ti-chi-bot bot pushed a commit that referenced this pull request May 22, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#…

9541258

…53101) (#53422) close #41052

ti-chi-bot bot pushed a commit that referenced this pull request May 22, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#…

e98aba5

…53101) (#53421) close #41052

ti-chi-bot bot pushed a commit that referenced this pull request May 22, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (#…

33cae3a

…53101) (#53419) close #41052

RidRisR pushed a commit to RidRisR/tidb that referenced this pull request May 23, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') (p…

db855c5

…ingcap#53101) close pingcap#41052

zanmato1984 reviewed Jul 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: wrong result of timestampadd(month,1,date '2024-01-31') #53101

expression: wrong result of timestampadd(month,1,date '2024-01-31') #53101

xzhangxian1008 commented May 8, 2024

xzhangxian1008 commented May 8, 2024

tiprow bot commented May 8, 2024

codecov bot commented May 8, 2024 •

edited

Loading

yibin87 May 8, 2024 •

edited

Loading

xzhangxian1008 May 8, 2024

xzhangxian1008 commented May 9, 2024

yibin87 left a comment

ti-chi-bot bot commented May 9, 2024

SeaRise commented May 9, 2024

windtalker left a comment

ti-chi-bot bot commented May 10, 2024

ti-chi-bot commented May 11, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

zanmato1984 Jul 25, 2024

xzhangxian1008 Jul 25, 2024

expression: wrong result of timestampadd(month,1,date '2024-01-31') #53101

expression: wrong result of timestampadd(month,1,date '2024-01-31') #53101

Conversation

xzhangxian1008 commented May 8, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

xzhangxian1008 commented May 8, 2024

tiprow bot commented May 8, 2024

codecov bot commented May 8, 2024 • edited Loading

Codecov Report

yibin87 May 8, 2024 • edited Loading

Choose a reason for hiding this comment

xzhangxian1008 May 8, 2024

Choose a reason for hiding this comment

xzhangxian1008 commented May 9, 2024

yibin87 left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented May 9, 2024

SeaRise commented May 9, 2024

windtalker left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented May 10, 2024

ti-chi-bot commented May 11, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

ti-chi-bot commented May 21, 2024

zanmato1984 Jul 25, 2024

Choose a reason for hiding this comment

xzhangxian1008 Jul 25, 2024

Choose a reason for hiding this comment

codecov bot commented May 8, 2024 •

edited

Loading

yibin87 May 8, 2024 •

edited

Loading