expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type #11343

AndrewDi · 2019-07-19T13:34:04Z

What problem does this PR solve?

Fix issue #11332

What is changed and how it works?

IF scalefunc is MAX or MIN, remove not null flag.

Check List

Tests

Unit test

codecov · 2019-07-20T13:59:48Z

Codecov Report

Merging #11343 into master will increase coverage by 0.3978%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             master     #11343        +/-   ##
================================================
+ Coverage   81.2663%   81.6642%   +0.3978%     
================================================
  Files           426        426                
  Lines         92075      93473      +1398     
================================================
+ Hits          74826      76334      +1508     
+ Misses        11880      11786        -94     
+ Partials       5369       5353        -16

AndrewDi · 2019-07-20T14:01:27Z

@winkyao @XuHuaiyu ExplainTest error output is too anti-human....
PTAL

AndrewDi · 2019-07-20T14:02:31Z

/run-all-tests

XuHuaiyu · 2019-07-22T06:19:40Z

expression/aggregation/base_func.go

@@ -184,6 +184,10 @@ func (a *baseFuncDesc) typeInfer4MaxMin(ctx sessionctx.Context) {
 		a.Args[0] = expression.BuildCastFunction(ctx, a.Args[0], tp)
 	}
 	a.RetTp = a.Args[0].GetType()
+	if (a.Name == ast.AggFuncMax || a.Name == ast.AggFuncMin) && !a.RetTp.Hybrid() {


Why do we need to check !a.RetTp.Hybrid()?

Hybird type is special

It seems like someone has modify a.RetTp = a.Args[0].GetType() when process Bit type.

expression/aggregation/base_func_test.go

winoros · 2019-07-23T05:46:31Z

expression/aggregation/base_func.go

@@ -184,6 +184,10 @@ func (a *baseFuncDesc) typeInfer4MaxMin(ctx sessionctx.Context) {
 		a.Args[0] = expression.BuildCastFunction(ctx, a.Args[0], tp)
 	}
 	a.RetTp = a.Args[0].GetType()
+	if (a.Name == ast.AggFuncMax || a.Name == ast.AggFuncMin) && !a.RetTp.Hybrid() {
+		a.RetTp = a.Args[0].GetType().Clone()
+		a.RetTp.Flag ^= mysql.NotNullFlag


^= is xor. It will filp that bit instead of resetting that bit.

@AndrewDi If we reset NotNullFlag instead of using xor, HybridType is not needed to be checked?

OK, change to a.RetTp.Flag &^= mysql.NotNullFlag

@XuHuaiyu a.RetTp = a.Args[0].GetType().Clone() will break current Hybird type process.

I do not really understand what's your meaning. @AndrewDi

@XuHuaiyu TypeBit should always not null, so we should not reset flag for this type.

zz-jason · 2019-07-23T13:13:09Z

It's weird, the last commit of that pr passed these tests: a4b3366

…11332

AndrewDi · 2019-07-23T13:20:35Z

It's weird, the last commit of that pr passed these tests: a4b3366

My mistake, already fix.

AndrewDi · 2019-07-23T13:24:04Z

/run-all-tests

sre-bot · 2019-07-29T08:50:27Z

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

AndrewDi · 2019-08-02T12:21:28Z

@XuHuaiyu PTAL

zz-jason

LGTM

XuHuaiyu

LGTM

XuHuaiyu · 2019-08-05T09:34:38Z

/run-all-tests

sre-bot · 2019-08-05T09:34:51Z

/run-all-tests

sre-bot · 2019-08-05T09:44:31Z

cherry pick to release-2.1 failed

sre-bot · 2019-08-05T09:47:18Z

cherry pick to release-3.0 in PR #11617

…inferring type (pingcap#11343)

…inferring type (#11343) (#11641)

sre-bot · 2020-04-07T10:22:19Z

It seems that, not for sure, we failed to cherry-pick this commit to release-2.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @XuHuaiyu PTAL.

XuHuaiyu · 2020-04-08T02:16:54Z

/run-cherry-picker

Signed-off-by: sre-bot <sre-bot@pingcap.com>

sre-bot · 2020-04-08T02:18:07Z

cherry pick to release-2.1 in PR #16140

XuHuaiyu · 2020-04-08T07:33:37Z

No need to cherry-pick to release-2.1 because this problem does not exist in release-2.1.

fix issue 11332

9cc16a4

AndrewDi force-pushed the fix_issue_11332 branch from 1da5962 to 9cc16a4 Compare July 20, 2019 02:07

AndrewDi added 2 commits July 20, 2019 20:14

fix ci

06e1984

fix ci

21dcca4

winkyao added the component/expression label Jul 20, 2019

AndrewDi added 2 commits July 20, 2019 21:51

fix ci

8a5ddf4

fix ci

c6a311c

Merge branch 'master' into fix_issue_11332

cdbd6f5

XuHuaiyu reviewed Jul 22, 2019

View reviewed changes

XuHuaiyu changed the title ~~expression: fix issue 11332~~ expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type Jul 22, 2019

XuHuaiyu added the type/bugfix This PR fixes a bug. label Jul 22, 2019

winoros reviewed Jul 23, 2019

View reviewed changes

zz-jason added needs-cherry-pick-2.1 contribution This PR is from a community contributor. labels Jul 23, 2019

AndrewDi and others added 2 commits July 23, 2019 21:03

fix suggestions

545bff7

Merge branch 'master' into fix_issue_11332

d32da48

AndrewDi and others added 2 commits July 23, 2019 21:15

Merge branch 'master' into fix_issue_11332

85f17f4

Merge remote-tracking branch 'origin/fix_issue_11332' into fix_issue_…

d5060fc

…11332

AndrewDi and others added 4 commits July 23, 2019 21:39

Merge branch 'master' into fix_issue_11332

4e71450

fix ci

ab4d1e0

fix ci

eb2674e

Merge branch 'master' into fix_issue_11332

3e567ab

XuHuaiyu requested a review from winoros July 29, 2019 08:45

Merge branch 'master' into fix_issue_11332

0cfec4b

zz-jason requested a review from XuHuaiyu August 5, 2019 09:28

zz-jason approved these changes Aug 5, 2019

View reviewed changes

zz-jason removed the request for review from XuHuaiyu August 5, 2019 09:29

zz-jason added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 5, 2019

XuHuaiyu approved these changes Aug 5, 2019

View reviewed changes

Merge branch 'master' into fix_issue_11332

5b70743

sre-bot merged commit 829ba98 into pingcap:master Aug 5, 2019

sre-bot mentioned this pull request Aug 5, 2019

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type (#11343) #11617

Closed

SunRunAway mentioned this pull request Aug 6, 2019

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type (#11343) #11631

Closed

SunRunAway pushed a commit to SunRunAway/tidb that referenced this pull request Aug 6, 2019

expression: remove the NotNullFlag for aggregation func MAX/MIN when …

7cf4d20

…inferring type (pingcap#11343)

SunRunAway mentioned this pull request Aug 6, 2019

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type (#11343) #11641

Merged

sre-bot pushed a commit that referenced this pull request Aug 8, 2019

expression: remove the NotNullFlag for aggregation func MAX/MIN when …

00801a7

…inferring type (#11343) (#11641)

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Apr 8, 2020

cherry pick pingcap#11343 to release-2.1

2fcd794

Signed-off-by: sre-bot <sre-bot@pingcap.com>

sre-bot mentioned this pull request Apr 8, 2020

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type (#11343) #16140

Closed

XuHuaiyu removed the needs-cherry-pick-2.1 label Apr 8, 2020

AndrewDi deleted the fix_issue_11332 branch February 1, 2021 04:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type #11343

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type #11343

AndrewDi commented Jul 19, 2019

codecov bot commented Jul 20, 2019 •

edited

Loading

AndrewDi commented Jul 20, 2019 •

edited

Loading

AndrewDi commented Jul 20, 2019

XuHuaiyu Jul 22, 2019

AndrewDi Jul 23, 2019

winoros Jul 23, 2019

XuHuaiyu Jul 23, 2019

AndrewDi Jul 23, 2019

AndrewDi Jul 23, 2019

XuHuaiyu Jul 24, 2019

AndrewDi Jul 26, 2019

zz-jason commented Jul 23, 2019

AndrewDi commented Jul 23, 2019

AndrewDi commented Jul 23, 2019

sre-bot commented Jul 29, 2019

AndrewDi commented Aug 2, 2019

zz-jason left a comment

XuHuaiyu left a comment

XuHuaiyu commented Aug 5, 2019

sre-bot commented Aug 5, 2019

sre-bot commented Aug 5, 2019

sre-bot commented Aug 5, 2019

sre-bot commented Apr 7, 2020

XuHuaiyu commented Apr 8, 2020

sre-bot commented Apr 8, 2020

XuHuaiyu commented Apr 8, 2020

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type #11343

expression: remove the NotNullFlag for aggregation func MAX/MIN when inferring type #11343

Conversation

AndrewDi commented Jul 19, 2019

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Jul 20, 2019 • edited Loading

Codecov Report

AndrewDi commented Jul 20, 2019 • edited Loading

AndrewDi commented Jul 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason commented Jul 23, 2019

AndrewDi commented Jul 23, 2019

AndrewDi commented Jul 23, 2019

sre-bot commented Jul 29, 2019

AndrewDi commented Aug 2, 2019

zz-jason left a comment

Choose a reason for hiding this comment

XuHuaiyu left a comment

Choose a reason for hiding this comment

XuHuaiyu commented Aug 5, 2019

sre-bot commented Aug 5, 2019

sre-bot commented Aug 5, 2019

sre-bot commented Aug 5, 2019

sre-bot commented Apr 7, 2020

XuHuaiyu commented Apr 8, 2020

sre-bot commented Apr 8, 2020

XuHuaiyu commented Apr 8, 2020

codecov bot commented Jul 20, 2019 •

edited

Loading

AndrewDi commented Jul 20, 2019 •

edited

Loading