Skip to content

sessionctx: Set foreign_key_checks = OFF#8358

Merged
zz-jason merged 12 commits into
pingcap:masterfrom
morgo:foreign-keys
Nov 26, 2018
Merged

sessionctx: Set foreign_key_checks = OFF#8358
zz-jason merged 12 commits into
pingcap:masterfrom
morgo:foreign-keys

Conversation

@morgo
Copy link
Copy Markdown
Contributor

@morgo morgo commented Nov 19, 2018

What problem does this PR solve?

Partially address #8349 by making metadata clearer that foreign keys are not supported.

What is changed and how it works?

This does two things:

  1. Changes the default for foreign_key_checks to OFF (differs from MySQL default)
  2. Shows a warning (and refuses to change) foreign_key_checks to ON. Since this is not supported by TiDB.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

I've manually verified it works. It does not actively change code, since it is currently an unused option - but I can add unit tests if desired.

Code changes

  • Minimal

Side effects

  • Breaking backward compatibility (for example, some exotic loaders may check for a warning on reload.)

Related changes

  • Need to update the documentation (default differences from mysql)
  • Need to update the tidb-ansible repository (I'm not sure - I don't think it is required).

This change is Reviewable

Add variable validation to show warning when attempting to enable.
Makes it clearer to users that TiDB does not yet support FKs
@gregwebs
Copy link
Copy Markdown
Contributor

LGTM

@ngaut
Copy link
Copy Markdown
Member

ngaut commented Nov 20, 2018

LGTM, but defer to @shenli @winkyao

@shenli
Copy link
Copy Markdown
Member

shenli commented Nov 20, 2018

@morgo Please add a unit test case.

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. component/server labels Nov 20, 2018
@ngaut
Copy link
Copy Markdown
Member

ngaut commented Nov 22, 2018

/run-all-tests

@morgo
Copy link
Copy Markdown
Contributor Author

morgo commented Nov 22, 2018

/run-all-tests

Copy link
Copy Markdown
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 merged commit cd7e27d into pingcap:master Nov 26, 2018
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
@morgo morgo deleted the foreign-keys branch April 8, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/server contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants