Skip to content
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

expression: fix behavior for builtin 'LTrim', 'RTrim' and 'Trim' #7291

Merged
merged 2 commits into from
Aug 7, 2018

Conversation

spongedu
Copy link
Contributor

@spongedu spongedu commented Aug 7, 2018

What have you changed? (mandatory)

In TiDB, Builtin LTrim, RTrim and Trim act differently with MySQL. In MySQL, \r\n\t are not trimed but TiDB do:
In MySQL:

mysql> select character_length(rtrim("bar\n     "));
+---------------------------------------+
| character_length(rtrim("bar\n     ")) |
+---------------------------------------+
|                                     4 |
+---------------------------------------+
1 row in set (0.00 sec)

mysql> select character_length(ltrim("\r  bar"));
+------------------------------------+
| character_length(ltrim("\r  bar")) |
+------------------------------------+
|                                  6 |
+------------------------------------+
1 row in set (0.00 sec)

mysql> select character_length(trim("\r  bar\n"));
+-------------------------------------+
| character_length(trim("\r  bar\n")) |
+-------------------------------------+
|                                   7 |
+-------------------------------------+
1 row in set (0.00 sec)

In TiDB:

tidb> select character_length(rtrim("bar\n     "));
+---------------------------------------+
| character_length(rtrim("bar\n     ")) |
+---------------------------------------+
|                                     3 |
+---------------------------------------+
1 row in set (0.01 sec)

tidb> select character_length(ltrim("\r  bar"));
+------------------------------------+
| character_length(ltrim("\r  bar")) |
+------------------------------------+
|                                  3 |
+------------------------------------+
1 row in set (0.00 sec)

tidb> select character_length(trim("\r  bar\n"));
+-------------------------------------+
| character_length(trim("\r  bar\n")) |
+-------------------------------------+
|                                   3 |
+-------------------------------------+
1 row in set (0.00 sec)

What is the type of the changes? (mandatory)

Compatibility

How has this PR been tested? (mandatory)

Add some unittests and integration tests

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Does this PR affect tidb-ansible update? (mandatory)

Does this PR need to be added to the release notes? (mandatory)

Refer to a related PR or issue link (optional)

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewable status: 0 of 3 files reviewed, all discussions resolved

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: 0 of 3 files reviewed, all discussions resolved

@zz-jason zz-jason added type/bugfix This PR fixes a bug. component/expression status/LGT1 Indicates that a PR has LGTM 1. labels Aug 7, 2018
@zz-jason
Copy link
Member

zz-jason commented Aug 7, 2018

/run-all-tests

Copy link
Member

@coocood coocood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@coocood coocood added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 7, 2018
@tiancaiamao
Copy link
Contributor

/run-all-tests

@coocood coocood merged commit 6e33d2e into pingcap:master Aug 7, 2018
@spongedu spongedu deleted the dc_0807 branch August 11, 2018 01:57
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants