Fix #1502 Add triggers support (#1503)

* Add trigger support to gh-ost based on openark#30

* Add comprehensive test cases for trigger support functionality

* Fix trigger-basic test by adding required --trigger-suffix parameter and remove fail-trigger-unsupported test

* Update trigger suffix in local test configurations

Modify extra_args files for trigger-complex, trigger-multiple, and trigger-self-reference tests to use different trigger suffixes

* Add --remove-trigger-suffix-if-exists to local test configurations

Update extra_args files for trigger tests to include the new --remove-trigger-suffix-if-exists option, ensuring consistent trigger handling across different test scenarios

* Fix trigger-long-name test by reducing trigger name length to be valid

* Standardize trigger drop statements in local test configurations

Update create.sql files across trigger test scenarios to:
- Consistently drop both original and ghost triggers
- Ensure clean slate before creating triggers
- Align with recent trigger suffix changes

This change improves test reliability and consistency by explicitly dropping all potential trigger variations before test setup.

* Consolidate and enhance trigger test configurations

Refactor local test scenarios for triggers by:
- Merging multiple trigger test configurations into comprehensive test cases
- Updating create.sql files with more complex and diverse trigger scenarios
- Standardizing trigger and table setup across different test configurations
- Removing redundant test directories while preserving test coverage

This change simplifies the trigger testing infrastructure and provides more robust test coverage for gh-ost's trigger handling capabilities.

* Add debug logging for ghost trigger validation process

Enhance ghost trigger existence check by adding detailed debug logging to:
- Log the ghost trigger name being searched
- Log the database schema and query details
- Log when an existing ghost trigger is found

This change improves visibility into the trigger validation process, making troubleshooting easier during migration scenarios.

* Improve ghost trigger validation with enhanced logging and verification

Modify validateGhostTriggersDontExist() to:
- Add a direct query to log all triggers in the database schema
- Refactor trigger existence check to use count-based query
- Improve debug logging for trigger validation process
- Provide more detailed error reporting for existing ghost triggers

This change enhances the robustness and observability of the trigger validation mechanism in gh-ost's migration process.

* Refactor ghost trigger validation to improve logging and error detection

Simplify and enhance the validateGhostTriggersDontExist() method by:
- Removing redundant direct query logging
- Streamlining trigger existence check
- Improving debug logging for trigger validation
- Consolidating trigger existence detection logic

The changes provide more concise and focused trigger validation with clearer error reporting.

* Simplify ghost trigger validation query and reduce logging verbosity

Refactor validateGhostTriggersDontExist() to:
- Streamline trigger existence check query
- Remove redundant debug logging statements
- Use a more concise approach to detecting existing triggers

The changes reduce code complexity while maintaining the core validation logic for ghost triggers.

* Enhance ghost trigger validation query to include table name filter

Modify validateGhostTriggersDontExist() to:
- Add table name filter to trigger existence check query
- Improve specificity of ghost trigger detection
- Prevent false positives from similarly named triggers in different tables

The change ensures more precise ghost trigger validation by incorporating the original table name into the query criteria.

* Enhance trigger test configuration with advanced features and consolidated test scenarios

Update trigger-advanced-features test configuration to:
- Add new column 'color' and 'modified_count' to test table
- Implement more complex trigger logic for color and count tracking
- Consolidate trigger test scenarios with richer data transformations
- Modify event to test both numeric and color-based updates
- Remove redundant trigger-basic-features directory

The changes provide a more comprehensive and nuanced test suite for gh-ost's trigger handling capabilities, demonstrating advanced trigger behaviors and self-referencing updates.

* Fix lint errors

* Update trigger test configuration with suffix change

Modify the extra_args file to use '_ght' trigger suffix instead of '_gho', maintaining consistency with recent trigger test configuration updates.

* Remove gh-ost-ci-env submodule

Clean up repository by removing the gh-ost-ci-env submodule, which appears to be no longer needed in the project structure.

* Update create.sql

---------

Co-authored-by: Yakir Gibraltar <yakir.g@taboola.com>

go/base/context.go

@@ -14,6 +14,7 @@ import (
 	"sync"
 	"sync/atomic"
 	"time"
+	"unicode/utf8"
 
 	uuid "github.com/google/uuid"
 
@@ -237,6 +238,11 @@ type MigrationContext struct {
 	MigrationIterationRangeMaxValues *sql.ColumnValues
 	ForceTmpTableName                string
 
+	IncludeTriggers     bool
+	RemoveTriggerSuffix bool
+	TriggerSuffix       string
+	Triggers            []mysql.Trigger
+
 	recentBinlogCoordinates mysql.BinlogCoordinates
 
 	BinlogSyncerMaxReconnectAttempts int
@@ -924,3 +930,20 @@ func (this *MigrationContext) ReadConfigFile() error {
 
 	return nil
 }
+
+// getGhostTriggerName generates the name of a ghost trigger, based on original trigger name
+// or a given trigger name
+func (this *MigrationContext) GetGhostTriggerName(triggerName string) string {
+	if this.RemoveTriggerSuffix && strings.HasSuffix(triggerName, this.TriggerSuffix) {
+		return strings.TrimSuffix(triggerName, this.TriggerSuffix)
+	}
+	// else
+	return triggerName + this.TriggerSuffix
+}
+
+// validateGhostTriggerLength check if the ghost trigger name length is not more than 64 characters
+func (this *MigrationContext) ValidateGhostTriggerLengthBelowMaxLength(triggerName string) bool {
+	ghostTriggerName := this.GetGhostTriggerName(triggerName)
+
+	return utf8.RuneCountInString(ghostTriggerName) <= mysql.MaxTableNameLength
+}

go/base/context_test.go

@@ -7,6 +7,7 @@ package base
 
 import (
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -58,6 +59,68 @@ func TestGetTableNames(t *testing.T) {
 	}
 }
 
+func TestGetTriggerNames(t *testing.T) {
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		require.Equal(t, "my_trigger"+context.TriggerSuffix, context.GetGhostTriggerName("my_trigger"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		context.RemoveTriggerSuffix = true
+		require.Equal(t, "my_trigger"+context.TriggerSuffix, context.GetGhostTriggerName("my_trigger"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		context.RemoveTriggerSuffix = true
+		require.Equal(t, "my_trigger", context.GetGhostTriggerName("my_trigger_gho"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		context.RemoveTriggerSuffix = false
+		require.Equal(t, "my_trigger_gho_gho", context.GetGhostTriggerName("my_trigger_gho"))
+	}
+}
+
+func TestValidateGhostTriggerLengthBelowMaxLength(t *testing.T) {
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_gho"
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength("my_trigger"))
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		require.False(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4))) // 64 characters + "_ghost"
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 3))) // 48 characters + "_ghost"
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		context.RemoveTriggerSuffix = true
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4))) // 64 characters + "_ghost" removed
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		context.RemoveTriggerSuffix = true
+		require.False(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4)+"X")) // 65 characters + "_ghost" not removed
+	}
+	{
+		context := NewMigrationContext()
+		context.TriggerSuffix = "_ghost"
+		context.RemoveTriggerSuffix = true
+		require.True(t, context.ValidateGhostTriggerLengthBelowMaxLength(strings.Repeat("my_trigger_ghost", 4)+"_ghost")) // 70 characters + last "_ghost"  removed
+	}
+}
+
 func TestReadConfigFile(t *testing.T) {
 	{
 		context := NewMigrationContext()

go/cmd/gh-ost/main.go

@@ -11,6 +11,7 @@ import (
 	"net/url"
 	"os"
 	"os/signal"
+	"regexp"
 	"syscall"
 
 	"github.com/github/gh-ost/go/base"
@@ -137,6 +138,10 @@ func main() {
 	flag.UintVar(&migrationContext.ReplicaServerId, "replica-server-id", 99999, "server id used by gh-ost process. Default: 99999")
 	flag.IntVar(&migrationContext.BinlogSyncerMaxReconnectAttempts, "binlogsyncer-max-reconnect-attempts", 0, "when master node fails, the maximum number of binlog synchronization attempts to reconnect. 0 is unlimited")
 
+	flag.BoolVar(&migrationContext.IncludeTriggers, "include-triggers", false, "When true, the triggers (if exist) will be created on the new table")
+	flag.StringVar(&migrationContext.TriggerSuffix, "trigger-suffix", "", "Add a suffix to the trigger name (i.e '_v2'). Requires '--include-triggers'")
+	flag.BoolVar(&migrationContext.RemoveTriggerSuffix, "remove-trigger-suffix-if-exists", false, "Remove given suffix from name of trigger. Requires '--include-triggers' and '--trigger-suffix'")
+
 	maxLoad := flag.String("max-load", "", "Comma delimited status-name=threshold. e.g: 'Threads_running=100,Threads_connected=500'. When status exceeds threshold, app throttles writes")
 	criticalLoad := flag.String("critical-load", "", "Comma delimited status-name=threshold, same format as --max-load. When status exceeds threshold, app panics and quits")
 	flag.Int64Var(&migrationContext.CriticalLoadIntervalMilliseconds, "critical-load-interval-millis", 0, "When 0, migration immediately bails out upon meeting critical-load. When non-zero, a second check is done after given interval, and migration only bails out if 2nd check still meets critical load")
@@ -257,7 +262,20 @@ func main() {
 		migrationContext.Log.Fatal("--ssl-allow-insecure requires --ssl")
 	}
 	if *replicationLagQuery != "" {
-		migrationContext.Log.Warning("--replication-lag-query is deprecated")
+		migrationContext.Log.Warningf("--replication-lag-query is deprecated")
+	}
+	if migrationContext.IncludeTriggers && migrationContext.TriggerSuffix == "" {
+		migrationContext.Log.Fatalf("--trigger-suffix must be used with --include-triggers")
+	}
+	if !migrationContext.IncludeTriggers && migrationContext.TriggerSuffix != "" {
+		migrationContext.Log.Fatalf("--trigger-suffix cannot be be used without --include-triggers")
+	}
+	if migrationContext.TriggerSuffix != "" {
+		regex := regexp.MustCompile(`^[\da-zA-Z_]+$`)
+
+		if !regex.Match([]byte(migrationContext.TriggerSuffix)) {
+			migrationContext.Log.Fatalf("--trigger-suffix must contain only alpha numeric characters and underscore (0-9,a-z,A-Z,_)")
+		}
 	}
 	if *storageEngine == "rocksdb" {
 		migrationContext.Log.Warning("RocksDB storage engine support is experimental")

go/logic/applier.go

@@ -415,6 +415,56 @@ func (this *Applier) dropTable(tableName string) error {
 	return nil
 }
 
+// dropTriggers drop the triggers on the applied host
+func (this *Applier) DropTriggersFromGhost() error {
+	if len(this.migrationContext.Triggers) > 0 {
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			query := fmt.Sprintf("drop trigger if exists %s", sql.EscapeName(triggerName))
+			_, err := sqlutils.ExecNoPrepare(this.db, query)
+			if err != nil {
+				return err
+			}
+			this.migrationContext.Log.Infof("Trigger '%s' dropped", triggerName)
+		}
+	}
+	return nil
+}
+
+// createTriggers creates the triggers on the applied host
+func (this *Applier) createTriggers(tableName string) error {
+	if len(this.migrationContext.Triggers) > 0 {
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			query := fmt.Sprintf(`create /* gh-ost */ trigger %s %s %s on %s.%s for each row
+		%s`,
+				sql.EscapeName(triggerName),
+				trigger.Timing,
+				trigger.Event,
+				sql.EscapeName(this.migrationContext.DatabaseName),
+				sql.EscapeName(tableName),
+				trigger.Statement,
+			)
+			this.migrationContext.Log.Infof("Createing trigger %s on %s.%s",
+				sql.EscapeName(triggerName),
+				sql.EscapeName(this.migrationContext.DatabaseName),
+				sql.EscapeName(tableName),
+			)
+			if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil {
+				return err
+			}
+		}
+		this.migrationContext.Log.Infof("Triggers created on %s", tableName)
+	}
+	return nil
+}
+
+// CreateTriggers creates the original triggers on applier host
+func (this *Applier) CreateTriggersOnGhost() error {
+	err := this.createTriggers(this.migrationContext.GetGhostTableName())
+	return err
+}
+
 // DropChangelogTable drops the changelog table on the applier host
 func (this *Applier) DropChangelogTable() error {
 	return this.dropTable(this.migrationContext.GetChangelogTableName())

go/logic/inspect.go

@@ -531,7 +531,7 @@ func (this *Inspector) validateTableForeignKeys(allowChildForeignKeys bool) erro
 	return nil
 }
 
-// validateTableTriggers makes sure no triggers exist on the migrated table
+// validateTableTriggers makes sure no triggers exist on the migrated table. if --include_triggers is used then it fetches the triggers
 func (this *Inspector) validateTableTriggers() error {
 	query := `
 		SELECT /* gh-ost */ COUNT(*) AS num_triggers
@@ -553,12 +553,72 @@ func (this *Inspector) validateTableTriggers() error {
 		return err
 	}
 	if numTriggers > 0 {
-		return this.migrationContext.Log.Errorf("Found triggers on %s.%s. Triggers are not supported at this time. Bailing out", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
+		if this.migrationContext.IncludeTriggers {
+			this.migrationContext.Log.Infof("Found %d triggers on %s.%s.", numTriggers, sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
+			this.migrationContext.Triggers, err = mysql.GetTriggers(this.db, this.migrationContext.DatabaseName, this.migrationContext.OriginalTableName)
+			if err != nil {
+				return err
+			}
+			if err := this.validateGhostTriggersDontExist(); err != nil {
+				return err
+			}
+			if err := this.validateGhostTriggersLength(); err != nil {
+				return err
+			}
+			return nil
+		}
+		return this.migrationContext.Log.Errorf("Found triggers on %s.%s. Tables with triggers are supported only when using \"include-triggers\" flag. Bailing out", sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))
 	}
 	this.migrationContext.Log.Debugf("Validated no triggers exist on table")
 	return nil
 }
 
+// verifyTriggersDontExist verifies before createing new triggers we want to make sure these triggers dont exist already in the DB
+func (this *Inspector) validateGhostTriggersDontExist() error {
+	if len(this.migrationContext.Triggers) > 0 {
+		var foundTriggers []string
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			query := "select 1 from information_schema.triggers where trigger_name = ? and trigger_schema = ? and event_object_table = ?"
+			err := sqlutils.QueryRowsMap(this.db, query, func(rowMap sqlutils.RowMap) error {
+				triggerExists := rowMap.GetInt("1")
+				if triggerExists == 1 {
+					foundTriggers = append(foundTriggers, triggerName)
+				}
+				return nil
+			},
+				triggerName,
+				this.migrationContext.DatabaseName,
+				this.migrationContext.OriginalTableName,
+			)
+			if err != nil {
+				return err
+			}
+		}
+		if len(foundTriggers) > 0 {
+			return this.migrationContext.Log.Errorf("Found gh-ost triggers (%s). Please use a different suffix or drop them. Bailing out", strings.Join(foundTriggers, ","))
+		}
+	}
+
+	return nil
+}
+
+func (this *Inspector) validateGhostTriggersLength() error {
+	if len(this.migrationContext.Triggers) > 0 {
+		var foundTriggers []string
+		for _, trigger := range this.migrationContext.Triggers {
+			triggerName := this.migrationContext.GetGhostTriggerName(trigger.Name)
+			if ok := this.migrationContext.ValidateGhostTriggerLengthBelowMaxLength(triggerName); !ok {
+				foundTriggers = append(foundTriggers, triggerName)
+			}
+		}
+		if len(foundTriggers) > 0 {
+			return this.migrationContext.Log.Errorf("Gh-ost triggers (%s) length > %d characters. Bailing out", strings.Join(foundTriggers, ","), mysql.MaxTableNameLength)
+		}
+	}
+	return nil
+}
+
 // estimateTableRowsViaExplain estimates number of rows on original table
 func (this *Inspector) estimateTableRowsViaExplain() error {
 	query := fmt.Sprintf(`explain select /* gh-ost */ * from %s.%s where 1=1`, sql.EscapeName(this.migrationContext.DatabaseName), sql.EscapeName(this.migrationContext.OriginalTableName))

go/logic/migrator.go

@@ -632,6 +632,12 @@ func (this *Migrator) cutOverTwoStep() (err error) {
 	if err := this.retryOperation(this.waitForEventsUpToLock); err != nil {
 		return err
 	}
+	// If we need to create triggers we need to do it here (only create part)
+	if this.migrationContext.IncludeTriggers && len(this.migrationContext.Triggers) > 0 {
+		if err := this.retryOperation(this.applier.CreateTriggersOnGhost); err != nil {
+			return err
+		}
+	}
 	if err := this.retryOperation(this.applier.SwapTablesQuickAndBumpy); err != nil {
 		return err
 	}
@@ -676,6 +682,13 @@ func (this *Migrator) atomicCutOver() (err error) {
 		return this.migrationContext.Log.Errore(err)
 	}
 
+	// If we need to create triggers we need to do it here (only create part)
+	if this.migrationContext.IncludeTriggers && len(this.migrationContext.Triggers) > 0 {
+		if err := this.applier.CreateTriggersOnGhost(); err != nil {
+			this.migrationContext.Log.Errore(err)
+		}
+	}
+
 	// Step 2
 	// We now attempt an atomic RENAME on original & ghost tables, and expect it to block.
 	this.migrationContext.RenameTablesStartTime = time.Now()

go/mysql/utils.go

@@ -30,6 +30,13 @@ type ReplicationLagResult struct {
 	Err error
 }
 
+type Trigger struct {
+	Name      string
+	Event     string
+	Statement string
+	Timing    string
+}
+
 func NewNoReplicationLagResult() *ReplicationLagResult {
 	return &ReplicationLagResult{Lag: 0, Err: nil}
 }
@@ -224,3 +231,24 @@ func Kill(db *gosql.DB, connectionID string) error {
 	_, err := db.Exec(`KILL QUERY %s`, connectionID)
 	return err
 }
+
+// GetTriggers reads trigger list from given table
+func GetTriggers(db *gosql.DB, databaseName, tableName string) (triggers []Trigger, err error) {
+	query := fmt.Sprintf(`select trigger_name as name, event_manipulation as event, action_statement as statement, action_timing as timing
+	from information_schema.triggers 
+	where trigger_schema = '%s' and event_object_table = '%s'`, databaseName, tableName)
+
+	err = sqlutils.QueryRowsMap(db, query, func(rowMap sqlutils.RowMap) error {
+		triggers = append(triggers, Trigger{
+			Name:      rowMap.GetString("name"),
+			Event:     rowMap.GetString("event"),
+			Statement: rowMap.GetString("statement"),
+			Timing:    rowMap.GetString("timing"),
+		})
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return triggers, nil
+}