Skip to content
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.

Sign up for GitHub

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

Jump to bottom

expression: fix week func format #9685

Merged
merged 11 commits into from
Mar 13, 2019
Merged

expression: fix week func format #9685

merged 11 commits into from
Mar 13, 2019

Conversation

glock42
Copy link
Contributor

@glock42 glock42 commented Mar 12, 2019 •
edited by xiekeyi98
Loading

What problem does this PR solve?

Fix the problem that the variable default_week_format doesn't work.

fix #9669

What is changed and how it works?

Use the SystemVar DefaultWeekFormat to invoke the Week function.

Check List

Tests

  • Unit test
  • Integration test

@CLAassistant
Copy link

CLAassistant commented Mar 12, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Mar 12, 2019 •
edited
Loading

Codecov Report

Merging #9685 into master will decrease coverage by 0.0021%.
The diff coverage is 77.7777%.

@@               Coverage Diff               @@
##            master      #9685        +/-   ##
===============================================
- Coverage   67.375%   67.3728%   -0.0022%     
===============================================
  Files          377        377                
  Lines        79384      79391         +7     
===============================================
+ Hits         53485      53488         +3     
- Misses       21123      21125         +2     
- Partials      4776       4778         +2

xiekeyi98
xiekeyi98 reviewed Mar 13, 2019
View reviewed changes
Copy link
Contributor

@xiekeyi98 xiekeyi98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!
Would you mind adding some unit tests to catch this issue?

@glock42
Copy link
Contributor Author

glock42 commented Mar 13, 2019

@xiekeyi98 Already added.

@xiekeyi98
Copy link
Contributor

/run-all-tests

xiekeyi98
xiekeyi98 reviewed Mar 13, 2019
View reviewed changes
Copy link
Contributor

@xiekeyi98 xiekeyi98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xiekeyi98 xiekeyi98 changed the title fix week func format issure9669 expression: fix week func format Mar 13, 2019
@xiekeyi98 xiekeyi98 added status/LGT1 Indicates that a PR has LGTM 1. contribution This PR is from a community contributor. component/expression labels Mar 13, 2019
@glock42
Copy link
Contributor Author

glock42 commented Mar 13, 2019

@imtbkcat PTAL.

imtbkcat
imtbkcat approved these changes Mar 13, 2019
View reviewed changes
Copy link

@imtbkcat imtbkcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@imtbkcat
Copy link

/run-all-tests

@imtbkcat imtbkcat merged commit 133a1ee into pingcap:master Mar 13, 2019
@qw4990
Copy link
Contributor

qw4990 commented Mar 15, 2019

@zyycj Thanks for your contribution.
Could you please cherry-pick this PR into branch release-2.1?

glock42 added a commit to glock42/tidb that referenced this pull request Mar 15, 2019
@glock42
expression: fix week func format (pingcap#9685)
b8dd8b8
@glock42 glock42 mentioned this pull request Mar 15, 2019
Merged
@qw4990 qw4990 mentioned this pull request Mar 18, 2019
Closed
30 tasks
zz-jason pushed a commit that referenced this pull request Mar 25, 2019
@glock42 @zz-jason
expression: fix week func format (#9685) (#9753)
e2bcb88
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiekeyi98 xiekeyi98 xiekeyi98 left review comments

@imtbkcat imtbkcat imtbkcat approved these changes

Assignees
No one assigned
Labels
component/expression contribution This PR is from a community contributor. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Variable default_week_format doesn't work
5 participants
@glock42 @CLAassistant @xiekeyi98 @imtbkcat @qw4990