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: handle max_allowed_packet warnings for space function. #7210

Merged
merged 3 commits into from
Aug 2, 2018
Merged

expression: handle max_allowed_packet warnings for space function. #7210

merged 3 commits into from
Aug 2, 2018

Conversation

hhu-cc
Copy link
Contributor

@hhu-cc hhu-cc commented Jul 31, 2018

What have you changed? (mandatory)

Return NULL and a warning when the result of space exceeds max_allowed_packetbs.

Before this PR:

tidb> select space(18446744073709551617);
+-----------------------------+
| space(18446744073709551617) |
+-----------------------------+
| NULL                        |
+-----------------------------+
1 row in set, 1 warning (0.00 sec)

tidb> show warnings;
+---------+------+-----------------------------------------------------------+
| Level   | Code | Message                                                   |
+---------+------+-----------------------------------------------------------+
| Warning | 1292 | Truncated incorrect DECIMAL value: '18446744073709551617' |
+---------+------+-----------------------------------------------------------+
1 row in set (0.00 sec)

After this PR:

tidb> select space(18446744073709551617);
+-----------------------------+
| space(18446744073709551617) |
+-----------------------------+
| NULL                        |
+-----------------------------+
1 row in set, 2 warnings (0.00 sec)

tidb> show warnings;
+---------+------+-----------------------------------------------------------------------------+
| Level   | Code | Message                                                                     |
+---------+------+-----------------------------------------------------------------------------+
| Warning | 1292 | Truncated incorrect DECIMAL value: '18446744073709551617'                   |
| Warning | 1301 | Result of space() was larger than max_allowed_packet (67108864) - truncated |
+---------+------+-----------------------------------------------------------------------------+
2 rows in set (0.00 sec)

What is the type of the changes? (mandatory)

Bug fix (non-breaking change which fixes an issue)

How has this PR been tested? (mandatory)

  • unit test
  • explain test

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

no

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

no

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

Yes

release note:

Return `NULL` when the result of function `SPACE` is larger than `max_allowed_packet`

Refer to a related PR or issue link (optional)

to #7153

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

@sre-bot
Copy link
Contributor

sre-bot commented Jul 31, 2018

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.

@hhu-cc
Copy link
Contributor Author

hhu-cc commented Jul 31, 2018

/run-all-tests

Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

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

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. status/all tests passed component/expression type/compatibility contribution This PR is from a community contributor. labels Aug 2, 2018
@zz-jason
Copy link
Member

zz-jason commented Aug 2, 2018

@XuHuaiyu Please give this PR an approve.

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/compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants