Add CLI flag for specifying default SKIP RANGE and START COUNTER WITH values for IDENTITY columns (#1236)

* Add default identity options to Conv struct for use when converting to IDENTITY auto gen columns

The actual setting of the default identity options from CLI will come in a later commit.

* Add target profile flags for setting the default skip range and start counter with values for IDENTITY columns

* Populate DefaultIdentityOptions in conv from values read in via target profile

* Update docs

* Ensure default identity options are used for postgres IDENTITY columns as well

* Update docs

* Add integration test for default identity options and fix issue with DDL generation

* Add comment describing expected format

Author: nmemond

conversion/conversion.go

@@ -84,7 +84,7 @@ func (ci *ConvImpl) SchemaConv(migrationProjectId string, sourceProfile profiles
 		conv, err = schemaFromSource.schemaFromDatabase(migrationProjectId, sourceProfile, targetProfile, &GetInfoImpl{}, &common.ProcessSchemaImpl{})
 	case constants.PGDUMP, constants.MYSQLDUMP:
 		expressionVerificationAccessor, _ := expressions_api.NewExpressionVerificationAccessorImpl(context.Background(), targetProfile.Conn.Sp.Project, targetProfile.Conn.Sp.Instance)
-		conv, err = schemaFromSource.SchemaFromDump(targetProfile.Conn.Sp.Project, targetProfile.Conn.Sp.Instance, sourceProfile.Driver, targetProfile.Conn.Sp.Dialect, ioHelper, &ProcessDumpByDialectImpl{ExpressionVerificationAccessor: expressionVerificationAccessor})
+		conv, err = schemaFromSource.SchemaFromDump(targetProfile.Conn.Sp.Project, targetProfile.Conn.Sp.Instance, sourceProfile.Driver, targetProfile.Conn.Sp.Dialect, ioHelper, &ProcessDumpByDialectImpl{ExpressionVerificationAccessor: expressionVerificationAccessor}, targetProfile.DefaultIdentityOptions)
 	default:
 		return nil, fmt.Errorf("schema conversion for driver %s not supported", sourceProfile.Driver)
 	}

conversion/conversion_from_source.go

@@ -32,14 +32,15 @@ import (
 	"github.com/GoogleCloudPlatform/spanner-migration-tool/profiles"
 	"github.com/GoogleCloudPlatform/spanner-migration-tool/sources/common"
 	"github.com/GoogleCloudPlatform/spanner-migration-tool/sources/csv"
+	"github.com/GoogleCloudPlatform/spanner-migration-tool/spanner/ddl"
 	"github.com/GoogleCloudPlatform/spanner-migration-tool/spanner/writer"
 	"github.com/GoogleCloudPlatform/spanner-migration-tool/streaming"
 	"go.uber.org/zap"
 )
 
 type SchemaFromSourceInterface interface {
 	schemaFromDatabase(migrationProjectId string, sourceProfile profiles.SourceProfile, targetProfile profiles.TargetProfile, getInfo GetInfoInterface, processSchema common.ProcessSchemaInterface) (*internal.Conv, error)
-	SchemaFromDump(SpProjectId string, SpInstanceId string, driver string, spDialect string, ioHelper *utils.IOStreams, processDump ProcessDumpByDialectInterface) (*internal.Conv, error)
+	SchemaFromDump(SpProjectId string, SpInstanceId string, driver string, spDialect string, ioHelper *utils.IOStreams, processDump ProcessDumpByDialectInterface, defaultIdentityOptions profiles.DefaultIdentityOptions) (*internal.Conv, error)
 }
 
 type SchemaFromSourceImpl struct {
@@ -60,6 +61,11 @@ func (sads *SchemaFromSourceImpl) schemaFromDatabase(migrationProjectId string,
 	conv.SpProjectId = targetProfile.Conn.Sp.Project
 	conv.SpInstanceId = targetProfile.Conn.Sp.Instance
 	conv.Source = sourceProfile.Driver
+	conv.DefaultIdentityOptions = ddl.IdentityOptions{
+		SkipRangeMin: targetProfile.DefaultIdentityOptions.SkipRangeMin,
+		SkipRangeMax: targetProfile.DefaultIdentityOptions.SkipRangeMax,
+		StartCounterWith: targetProfile.DefaultIdentityOptions.StartCounterWith,
+	}
 	//handle fetching schema differently for sharded migrations, we only connect to the primary shard to
 	//fetch the schema. We reuse the SourceProfileConnection object for this purpose.
 	var infoSchema common.InfoSchema
@@ -112,7 +118,7 @@ func (sads *SchemaFromSourceImpl) schemaFromDatabase(migrationProjectId string,
 	return conv, processSchema.ProcessSchema(conv, infoSchema, common.DefaultWorkers, additionalSchemaAttributes, &schemaToSpanner, &common.UtilsOrderImpl{}, &common.InfoSchemaImpl{})
 }
 
-func (sads *SchemaFromSourceImpl) SchemaFromDump(SpProjectId string, SpInstanceId string, driver string, spDialect string, ioHelper *utils.IOStreams, processDump ProcessDumpByDialectInterface) (*internal.Conv, error) {
+func (sads *SchemaFromSourceImpl) SchemaFromDump(SpProjectId string, SpInstanceId string, driver string, spDialect string, ioHelper *utils.IOStreams, processDump ProcessDumpByDialectInterface, defaultIdentityOptions profiles.DefaultIdentityOptions) (*internal.Conv, error) {
 	f, n, err := getSeekable(ioHelper.In)
 	if err != nil {
 		utils.PrintSeekError(driver, err, ioHelper.Out)
@@ -125,6 +131,11 @@ func (sads *SchemaFromSourceImpl) SchemaFromDump(SpProjectId string, SpInstanceI
 	conv.Source = driver
 	conv.SpProjectId = SpProjectId
 	conv.SpInstanceId = SpInstanceId
+	conv.DefaultIdentityOptions = ddl.IdentityOptions{
+		SkipRangeMin: defaultIdentityOptions.SkipRangeMin,
+		SkipRangeMax: defaultIdentityOptions.SkipRangeMax,
+		StartCounterWith: defaultIdentityOptions.StartCounterWith,
+	}
 	p := internal.NewProgress(n, "Generating schema", internal.Verbose(), false, int(internal.SchemaCreationInProgress))
 	r := internal.NewReader(bufio.NewReader(f), p)
 	conv.SetSchemaMode() // Build schema and ignore data in dump.

conversion/mocks.go

@@ -65,7 +65,7 @@ func (msads *MockSchemaFromSource) schemaFromDatabase(migrationProjectId string,
 	args := msads.Called(migrationProjectId, sourceProfile, targetProfile, getInfo, processSchema)
 	return args.Get(0).(*internal.Conv), args.Error(1)
 }
-func (msads *MockSchemaFromSource) SchemaFromDump(SpProjectId string, SpInstanceId string, driver string, spDialect string, ioHelper *utils.IOStreams, processDump ProcessDumpByDialectInterface) (*internal.Conv, error) {
+func (msads *MockSchemaFromSource) SchemaFromDump(SpProjectId string, SpInstanceId string, driver string, spDialect string, ioHelper *utils.IOStreams, processDump ProcessDumpByDialectInterface, defaultIdentityOptions profiles.DefaultIdentityOptions) (*internal.Conv, error) {
 	args := msads.Called(driver, spDialect, ioHelper, processDump)
 	return args.Get(0).(*internal.Conv), args.Error(1)
 }

docs/cli/flags.md

@@ -78,3 +78,11 @@ Time Zone Database](https://www.iana.org/time-zones). If not specified, the defa
 the Spanner database, and the database will therefore default to `America/Los_Angeles` (the default timezone for
 Spanner databases). Note, the default timezone can only be set on an empty Spanner database without any tables; a
 warning will be logged and this setting will be ignored if the database already includes tables.
+
+* **`defaultIdentitySkipRange`**: Optional flag. Specifies the default SKIP RANGE values to use for IDENTITY columns. Specified as `<min>-<max>`, where both `<min>` and `<max>` are positive integers and `<min>` must be less than `<max>`. For example, `defaultIdentitySkipRange=10-50`. For
+  instructions on setting SKIP RANGE values for individual columns, see
+  [here](../data-types/mysql.md#auto-increment-columns).
+
+* **`defaultIdentityStartCounterWith`**: Optional flag. Specifies the default START COUNTER WITH value to use for IDENTITY columns. This should be a positive integer. For example, `defaultIdentityStartCounterWith=1000`. For
+  instructions on setting the START COUNTER WITH value for individual columns, see
+  [here](../data-types/mysql.md#auto-increment-columns).

docs/data-types/mysql.md

@@ -176,37 +176,46 @@ The tool maps auto-increment columns to [Spanner IDENTITY
 columns](https://cloud.google.com/spanner/docs/primary-key-default-value#identity-columns).
 Users need to set the SKIP RANGE and/or START COUNTER WITH values to avoid duplicate key errors.
 
-The SKIP RANGE and START COUNTER WITH values can be set most via both the web UI (recommended) and the CLI.
+The SKIP RANGE and START COUNTER WITH values can be set via both the web UI (recommended) and the CLI.
 
 The Column tab of the web UI exposes fields to set the SKIP RANGE and START COUNTER WITH values. For more details, see [here](../ui/schema-conv/spanner-draft.md).
 
-To set the SKIP RANGE and/or START COUNTER WITH values via the CLI, the recommended steps are as follows:
+To set the SKIP RANGE and/or START COUNTER WITH values via the CLI, there are two options: either specify default
+values to be used by all IDENTITY columns, or specify values on a per-column basis. Both options can be used in
+conjuction with one another.
+
+To specify default SKIP RANGE and/or START COUNTER WITH values to be used by all columns, include the following flags
+in the `targetProfile` parameter: `defaultIdentitySkipRange` and `defaultIdentityStartCounterWith`, for example:
+```sh
+--targetProfile="instance=my-instance,defaultIdentitySkipRange=1000-5000,defaultIdentityStartCounterWith=100"
+```
+For more details on both flags, see [here](../cli/flags.md#target-profile).
+
+To specify SKIP RANGE and/or START COUNTER WITH values on per-column basis via the CLI, do the following:
 - Do a dry-run schema-only migration to generate a session JSON file:
 ```sh
-    spanner-migration-tool schema -dry-run ...
+spanner-migration-tool schema -dry-run ...
 ```
 - Open the resulting session file and find the relevant column definition(s) in the `ColDefs` collection of the table
   it belongs to
 - Set the appropriate fields in that column's `AutoGen.AutoIncrementOptions` node. All three values are expected to be
   strings containing a numeric value. For example:
 ```json
-    {
-        "SpSchema": {
-            "table1": {
-                "Name": "SomeTable",
-                "ColDefs": {
-                    "column1": {
-                        "Name": "some_column",
-                        "AutoGen": {
-                            "Name": "Auto Increment",
-                            "GenerationType": "Auto Increment",
-                            "AutoIncrementOptions": {
-                                "SkipRangeMin": "1000",
-                                "SkipRangeMax": "10000",
-                                "StartCounterWith": "500"
-                            }
-                        },
-                        ...
+{
+    "SpSchema": {
+        "table1": {
+            "Name": "SomeTable",
+            "ColDefs": {
+                "column1": {
+                    "Name": "some_column",
+                    "AutoGen": {
+                        "Name": "Auto Increment",
+                        "GenerationType": "Auto Increment",
+                        "AutoIncrementOptions": {
+                            "SkipRangeMin": "1000",
+                            "SkipRangeMax": "10000",
+                            "StartCounterWith": "500"
+                        }
                     },
                     ...
                 },
@@ -215,11 +224,13 @@ To set the SKIP RANGE and/or START COUNTER WITH values via the CLI, the recommen
             ...
         },
         ...
-    }
+    },
+    ...
+}
 ```
 - Save the session file and run your desired migration using the updated session file:
 ```sh
-    spanner-migration-tool schema -session=<path to session file> ...
+spanner-migration-tool schema -session=<path to session file> ...
 ```
 
 ## Other MySQL features

docs/data-types/postgres.md

@@ -66,37 +66,46 @@ with type `INT64`.
 
 Users need to set the SKIP RANGE and/or START COUNTER WITH values to avoid duplicate key errors.
 
-The SKIP RANGE and START COUNTER WITH values can be set most via both the web UI (recommended) and the CLI.
+The SKIP RANGE and START COUNTER WITH values can be set via both the web UI (recommended) and the CLI.
 
 The Column tab of the web UI exposes fields to set the SKIP RANGE and START COUNTER WITH values. For more details, see [here](../ui/schema-conv/spanner-draft.md).
 
-To set the SKIP RANGE and/or START COUNTER WITH values via the CLI, the recommended steps are as follows:
+To set the SKIP RANGE and/or START COUNTER WITH values via the CLI, there are two options: either specify default
+values to be used by all IDENTITY columns, or specify values on a per-column basis. Both options can be used in
+conjuction with one another.
+
+To specify default SKIP RANGE and/or START COUNTER WITH values to be used by all columns, include the following flags
+in the `targetProfile` parameter: `defaultIdentitySkipRange` and `defaultIdentityStartCounterWith`, for example:
+```sh
+--targetProfile="instance=my-instance,defaultIdentitySkipRange=1000-5000,defaultIdentityStartCounterWith=100"
+```
+For more details on both flags, see [here](../cli/flags.md#target-profile).
+
+To specify SKIP RANGE and/or START COUNTER WITH values on per-column basis via the CLI, do the following:
 - Do a dry-run schema-only migration to generate a session JSON file:
 ```sh
-    spanner-migration-tool schema -dry-run ...
+spanner-migration-tool schema -dry-run ...
 ```
 - Open the resulting session file and find the relevant column definition(s) in the `ColDefs` collection of the table
   it belongs to
 - Set the appropriate fields in that column's `AutoGen.AutoIncrementOptions` node. All three values are expected to be
   strings containing a numeric value. For example:
 ```json
-    {
-        "SpSchema": {
-            "table1": {
-                "Name": "SomeTable",
-                "ColDefs": {
-                    "column1": {
-                        "Name": "some_column",
-                        "AutoGen": {
-                            "Name": "Auto Increment",
-                            "GenerationType": "Auto Increment",
-                            "AutoIncrementOptions": {
-                                "SkipRangeMin": "1000",
-                                "SkipRangeMax": "10000",
-                                "StartCounterWith": "500"
-                            }
-                        },
-                        ...
+{
+    "SpSchema": {
+        "table1": {
+            "Name": "SomeTable",
+            "ColDefs": {
+                "column1": {
+                    "Name": "some_column",
+                    "AutoGen": {
+                        "Name": "Auto Increment",
+                        "GenerationType": "Auto Increment",
+                        "AutoIncrementOptions": {
+                            "SkipRangeMin": "1000",
+                            "SkipRangeMax": "10000",
+                            "StartCounterWith": "500"
+                        }
                     },
                     ...
                 },
@@ -105,11 +114,13 @@ To set the SKIP RANGE and/or START COUNTER WITH values via the CLI, the recommen
             ...
         },
         ...
-    }
+    },
+    ...
+}
 ```
 - Save the session file and run your desired migration using the updated session file:
 ```sh
-    spanner-migration-tool schema -session=<path to session file> ...
+spanner-migration-tool schema -session=<path to session file> ...
 ```
 
 ## TIMESTAMP

internal/convert.go

@@ -59,6 +59,7 @@ type Conv struct {
 	SpInstanceId       string                  // Spanner Instance Id
 	Source             string                  // Source Database type being migrated
 	DatabaseOptions    ddl.DatabaseOptions
+	DefaultIdentityOptions ddl.IdentityOptions // Default values to use for IDENTITY columns
 }
 
 type InvalidCheckExp struct {

profiles/target_profile.go

@@ -17,6 +17,7 @@ package profiles
 import (
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -57,6 +58,13 @@ type TargetProfileConnection struct {
 type TargetProfile struct {
 	Ty   TargetProfileType
 	Conn TargetProfileConnection
+	DefaultIdentityOptions DefaultIdentityOptions
+}
+
+type DefaultIdentityOptions struct {
+	SkipRangeMin string
+	SkipRangeMax string
+	StartCounterWith string
 }
 
 // This expects that GetResourceIds has already been called once and the project, instance and dbName
@@ -156,6 +164,11 @@ func NewTargetProfile(s string) (TargetProfile, error) {
 		return TargetProfile{}, fmt.Errorf("dialect not supported %v", sp.Dialect)
 	}
 
+	defaultIdentityOptions, err := extractDefaultIdentityOptions(params)
+	if err != nil {
+		return TargetProfile{}, err
+	}
+
 	// if target-profile is not empty, it must contain spanner instance
 	if s != "" && sp.Instance == "" {
 		return TargetProfile{}, fmt.Errorf("found empty string for instance. please specify instance (spanner instance) in the target-profile")
@@ -169,5 +182,51 @@ func NewTargetProfile(s string) (TargetProfile, error) {
 	}
 
 	conn := TargetProfileConnection{Ty: TargetProfileConnectionTypeSpanner, Sp: sp}
-	return TargetProfile{Ty: TargetProfileTypeConnection, Conn: conn}, nil
+	return TargetProfile{Ty: TargetProfileTypeConnection, Conn: conn, DefaultIdentityOptions: defaultIdentityOptions}, nil
+}
+
+func extractDefaultIdentityOptions(params map[string]string) (DefaultIdentityOptions, error) {
+	defaultIdentityOptions := DefaultIdentityOptions{}
+	if defaultSkipRangeStr, ok := params["defaultIdentitySkipRange"]; ok {
+		skipRangeMin, skipRangeMax, err := parseDefaultSkipRange(defaultSkipRangeStr)
+		if err != nil {
+			return DefaultIdentityOptions{}, fmt.Errorf(fmt.Sprintf("Invalid value for defaultIdentitySkipRange: %s, expected <min>-<max> where both <min> and <max> are integers and <min> is less than <max>. %s", defaultSkipRangeStr, err))
+		}
+		defaultIdentityOptions.SkipRangeMin = skipRangeMin
+		defaultIdentityOptions.SkipRangeMax = skipRangeMax
+	}
+	if defaultStartCounterWith, ok := params["defaultIdentityStartCounterWith"]; ok {
+		startCounterWithInt, err := strconv.Atoi(defaultStartCounterWith)
+		if err != nil || startCounterWithInt <= 0 {
+			return DefaultIdentityOptions{}, fmt.Errorf("Invalid value for defaultIdentityStartCounterWith: %s, expected a positive integer.", defaultStartCounterWith)
+		}
+		defaultIdentityOptions.StartCounterWith = defaultStartCounterWith
+	}
+	return defaultIdentityOptions, nil
+}
+
+// This attempts to parse the skip range assuming the following format: "<min>-<max>" where both <min> and <max> are
+// positive integers and <min> is less than <max>.
+func parseDefaultSkipRange(defaultSkipRangeStr string) (string, string, error) {
+	parts := strings.Split(defaultSkipRangeStr, "-")
+	if len(parts) < 2 {
+		return "", "", fmt.Errorf("No range specified")
+	}
+	if len(parts) > 2 {
+		return "", "", fmt.Errorf("Too many elements in range")
+	}
+	skipRangeMinStr := parts[0]
+	skipRangeMaxStr := parts[1]
+	skipRangeMinInt, err := strconv.Atoi(skipRangeMinStr)
+	if err != nil {
+		return "", "", fmt.Errorf("Failed to convert min value to integer: %s", err)
+	}
+	skipRangeMaxInt, err := strconv.Atoi(skipRangeMaxStr)
+	if err != nil {
+		return "", "", fmt.Errorf("Failed to convert max value to integer: %s", err)
+	}
+	if skipRangeMinInt >= skipRangeMaxInt {
+		return "", "", fmt.Errorf("Min value must be less than max value")
+	}
+	return skipRangeMinStr, skipRangeMaxStr, nil
 }