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

executor: fix the issue that SELECT ... INTO OUTFILE returns runtime error #19438

Merged
merged 6 commits into from
Sep 1, 2020

Conversation

qw4990
Copy link
Contributor

@qw4990 qw4990 commented Aug 25, 2020

What problem does this PR solve?

Issue Number: close #19314

Problem Summary: SELECT ... INTO OUTFILE returns runtime error

What is changed and how it works?

Handle bit type correctly when dumping rows into files.

Check List

Tests

  • Unit test

Release note

  • executor: fix the issue that SELECT ... INTO OUTFILE returns runtim…

@qw4990 qw4990 added type/bugfix This PR fixes a bug. sig/execution SIG execution needs-cherry-pick-4.0 labels Aug 25, 2020
@qw4990 qw4990 requested a review from a team as a code owner August 25, 2020 07:51
@qw4990 qw4990 requested review from XuHuaiyu and removed request for a team August 25, 2020 07:51
@qw4990 qw4990 changed the title executor: fix the issue that SELECT ... INTO OUTFILE returns runtim… executor: fix the issue that SELECT ... INTO OUTFILE returns runtime error Aug 25, 2020
@qw4990 qw4990 requested a review from SunRunAway August 25, 2020 08:21
switch col.GetType().EvalType() {
case types.ETInt:
switch col.GetType().Tp {
case mysql.TypeTiny, mysql.TypeShort, mysql.TypeInt24, mysql.TypeLong:
Copy link
Member

Choose a reason for hiding this comment

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

Why this branch doesn't need to consider signed or unsigned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For these types, we ignore their signs and store them simply as int64 in TiDB since there is no risk to overflow.
I add one more test for this.
PTAL @winoros

Copy link
Contributor

@XuHuaiyu XuHuaiyu Sep 1, 2020

Choose a reason for hiding this comment

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

Can we load this file into mysql successfully if we ignore the signs?
e.g. tinyint unsigned column, the value is 127

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I test it successfully in local:

mysql> create table t (v tinyint unsigned);
Query OK, 0 rows affected (0.01 sec)

mysql> load data local infile '/tmp/rrrr' into table t;
Query OK, 1 row affected (0.00 sec)
Records: 1  Deleted: 0  Skipped: 0  Warnings: 0

mysql> select * from t;
+------+
| v    |
+------+
|  127 |
+------+
1 row in set (0.00 sec)

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

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 1, 2020
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 1, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 1, 2020
@qw4990
Copy link
Contributor Author

qw4990 commented Sep 1, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 1, 2020
@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 19658
  • 19658

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit a5839ab into pingcap:master Sep 1, 2020
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 1, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #19672

winoros pushed a commit that referenced this pull request Sep 3, 2020
…e error (#19438) (#19672)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: Yuanjia Zhang <zhangyuanjia@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. 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.

SELECT ... INTO OUTFILE returns runtime error
4 participants