VReplication: Remove auto_increment clauses for MoveTables to a shard…

…ed keyspace (#15679)

Signed-off-by: Matt Lord <[email protected]>

changelog/20.0/20.0.0/summary.md

@@ -11,6 +11,7 @@
     - [New `unmanaged` Flag and `disable_active_reparents` deprecation](#unmanaged-flag)
     - [`recovery-period-block-duration` Flag deprecation](#recovery-block-deprecation)
     - [`mysqlctld` `onterm-timeout` Default Change](#mysqlctld-onterm-timeout)
+    - [`MoveTables` now removes `auto_increment` clauses by default when moving tables from an unsharded keyspace to a sharded one](#move-tables-auto-increment)
     - [`Durabler` interface method renaming](#durabler-interface-method-renaming)
   - **[Query Compatibility](#query-compatibility)**
     - [Vindex Hints](#vindex-hints)
@@ -106,6 +107,10 @@ The `--onterm_timeout` flag default value has changed for `mysqlctld`. It now is
 
 This is necessary since otherwise MySQL would never shut down cleanly with the old defaults, since `mysqlctld` would shut down already after 10 seconds by default.
 
+#### <a id="move-tables-auto-increment"/>`MoveTables` now removes `auto_increment` clauses by default when moving tables from an unsharded keyspace to a sharded one
+
+A new `--remove-sharded-auto-increment` flag has been added to the [`MoveTables` create sub-command](https://vitess.io/docs/20.0/reference/programs/vtctldclient/vtctldclient_movetables/vtctldclient_movetables_create/) and it is set to `true` by default. This flag controls whether any [MySQL `auto_increment`](https://dev.mysql.com/doc/refman/en/example-auto-increment.html) clauses should be removed from the table definitions when moving tables from an unsharded keyspace to a sharded one. This is now done by default as `auto_increment` clauses should not typically be used with sharded tables and you should instead rely on externally generated values such as a form of universally/globally unique identifiers or use [Vitess sequences](https://vitess.io/docs/reference/features/vitess-sequences/) in order to ensure that each row has a unique identifier (Primary Key value) across all shards. If for some reason you want to retain them you can set this new flag to `false` when creating the workflow.
+
 #### <a id="durabler-interface-method-renaming"/>`Durabler` interface method renaming
 
 The methods of [the `Durabler` interface](https://github.com/vitessio/vitess/blob/main/go/vt/vtctl/reparentutil/durability.go#L70-L79) in `go/vt/vtctl/reparentutil` were renamed to be public _(capitalized)_ methods to make it easier to integrate custom Durability Policies from external packages. See [RFC for details](https://github.com/vitessio/vitess/issues/15544).

go/cmd/vtctldclient/command/vreplication/movetables/movetables.go

@@ -49,6 +49,7 @@ func registerCommands(root *cobra.Command) {
 	create.Flags().BoolVar(&createOptions.AtomicCopy, "atomic-copy", false, "(EXPERIMENTAL) A single copy phase is run for all tables from the source. Use this, for example, if your source keyspace has tables which use foreign key constraints.")
 	create.Flags().StringVar(&createOptions.WorkflowOptions.TenantId, "tenant-id", "", "(EXPERIMENTAL) The tenant ID to use for the MoveTables workflow into a multi-tenant keyspace.")
 	create.Flags().StringVar(&createOptions.WorkflowOptions.SourceKeyspaceAlias, "source-keyspace-alias", "", "(EXPERIMENTAL) Used currently only for multi-tenant migrations. This value will be used instead of the source keyspace name in the keyspace routing rules.")
+	create.Flags().BoolVar(&createOptions.WorkflowOptions.StripShardedAutoIncrement, "remove-sharded-auto-increment", true, "If moving the table(s) to a sharded keyspace, remove any auto_increment clauses when copying the schema to the target as sharded keyspaces should rely on either user/application generated values or Vitess sequences to ensure uniqueness.")
 	base.AddCommand(create)
 
 	opts := &common.SubCommandsOpts{

go/test/endtoend/vreplication/config_test.go

@@ -20,9 +20,11 @@ package vreplication
 // We violate the NO_ZERO_DATES and NO_ZERO_IN_DATE sql_modes that are enabled by default in
 // MySQL 5.7+ and MariaDB 10.2+ to ensure that vreplication still works everywhere and the
 // permissive sql_mode now used in vreplication causes no unwanted side effects.
-// The customer table also tests two important things:
+// The customer table also tests several important things:
 //  1. Composite or multi-column primary keys
 //  2. PKs that contain an ENUM column
+//  3. That we properly handle tables with auto_increment columns (which are stripped by default when
+//     moving the table to a sharded keyspace with vtctldclient and left in place when using vtctlclient)
 //
 // The Lead and Lead-1 tables also allows us to test several things:
 //  1. Mixed case identifiers
@@ -40,7 +42,7 @@ var (
 	// All standard user tables should have a primary key and at least one secondary key.
 	initialProductSchema = `
 create table product(pid int, description varbinary(128), date1 datetime not null default '0000-00-00 00:00:00', date2 datetime not null default '2021-00-01 00:00:00', primary key(pid), key(date1,date2)) CHARSET=utf8mb4;
-create table customer(cid int, name varchar(128) collate utf8mb4_bin, meta json default null, typ enum('individual','soho','enterprise'), sport set('football','cricket','baseball'),
+create table customer(cid int auto_increment, name varchar(128) collate utf8mb4_bin, meta json default null, typ enum('individual','soho','enterprise'), sport set('football','cricket','baseball'),
 	ts timestamp not null default current_timestamp, bits bit(2) default b'11', date1 datetime not null default '0000-00-00 00:00:00', 
 	date2 datetime not null default '2021-00-01 00:00:00', dec80 decimal(8,0), blb blob, primary key(cid,typ), key(name)) CHARSET=utf8mb4;
 create table customer_seq(id int, next_id bigint, cache bigint, primary key(id)) comment 'vitess_sequence';

go/test/endtoend/vreplication/resharding_workflows_v2_test.go

@@ -605,6 +605,14 @@ func testMoveTablesV2Workflow(t *testing.T) {
 
 	testVSchemaForSequenceAfterMoveTables(t)
 
+	// Confirm that the auto_increment clause on customer.cid was removed.
+	cs, err := vtgateConn.ExecuteFetch("show create table customer", 1, false)
+	require.NoError(t, err)
+	require.Len(t, cs.Rows, 1)
+	require.Len(t, cs.Rows[0], 2) // Table and "Create Table"
+	csddl := strings.ToLower(cs.Rows[0][1].ToString())
+	require.NotContains(t, csddl, "auto_increment", "customer table still has auto_increment clause: %s", csddl)
+
 	createMoveTablesWorkflow(t, "Lead,Lead-1")
 	output, err = vc.VtctldClient.ExecuteCommandWithOutput(listAllArgs...)
 	require.NoError(t, err)