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

backend/tidb: temporarily disable the strict-mode value check in tidb backend #551

Merged
merged 2 commits into from
Jan 18, 2021

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Jan 15, 2021

What problem does this PR solve?

#378 added string value check in tidb backend if the SQL mode is strict. But this causes #550 because tidb backend doesn't fetch the full table meta which causes CastValue panic.

What is changed and how it works?

Disable this feature before we come up with a correct solution.

Check List

Tests

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

Side effects

Related changes

@glorv
Copy link
Contributor Author

glorv commented Jan 15, 2021

/run-all-tests

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 Show resolved Hide resolved
@kennytm
Copy link
Collaborator

kennytm commented Jan 15, 2021

LGTM

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM (LGTM1) label Jan 15, 2021
@lance6716
Copy link
Contributor

/lgtm

@ti-srebot ti-srebot added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/LGT1 One reviewer already commented LGTM (LGTM1) labels Jan 18, 2021
@kennytm kennytm merged commit deb5243 into master Jan 18, 2021
@kennytm kennytm deleted the disable-tidb-strict branch January 18, 2021 06:33
glorv added a commit that referenced this pull request Jan 29, 2021
… backend (#551)

* temporarily disable the strict-mode value check in tidb backend since it's buggy

* add link to related issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants