core: Fix confusing behaviors of FROM_BASE64 function in WHERE clause #28229

zhaoxugang · 2021-09-21T14:50:06Z

core: Fix confusing behaviors of FROM_BASE64 function in WHERE clause

What problem does this PR solve?

Issue Number: close #28154

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

Check List

Tests

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

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Coorercing the function `FROM_BASE64()` to a boolean value could return invalid results.

ti-chi-bot · 2021-09-21T14:50:09Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

morgo
xhebox

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.

morgo · 2021-09-21T21:13:27Z

MySQL 5.7 and MySQL 8.0 behave differently here:

mysql [localhost:5731] {msandbox} (test) > select * from t where from_base64('');
Empty set (0.00 sec)

mysql [localhost:5731] {msandbox} (test) > update t set a = 'def' where from_base64('');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:5731] {msandbox} (test) > select * from t where from_base64('invalidbase64');
Empty set (0.00 sec)

mysql [localhost:5731] {msandbox} (test) > update t set a = 'hig' where from_base64('invalidbase64');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:5731] {msandbox} (test) > select * from t where from_base64('test');
Empty set (0.00 sec)

mysql [localhost:5731] {msandbox} (test) > update t set a = 'xyz' where from_base64('test');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:5731] {msandbox} (test) > select * from t;
+------+
| a    |
+------+
| abc  |
+------+
1 row in set (0.01 sec)

Versus:

mysql [localhost:8024] {root} (test) > insert into t values('abc');
Query OK, 1 row affected (0.00 sec)

mysql [localhost:8024] {root} (test) > select * from t where from_base64('');
Empty set (0.00 sec)

mysql [localhost:8024] {root} (test) > update t set a = 'def' where from_base64('');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:8024] {root} (test) > select * from t where from_base64('invalidbase64');
Empty set (0.00 sec)

mysql [localhost:8024] {root} (test) > update t set a = 'hig' where from_base64('invalidbase64');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:8024] {root} (test) > select * from t where from_base64('test');
Empty set, 1 warning (0.00 sec) <-- the warning is a truncated value

mysql [localhost:8024] {root} (test) > update t set a = 'xyz' where from_base64('test');
ERROR 1292 (22007): Truncated incorrect DOUBLE value: '\xB5\xEB-'
mysql [localhost:8024] {root} (test) > select * from t;
+------+
| a    |
+------+
| abc  |
+------+
1 row in set (0.00 sec)

zhaoxugang · 2021-09-22T15:56:43Z

MySQL 5.7 and MySQL 8.0 behave differently here:

mysql [localhost:5731] {msandbox} (test) > select * from t where from_base64('');
Empty set (0.00 sec)

mysql [localhost:5731] {msandbox} (test) > update t set a = 'def' where from_base64('');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:5731] {msandbox} (test) > select * from t where from_base64('invalidbase64');
Empty set (0.00 sec)

mysql [localhost:5731] {msandbox} (test) > update t set a = 'hig' where from_base64('invalidbase64');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:5731] {msandbox} (test) > select * from t where from_base64('test');
Empty set (0.00 sec)

mysql [localhost:5731] {msandbox} (test) > update t set a = 'xyz' where from_base64('test');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:5731] {msandbox} (test) > select * from t;
+------+
| a    |
+------+
| abc  |
+------+
1 row in set (0.01 sec)

Versus:

mysql [localhost:8024] {root} (test) > insert into t values('abc');
Query OK, 1 row affected (0.00 sec)

mysql [localhost:8024] {root} (test) > select * from t where from_base64('');
Empty set (0.00 sec)

mysql [localhost:8024] {root} (test) > update t set a = 'def' where from_base64('');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:8024] {root} (test) > select * from t where from_base64('invalidbase64');
Empty set (0.00 sec)

mysql [localhost:8024] {root} (test) > update t set a = 'hig' where from_base64('invalidbase64');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0  Changed: 0  Warnings: 0

mysql [localhost:8024] {root} (test) > select * from t where from_base64('test');
Empty set, 1 warning (0.00 sec) <-- the warning is a truncated value

mysql [localhost:8024] {root} (test) > update t set a = 'xyz' where from_base64('test');
ERROR 1292 (22007): Truncated incorrect DOUBLE value: '\xB5\xEB-'
mysql [localhost:8024] {root} (test) > select * from t;
+------+
| a    |
+------+
| abc  |
+------+
1 row in set (0.00 sec)

I tried in mysql 8.0.21,following:
mysql> insert into t values('abc');
Query OK, 1 row affected (0.00 sec)

mysql> select * from t where from_base64('');
Empty set (0.00 sec)

mysql> update t set a = 'def' where from_base64('');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0 Changed: 0 Warnings: 0

mysql> select * from t where from_base64('invalidbase64');
Empty set (0.00 sec)

mysql> update t set a = 'hig' where from_base64('invalidbase64');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0 Changed: 0 Warnings: 0

mysql> select * from t where from_base64('test');
Empty set (0.00 sec)

mysql> update t set a = 'xyz' where from_base64('test');
Query OK, 0 rows affected (0.00 sec)
Rows matched: 0 Changed: 0 Warnings: 0

mysql> select * from t;
+------+
| a |
+------+
| abc |
+------+
1 row in set (0.01 sec)

mysql> select @@Version;
+-----------+
| @@Version |
+-----------+
| 8.0.21 |
+-----------+
1 row in set (0.00 sec)

zhaoxugang · 2021-09-23T16:40:20Z

/cc @morgo @xhebox

morgo

LGTM, I've updated the release note

xhebox

LGTM

xhebox · 2021-09-27T01:08:46Z

/merge

ti-chi-bot · 2021-09-27T01:08:48Z

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

Commit hash: 85a9953

xiongjiwei · 2021-09-28T06:15:19Z

planner/core/logical_plan_builder.go

@@ -987,7 +987,10 @@ func (b *PlanBuilder) buildSelection(ctx context.Context, p LogicalPlan, where a
 		for _, item := range cnfItems {
 			if con, ok := item.(*expression.Constant); ok && con.DeferredExpr == nil && con.ParamMarker == nil {
 				ret, _, err := expression.EvalBool(b.ctx, expression.CNFExprs{con}, chunk.Row{})
-				if err != nil || ret {
+				if err != nil {
+					return nil, errors.Trace(err)


return error here may cause problems

update t set a = 'b' where ' ';

in tidb, it returns error, in MySQL it is ok.

ref #27954

@zhaoxugang PTAL

IMO the main question is that when convert err is present we should return err or return empty set？if return err maybe effect the user of tidb if return empty will confuse with actual return empty set.I need some advices
@xiongjiwei @yuqi1129 @morgo

zhaoxugang added 12 commits June 20, 2021 00:23

fix test

7df7198

Merge remote-tracking branch 'origin/master'

1dc4efb

Merge branch 'master-upstream'

363f5a6

Merge branch 'master-upstream'

f40ef3a

Merge branch 'master-upstream'

8144f54

Merge branch 'master-upstream'

b33137f

Merge branch 'master-upstream'

0e96845

Merge branch 'master-upstream'

a1e0173

Merge branch 'master-upstream'

865d1ed

Merge branch 'master-upstream'

27cb438

Merge remote-tracking branch 'origin/master'

d5da87c

Fix confusing behaviors of FROM_BASE64 function in WHERE clause

f7850a0

ti-chi-bot added release-note size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 21, 2021

ti-chi-bot requested review from morgo and xhebox September 23, 2021 16:40

zhaoxugang added 2 commits September 24, 2021 01:01

fix test

88e18e8

fix test

fbdb2c0

ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 23, 2021

fix test

85a9953

morgo approved these changes Sep 27, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 27, 2021

xhebox approved these changes Sep 27, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 27, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 27, 2021

ti-chi-bot added 2 commits September 27, 2021 09:08

Merge branch 'master' into issue_28154

4cfbcd4

Merge branch 'master' into issue_28154

4960c13

ti-chi-bot merged commit 582a9f2 into pingcap:master Sep 27, 2021

xiongjiwei mentioned this pull request Sep 28, 2021

planner: Fix Empty string has different meanings in SELECT and UPDATE #27954

Open

12 tasks

xiongjiwei reviewed Sep 28, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: Fix confusing behaviors of FROM_BASE64 function in WHERE clause #28229

core: Fix confusing behaviors of FROM_BASE64 function in WHERE clause #28229

zhaoxugang commented Sep 21, 2021 •

edited by morgo

Loading

ti-chi-bot commented Sep 21, 2021 •

edited

Loading

morgo commented Sep 21, 2021

zhaoxugang commented Sep 22, 2021

zhaoxugang commented Sep 23, 2021

morgo left a comment

xhebox left a comment

xhebox commented Sep 27, 2021

ti-chi-bot commented Sep 27, 2021

xiongjiwei Sep 28, 2021 •

edited

Loading

xiongjiwei Sep 28, 2021

yuqi1129 Sep 28, 2021

zhaoxugang Sep 28, 2021

core: Fix confusing behaviors of FROM_BASE64 function in WHERE clause #28229

core: Fix confusing behaviors of FROM_BASE64 function in WHERE clause #28229

Conversation

zhaoxugang commented Sep 21, 2021 • edited by morgo Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Sep 21, 2021 • edited Loading

morgo commented Sep 21, 2021

zhaoxugang commented Sep 22, 2021

zhaoxugang commented Sep 23, 2021

morgo left a comment

Choose a reason for hiding this comment

xhebox left a comment

Choose a reason for hiding this comment

xhebox commented Sep 27, 2021

ti-chi-bot commented Sep 27, 2021

xiongjiwei Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

xiongjiwei Sep 28, 2021

Choose a reason for hiding this comment

yuqi1129 Sep 28, 2021

Choose a reason for hiding this comment

zhaoxugang Sep 28, 2021

Choose a reason for hiding this comment

zhaoxugang commented Sep 21, 2021 •

edited by morgo

Loading

ti-chi-bot commented Sep 21, 2021 •

edited

Loading

xiongjiwei Sep 28, 2021 •

edited

Loading