expression: fix incorrect result of logical operators when isTrue/ isFalse function is pushed down by Reminiscent · Pull Request #15926 · pingcap/tidb

Reminiscent · 2020-03-31T13:11:36Z

What problem does this PR solve?

Close: #15718 #15987
Related PR: #12173
Need to review TiPB176 first
Related to #13962

What is changed and how it works?

See an example of this comment
Before this PR:

MySQL [test]> select a and b as c, a, b from tt order by c limit 4;
+---+------+------+
| c | a    | b    |
+---+------+------+
| NULL |    7 | NULL |
| NULL | NULL | 2    |
| 0 |    0 | 2    |
| 0 | NULL | w    |
+---+------+------+
4 rows in set, 4 warnings (0.00 sec)

MySQL [test]> select a and b as c, a, b from tt order by c;
+---+------+------+
| c | a    | b    |
+---+------+------+
| NULL | NULL | NULL |
| NULL |    7 | NULL |
| NULL | NULL | 2    |
| NULL | NULL | 2    |
| 0 | NULL | w    |
| 0 |    0 | 2    |
| 0 |    7 | f    |
| 1 |    7 | 3    |
+---+------+------+
8 rows in set, 2 warnings (0.01 sec)

After this PR:

mysql> create table tt(a decimal(10, 0), b varchar(1));
Query OK, 0 rows affected (0.02 sec)

mysql> insert into tt values(0, '2'), (7, null), (7, '3'), (NULL, 'w'), (NULL, '2'), (NULL, '2'), (NULL, NULL), (7, 'f');
Query OK, 8 rows affected (0.00 sec)
Records: 8  Duplicates: 0  Warnings: 0

mysql> select a and b as c, a, b from tt order by c limit 4;
+---+------+------+
| c | a    | b    |
+---+------+------+
| NULL | NULL | NULL |
| NULL | NULL | 2    |
| NULL | NULL | 2    |
| NULL |    7 | NULL |
+---+------+------+
4 rows in set, 2 warnings (0.01 sec)

mysql> select a and b as c, a, b from tt order by c;
+---+------+------+
| c | a    | b    |
+---+------+------+
| NULL | NULL | NULL |
| NULL |    7 | NULL |
| NULL | NULL | 2    |
| NULL | NULL | 2    |
| 0 | NULL | w    |
| 0 |    0 | 2    |
| 0 |    7 | f    |
| 1 |    7 | 3    |
+---+------+------+
8 rows in set, 2 warnings (0.00 sec)

The cause of this problem is that when the order by expression in the topN executor is logicalAnd or logicalOr, keepNull should be set to true when the topN executor is pushed down.

Check List

Tests

Unit test
Integration test

codecov · 2020-03-31T13:18:35Z

Codecov Report

Merging #15926 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15926   +/-   ##
===========================================
  Coverage   80.3437%   80.3437%           
===========================================
  Files           506        506           
  Lines        136618     136618           
===========================================
  Hits         109764     109764           
  Misses        18276      18276           
  Partials       8578       8578

XuHuaiyu

LGTM

wshwsh12

LGTM

XuHuaiyu · 2020-04-01T08:28:36Z

/merge

sre-bot · 2020-04-01T08:28:39Z

Your auto merge job has been accepted, waiting for 15907, 15845, 15758, 15956, 15776, 15889

sre-bot · 2020-04-01T08:54:01Z

/run-all-tests

sre-bot · 2020-04-01T08:57:30Z

@Reminiscent merge failed.

XuHuaiyu · 2020-04-01T09:15:27Z

@Reminiscent CI failed, please check it.

expression/distsql_builtin.go

XuHuaiyu · 2020-04-16T06:44:10Z

Is this expectant?

[2020-04-16T06:33:09.000Z] FAIL: const_test.go:180: testMySQLConstSuite.TestHighNotPrecedenceMode

[2020-04-16T06:33:09.000Z] 

[2020-04-16T06:33:09.000Z] const_test.go:191:

[2020-04-16T06:33:09.000Z]     r = tk.MustQuery(`SELECT * FROM t1 WHERE NOT a BETWEEN 2 AND 3;`)

[2020-04-16T06:33:09.000Z] /home/jenkins/agent/workspace/tidb_ghpr_check_2@2/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:288:

[2020-04-16T06:33:09.000Z]     tk.c.Check(err, check.IsNil, comment)

[2020-04-16T06:33:09.000Z] ... value *errors.fundamental = runtime error: invalid memory address or nil pointer dereference ("runtime error: invalid memory address or nil pointer dereference")

[2020-04-16T06:33:09.000Z] ... sql:SELECT * FROM t1 WHERE NOT a BETWEEN 2 AND 3;, args:[]

XuHuaiyu · 2020-04-16T09:20:59Z

[2020-04-16T09:05:14.887Z] PANIC: distsql_builtin_test.go:159: testEvalSuite.TestEval

[2020-04-16T09:05:14.887Z] 

[2020-04-16T09:05:14.887Z] ... Panic: interface conversion: proto.Message is nil, not *tipb.InUnionMetadata (PC=0xFAA711)

[2020-04-16T09:05:14.887Z] 

[2020-04-16T09:05:14.887Z] /usr/local/go/src/runtime/panic.go:679

[2020-04-16T09:05:14.887Z]   in gopanic

[2020-04-16T09:05:14.887Z] /usr/local/go/src/runtime/iface.go:255

[2020-04-16T09:05:14.887Z]   in panicdottypeE

[2020-04-16T09:05:14.887Z] /usr/local/go/src/runtime/iface.go:265

[2020-04-16T09:05:14.887Z]   in panicdottypeI

[2020-04-16T09:05:14.887Z] distsql_builtin.go:74

[2020-04-16T09:05:14.887Z]   in getSignatureByPB

[2020-04-16T09:05:14.887Z] distsql_builtin.go:1056

[2020-04-16T09:05:14.887Z]   in newDistSQLFunctionBySig

[2020-04-16T09:05:14.887Z] distsql_builtin.go:1138

[2020-04-16T09:05:14.887Z]   in PBToExpr

[2020-04-16T09:05:14.887Z] distsql_builtin_test.go:856

[2020-04-16T09:05:14.887Z]   in testEvalSuite.TestEval

[2020-04-16T09:05:14.887Z] /usr/local/go/src/reflect/value.go:321

[2020-04-16T09:05:14.887Z]   in Value.Call

[2020-04-16T09:05:14.887Z] /home/jenkins/agent/workspace/tidb_ghpr_check_2@2/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:850

[2020-04-16T09:05:14.887Z]   in suiteRunner.forkTest.func1

[2020-04-16T09:05:14.887Z] /home/jenkins/agent/workspace/tidb_ghpr_check_2@2/go/pkg/mod/github.com/pingcap/check@v0.0.0-20200212061837-5e12011dc712/check.go:739

[2020-04-16T09:05:14.887Z]   in suiteRunner.forkCall.func1

[2020-04-16T09:05:14.887Z] /usr/local/go/src/runtime/asm_amd64.s:1357

[2020-04-16T09:05:14.887Z]   in goexit

Reminiscent · 2020-04-16T09:47:06Z

@XuHuaiyu @breeswish PTAL. Thanks!

XuHuaiyu

LGTM

XuHuaiyu · 2020-04-16T09:50:34Z

/merge

sre-bot · 2020-04-16T09:50:47Z

/run-all-tests

sre-bot · 2020-04-16T09:54:55Z

/run-all-tests

sre-bot · 2020-04-16T09:56:57Z

@Reminiscent merge failed.

breezewish · 2020-04-17T02:49:08Z

Do we need to fix at TiKV side as well? Otherwise the results will be different so that copr-test will fail.

Reminiscent · 2020-04-17T07:59:37Z

After an offline discussion, we decided to use another method to solve this problem. We will close this PR and reopen a new one.

expression: fix incorrect result of logical operators

0b6abb0

Reminiscent added type/bugfix This PR fixes a bug. component/expression needs-cherry-pick-4.0 labels Mar 31, 2020

Reminiscent requested review from SunRunAway, XuHuaiyu and qw4990 March 31, 2020 13:11

Reminiscent requested a review from a team as a code owner March 31, 2020 13:11

ghost removed their request for review March 31, 2020 13:11

github-actions bot added the sig/execution SIG execution label Mar 31, 2020

XuHuaiyu reviewed Apr 1, 2020

View reviewed changes

wshwsh12 previously approved these changes Apr 1, 2020

View reviewed changes

sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 1, 2020

XuHuaiyu added the status/LGT2 Indicates that a PR has LGTM 2. label Apr 1, 2020

Merge branch 'master' into 15718

4559105

Merge branch 'master' of https://github.com/pingcap/tidb into 15718

faa0c73

Reminiscent mentioned this pull request Apr 2, 2020

OR clause on FLOAT/DOUBLE column unexpectedly evaluates to TRUE #15987

Closed

XuHuaiyu added the status/WIP label Apr 3, 2020

XuHuaiyu removed request for SunRunAway and qw4990 April 3, 2020 02:17

zz-jason assigned Reminiscent Apr 4, 2020

Reminiscent added 3 commits April 7, 2020 13:25

add baseBuiltinIsTrueOrFalseFunc

ee2b78a

add metadata to mocktikv

d674868

Merge branch 'master' of https://github.com/pingcap/tidb into 15718

a97034c

XuHuaiyu reviewed Apr 14, 2020

View reviewed changes

expression/distsql_builtin.go Show resolved Hide resolved

Reminiscent added 2 commits April 14, 2020 17:29

Merge branch 'master' of https://github.com/pingcap/tidb into 15718

6c4d9af

address comments

367722e

XuHuaiyu mentioned this pull request Apr 16, 2020

A NOT NULL predicate unexpectedly evaluates to TRUE #16440

Closed

Reminiscent requested a review from XuHuaiyu April 16, 2020 06:25

Reminiscent added 2 commits April 16, 2020 14:27

Merge branch 'master' of https://github.com/pingcap/tidb into 15718

6484171

run go mod tidy

fcb0d25

Reminiscent added 2 commits April 16, 2020 17:02

fix unit-test

bb6e839

Merge branch 'master' of https://github.com/pingcap/tidb into 15718

f44be47

fix unit-test

5fa7b5e

XuHuaiyu approved these changes Apr 16, 2020

View reviewed changes

sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 16, 2020

XuHuaiyu added the status/LGT2 Indicates that a PR has LGTM 2. label Apr 16, 2020

Merge branch 'master' into 15718

d156214

Reminiscent closed this Apr 17, 2020

Reminiscent mentioned this pull request Apr 17, 2020

expression: split isTrue and isFalse expression sig #16542

Merged

Reminiscent deleted the 15718 branch August 5, 2021 02:02

Conversation

Reminiscent commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

codecov bot commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

XuHuaiyu left a comment

Choose a reason for hiding this comment

Uh oh!

wshwsh12 left a comment

Choose a reason for hiding this comment

Uh oh!

XuHuaiyu commented Apr 1, 2020

Uh oh!

sre-bot commented Apr 1, 2020

Uh oh!

sre-bot commented Apr 1, 2020

Uh oh!

sre-bot commented Apr 1, 2020

Uh oh!

XuHuaiyu commented Apr 1, 2020

Uh oh!

Uh oh!

XuHuaiyu commented Apr 16, 2020

Uh oh!

XuHuaiyu commented Apr 16, 2020

Uh oh!

Reminiscent commented Apr 16, 2020

Uh oh!

XuHuaiyu left a comment

Choose a reason for hiding this comment

Uh oh!

XuHuaiyu commented Apr 16, 2020

Uh oh!

sre-bot commented Apr 16, 2020

Uh oh!

sre-bot commented Apr 16, 2020

Uh oh!

sre-bot commented Apr 16, 2020

Uh oh!

breezewish commented Apr 17, 2020

Uh oh!

Reminiscent commented Apr 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Reminiscent commented Mar 31, 2020 •

edited

Loading

codecov bot commented Mar 31, 2020 •

edited

Loading