Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

encoder: check string value for tidb encoder #378

Merged
merged 5 commits into from
Aug 31, 2020
Merged

encoder: check string value for tidb encoder #378

merged 5 commits into from
Aug 31, 2020

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Aug 25, 2020 •
edited
Loading

What problem does this PR solve?

close #205

What is changed and how it works?

if set sql-mode to strict mode, will check each string value by table.CastValue

Check List

Tests

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

Side effects

Related changes

@glorv glorv requested a review from kennytm August 25, 2020 11:34
@kennytm
Copy link
Collaborator

kennytm commented Aug 25, 2020

test failed.

@kennytm
Copy link
Collaborator

kennytm commented Aug 26, 2020 •
edited
Loading

i think we should better check using the type's charset (see TiDB's func CastValue). in strict mode, if we write some "Ũñĩĉõđẽ" into a varchar(100) charset ascii it should still fail.

kennytm
kennytm reviewed Aug 26, 2020
View reviewed changes
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

rest lgtm

lightning/backend/tidb.go Outdated Show resolved Hide resolved
lightning/backend/tidb_test.go Outdated Show resolved Hide resolved
@kennytm kennytm added the status/LGT1 One reviewer already commented LGTM (LGTM1) label Aug 26, 2020
@YuJuncen
Copy link
Contributor

LGTM

@kennytm kennytm added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/LGT1 One reviewer already commented LGTM (LGTM1) labels Aug 31, 2020
@kennytm kennytm merged commit 63c72b3 into master Aug 31, 2020
@kennytm kennytm deleted the valid-utf8 branch August 31, 2020 07:04
@glorv glorv mentioned this pull request Jan 15, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kennytm kennytm kennytm approved these changes

Assignees
No one assigned
Labels
status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inserting non-UTF-8 data into a VARCHAR column should produce an error in strict SQL_MODE
3 participants
@glorv @kennytm @YuJuncen