*:fix a bunch of functions. #3805

ywqzzy · 2022-01-05T07:25:36Z

What problem does this PR solve?

Issue Number: #2315 #2843

Problem Summary:

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Release note

None

ti-chi-bot · 2022-01-05T07:25:37Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

fuzhe1989

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ywqzzy · 2022-01-05T07:26:14Z

/cc @fuzhe1989 @SeaRise @JaySon-Huang

dbms/src/Functions/FunctionsConditional.h

dbms/src/Functions/FunctionsComparison.h

dbms/src/Functions/FunctionsConditional.h

dbms/src/Functions/FunctionsDateTime.h

ywqzzy · 2022-01-11T07:12:26Z

/run-all-tests

sre-bot · 2022-01-11T07:47:31Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/929/cobertura/
(Coverage detail url is limited office network access)

lines: 43.9% (49989 out of 113973)
branches: 6.5% (81346 out of 1257030)

JaySon-Huang · 2022-01-12T13:54:28Z

dbms/src/Functions/FunctionsComparison.h

@@ -1565,8 +1562,7 @@ class FunctionComparison : public IFunction
        bool right_is_fixed_string = false;
        const DataTypeTuple * right_tuple = nullptr;

-        false
-            || (right_is_date = checkAndGetDataType<DataTypeDate>(arguments[1].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[1].get()))
+        right_is_date = checkAndGetDataType<DataTypeDate>(arguments[1].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[1].get())


Suggested change

right_is_date = checkAndGetDataType<DataTypeDate>(arguments[1].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[1].get())

(right_is_date = checkAndGetDataType<DataTypeDate>(arguments[1].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[1].get()))

I think we need parentheses here?

JaySon-Huang · 2022-01-12T13:54:41Z

dbms/src/Functions/FunctionsComparison.h

@@ -1546,8 +1544,7 @@ class FunctionComparison : public IFunction
        bool left_is_fixed_string = false;
        const DataTypeTuple * left_tuple = nullptr;

-        false
-            || (left_is_date = checkAndGetDataType<DataTypeDate>(arguments[0].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[0].get()))
+        left_is_date = checkAndGetDataType<DataTypeDate>(arguments[0].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[0].get())


Suggested change

left_is_date = checkAndGetDataType<DataTypeDate>(arguments[0].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[0].get())

(left_is_date = checkAndGetDataType<DataTypeDate>(arguments[0].get()) || checkAndGetDataType<DataTypeMyDate>(arguments[0].get()))

I think we need parentheses here?

dbms/src/Functions/FunctionsComparison.h

dbms/src/Functions/FunctionsDateTime.h

Co-authored-by: JaySon <[email protected]>

…wqzzy/tics into clean_functionBinaryArithmetic

ywqzzy · 2022-01-13T02:52:38Z

/run-all-tests

sre-bot · 2022-01-13T03:24:55Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/988/cobertura/
(Coverage detail url is limited office network access)

lines: 43.9% (49989 out of 113973)
branches: 6.5% (81346 out of 1257002)

JaySon-Huang · 2022-01-17T03:45:05Z

/merge

ti-chi-bot · 2022-01-17T03:45:05Z

@JaySon-Huang: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

ti-chi-bot · 2022-01-17T03:45:11Z

This pull request has been accepted and is ready to merge.

Commit hash: 2ec0991

SeaRise · 2022-01-17T04:02:36Z

/merge

ti-chi-bot · 2022-01-17T04:02:37Z

@SeaRise: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

SeaRise · 2022-01-17T04:03:32Z

/build

sre-bot · 2022-01-17T04:31:53Z

Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/1013/cobertura/
(Coverage detail url is limited office network access)

lines: 44.7% (51502 out of 115232)
branches: 7.0% (89805 out of 1275534)

ywqzzy added 2 commits January 5, 2022 14:22

update.

c8e8aff

update.

36c5ef6

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 5, 2022

ti-chi-bot requested review from fuzhe1989, JaySon-Huang and SeaRise January 5, 2022 07:26

ywqzzy commented Jan 5, 2022

View reviewed changes

dbms/src/Functions/FunctionsConditional.h Outdated Show resolved Hide resolved

Update dbms/src/Functions/FunctionsConditional.h

a9fdf35

ywqzzy commented Jan 5, 2022

View reviewed changes

dbms/src/Functions/FunctionsComparison.h Outdated Show resolved Hide resolved

Update dbms/src/Functions/FunctionsComparison.h

8c34a9d

ywqzzy commented Jan 5, 2022

View reviewed changes

dbms/src/Functions/FunctionsConditional.h Outdated Show resolved Hide resolved

dbms/src/Functions/FunctionsDateTime.h Outdated Show resolved Hide resolved

dbms/src/Functions/FunctionsDateTime.h Outdated Show resolved Hide resolved

ywqzzy added 4 commits January 5, 2022 16:13

Update dbms/src/Functions/FunctionsConditional.h

635a53a

Update dbms/src/Functions/FunctionsDateTime.h

16335f0

Update dbms/src/Functions/FunctionsDateTime.h

a24785f

Merge branch 'master' into clean_functionBinaryArithmetic

df68ee8

fuzhe1989 approved these changes Jan 11, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 11, 2022

JaySon-Huang reviewed Jan 12, 2022

View reviewed changes

dbms/src/Functions/FunctionsDateTime.h Outdated Show resolved Hide resolved

ywqzzy and others added 4 commits January 12, 2022 22:25

Update dbms/src/Functions/FunctionsComparison.h

6e95cf8

Co-authored-by: JaySon <[email protected]>

Update dbms/src/Functions/FunctionsDateTime.h

6492da6

Co-authored-by: JaySon <[email protected]>

update.

730f9e8

Merge branch 'clean_functionBinaryArithmetic' of https://github.com/y…

2ec0991

…wqzzy/tics into clean_functionBinaryArithmetic

ywqzzy requested a review from JaySon-Huang January 13, 2022 02:34

SeaRise approved these changes Jan 15, 2022

View reviewed changes

JaySon-Huang approved these changes Jan 17, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 17, 2022

Merge branch 'master' into clean_functionBinaryArithmetic

f6ac8fb

ti-chi-bot merged commit 2a18ed1 into pingcap:master Jan 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*:fix a bunch of functions. #3805

*:fix a bunch of functions. #3805

ywqzzy commented Jan 5, 2022 •

edited

Loading

ti-chi-bot commented Jan 5, 2022 •

edited

Loading

ywqzzy commented Jan 5, 2022

ywqzzy commented Jan 11, 2022

sre-bot commented Jan 11, 2022

JaySon-Huang Jan 12, 2022

JaySon-Huang Jan 12, 2022

ywqzzy commented Jan 13, 2022

sre-bot commented Jan 13, 2022

JaySon-Huang commented Jan 17, 2022

ti-chi-bot commented Jan 17, 2022

ti-chi-bot commented Jan 17, 2022

SeaRise commented Jan 17, 2022

ti-chi-bot commented Jan 17, 2022

SeaRise commented Jan 17, 2022

sre-bot commented Jan 17, 2022

	right_is_date = checkAndGetDataType<DataTypeDate>(arguments[1].get()) \|\| checkAndGetDataType<DataTypeMyDate>(arguments[1].get())
	(right_is_date = checkAndGetDataType<DataTypeDate>(arguments[1].get()) \|\| checkAndGetDataType<DataTypeMyDate>(arguments[1].get()))

	left_is_date = checkAndGetDataType<DataTypeDate>(arguments[0].get()) \|\| checkAndGetDataType<DataTypeMyDate>(arguments[0].get())
	(left_is_date = checkAndGetDataType<DataTypeDate>(arguments[0].get()) \|\| checkAndGetDataType<DataTypeMyDate>(arguments[0].get()))

*:fix a bunch of functions. #3805

*:fix a bunch of functions. #3805

Conversation

ywqzzy commented Jan 5, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jan 5, 2022 • edited Loading

ywqzzy commented Jan 5, 2022

ywqzzy commented Jan 11, 2022

sre-bot commented Jan 11, 2022

JaySon-Huang Jan 12, 2022

Choose a reason for hiding this comment

JaySon-Huang Jan 12, 2022

Choose a reason for hiding this comment

ywqzzy commented Jan 13, 2022

sre-bot commented Jan 13, 2022

JaySon-Huang commented Jan 17, 2022

ti-chi-bot commented Jan 17, 2022

ti-chi-bot commented Jan 17, 2022

SeaRise commented Jan 17, 2022

ti-chi-bot commented Jan 17, 2022

SeaRise commented Jan 17, 2022

sre-bot commented Jan 17, 2022

ywqzzy commented Jan 5, 2022 •

edited

Loading

ti-chi-bot commented Jan 5, 2022 •

edited

Loading