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

Make TiDB output of show create table simpler and allow to break MySQL compatibility #36137

Open
mjonss opened this issue Jul 12, 2022 · 3 comments · May be fixed by #36141
Open

Make TiDB output of show create table simpler and allow to break MySQL compatibility #36137

mjonss opened this issue Jul 12, 2022 · 3 comments · May be fixed by #36141
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@mjonss
Copy link
Contributor

mjonss commented Jul 12, 2022

Enhancement

Working on #35683 the need of allowing non-compatible MySQL syntax is clear. But it would also remove non-needed MySQL compatibility syntax that is not used in TiDB, like ENGINE = InnoDB

This Feature request is to create tidb_extension_non_mysql_compatible system variable to allow TiDB extensions that are not compatible with MySQL, and remove MySQL specific syntax that is not used by TiDB.

@mjonss mjonss added the type/enhancement The issue or PR belongs to an enhancement. label Jul 12, 2022
@dveeden
Copy link
Contributor

dveeden commented Jul 12, 2022

I think using TiDB feature checks like /*T![clustered_index] CLUSTERED */ allow MySQL compatibility and backwards compatibility with TiDB. Any reason not to use that instead?

Could this cause issues with TiDB to MySQL replication via TiCDC?

@mjonss
Copy link
Contributor Author

mjonss commented Jul 12, 2022

I think using TiDB feature checks like /*T![clustered_index] CLUSTERED */ allow MySQL compatibility and backwards compatibility with TiDB. Any reason not to use that instead?

For #35683 it needs to replace the old partitioning syntax with the new more compact INTERVAL syntax, so it is not only to allow the new INTERVAL syntax, but also allow the old verbose partitioning syntax to be removed.

The new system variable tidb_extension_non_mysql_compatible should never be used for exporting table structures etc. between systems (especially not between TiDB and MySQL). But will also break backwards compatibility with older TiDB (that does not have support for a specific TiDB feature like [clustered_index]).

The use case here are:

  • Allow showing RANGE INTERVAL partitioning instead of all partitions (prepare for Support RANGE INTERVAL partitioning #35683)
  • Allow more human friendly SHOW CREATE TABLE output (remove non relevant comments and options that does not have any effect on the current session or same version of TiDB).

Could this cause issues with TiDB to MySQL replication via TiCDC?

It should not since it should be off by default. Also I intend to support identifying valid INTERVAL partitioning for existing RANGE [COLUMNS] partitioning, so for machine communication it should still be possible to only use legacy partitioning schemes. I will probably add the INTERVAL syntax within feature comments, like /*T![interval_partition] ... */, for simplify validation.

@dveeden
Copy link
Contributor

dveeden commented Jul 14, 2022

Could this cause issues with TiDB to MySQL replication via TiCDC?

It should not since it should be off by default. Also I intend to support identifying valid INTERVAL partitioning for existing RANGE [COLUMNS] partitioning, so for machine communication it should still be possible to only use legacy partitioning schemes. I will probably add the INTERVAL syntax within feature comments, like /*T![interval_partition] ... */, for simplify validation.

If this is a global variable then we should make TiCDC set this to the right value when connecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants