-
Notifications
You must be signed in to change notification settings - Fork 134
[CBRD-26457] Fixed an issue where adddate() and subdate() results would be inaccurate when a leap year was included #6728
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
Draft
ctshim
wants to merge
1
commit into
CUBRID:develop
Choose a base branch
from
ctshim:CBRD-26457_fix-adddate_subdate
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19682,15 +19682,17 @@ add_and_normalize_date_time (int *year, int *month, int *day, int *hour, int *mi | |
| _m = 1; | ||
|
|
||
| /* days for years */ | ||
| while (_d >= 366) | ||
|
|
||
| int maxdays = (days[2] == 29) ? 366 : 365; | ||
childyouth marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| while (_d >= maxdays) | ||
| { | ||
| days[2] = LEAP (_y) ? 29 : 28; | ||
| _d -= (days[2] == 29) ? 366 : 365; | ||
| _d -= maxdays; | ||
| _y++; | ||
| if (_y > 9999) | ||
| { | ||
| goto set_and_return; | ||
| } | ||
| maxdays = LEAP (_y) ? 366 : 365; | ||
| } | ||
|
|
||
| /* days within a year */ | ||
|
|
@@ -19707,11 +19709,14 @@ add_and_normalize_date_time (int *year, int *month, int *day, int *hour, int *mi | |
|
|
||
| if (_m == 0) | ||
| { | ||
| assert (false); | ||
| _m = 1; | ||
| } | ||
| if (_d == 0) | ||
| { | ||
| _d = 1; | ||
| _y--; | ||
| _m = (_m == 1) ? 12 : (_m - 1); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _d가 0인 경우는 /* days within a year */ 루프 전부터 _d = 0인 경우밖에 없을 것 같습니다. 해당 경우엔 항상 _y--; _m = 12; 일 것 같습니다. |
||
| _d = days[_m]; | ||
| } | ||
|
|
||
| set_and_return: | ||
|
|
@@ -19875,8 +19880,8 @@ sub_and_normalize_date_time (int *year, int *month, int *day, int *hour, int *mi | |
| { | ||
| goto set_and_return; | ||
| } | ||
| days[2] = LEAP (_y) ? 29 : 28; | ||
| _d += (days[2] == 29) ? 366 : 365; | ||
|
|
||
| _d += (LEAP (_y) ? 366 : 365); | ||
| } | ||
|
|
||
| /* days within a year */ | ||
|
|
@@ -19897,7 +19902,9 @@ sub_and_normalize_date_time (int *year, int *month, int *day, int *hour, int *mi | |
| } | ||
| if (_d == 0) | ||
| { | ||
| _d = 1; | ||
| _y--; | ||
| _m = (_m == 1) ? 12 : (_m - 1); | ||
| _d = days[_m]; | ||
| } | ||
|
|
||
| set_and_return: | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
이슈와 별개로 해당 부분을 삭제할 수 있을 거 같습니다.
Uh oh!
There was an error while loading. Please reload this page.
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.
19693 라인의 goto 문에서 19722 라인을 거쳐 19730 라인에서 _m 을 사용하기 때문에 삭제할 수 없을 것 같습니다.
19693 라인에서 언제나 _m 이 1 이라고 볼 수 없을 것 같습니다.
단, 18700 라인의 for 문에서 _m = 1 은 지울 수 있을 것 같습니다.
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.
19693라인의 goto 문을 통했을 때, 19730 라인을 갈 수 있나요?
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.
컴파일러가 최적화 과정에서 for 문의 _m = 1 을 삭제할 수는 있겠지만,
readability 를 위해서 생략하지 않는 것이 좋겠습니다.
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.
@kangmin5505 갈 수 있지요.
Uh oh!
There was an error while loading. Please reload this page.
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.
_m = 1;를goto set_and_return;앞으로 옮기도록 하겠습니다.이렇게 되면 가독성 문제도 해결 되고, 기존과 동일한 결과도 기대할 수 있겠네요
그런데 이 경우에는 어차피 에러를 리턴하고
_m은 사용되지 않는군요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.
goto 직전 검사하는 조건을 볼 때 19730 라인을 갈 수 없다는 것을 알게 되었습니다.
혼란을 드려 죄송합니다.