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.

Sign up for GitHub

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

Jump to bottom

Fix #1502 Add triggers support #1503

Merged
merged 18 commits into from
Mar 3, 2025
Merged

Fix #1502 Add triggers support #1503

merged 18 commits into from
Mar 3, 2025
+407 −3

Conversation

yakirgb
Copy link
Contributor

@yakirgb yakirgb commented Mar 3, 2025

Fix #1502
Based on https://github.com/openark/gh-ost/pull/30/files
Also added localtests, and one minor change compare to openark repo,
Changed the query of :

// 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 = ?

To include also : and event_object_table = ?"

@Yakir-Taboola
Add trigger support to gh-ost based on openark#30
94fa782
@Yakir-Taboola
Add comprehensive test cases for trigger support functionality
afd5dd9
@Yakir-Taboola
Fix trigger-basic test by adding required --trigger-suffix parameter …
82abf3c 
…and remove fail-trigger-unsupported test
@Yakir-Taboola
Update trigger suffix in local test configurations
7381b9b 
Modify extra_args files for trigger-complex, trigger-multiple, and trigger-self-reference tests to use different trigger suffixes
@Yakir-Taboola
Add --remove-trigger-suffix-if-exists to local test configurations
d3fa7a1 
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
@Yakir-Taboola
Fix trigger-long-name test by reducing trigger name length to be valid
0b2183e
@Yakir-Taboola
Standardize trigger drop statements in local test configurations
f8a820a 
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.
@Yakir-Taboola
Consolidate and enhance trigger test configurations
186ce33 
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.
@Yakir-Taboola
Add debug logging for ghost trigger validation process
a6518ee 
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.
@Yakir-Taboola
Improve ghost trigger validation with enhanced logging and verification
e764d7b 
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.
@Yakir-Taboola
Refactor ghost trigger validation to improve logging and error detection
73e42eb 
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.
@Yakir-Taboola
Simplify ghost trigger validation query and reduce logging verbosity
2006998 
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.
@Yakir-Taboola
Enhance ghost trigger validation query to include table name filter
3b00d64 
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.
@Yakir-Taboola
Enhance trigger test configuration with advanced features and consoli…
dc9c563 
…dated 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.
@Yakir-Taboola
Fix lint errors
e83c4e2
@Yakir-Taboola
Update trigger test configuration with suffix change
e0d325a 
Modify the extra_args file to use '_ght' trigger suffix instead of '_gho', maintaining consistency with recent trigger test configuration updates.
@Yakir-Taboola
Remove gh-ost-ci-env submodule
8ec1811 
Clean up repository by removing the gh-ost-ci-env submodule, which appears to be no longer needed in the project structure.
@yakirgb yakirgb changed the title Fix https://github.com/github/gh-ost/issues/1502 Fix #1502 Mar 3, 2025
@yakirgb yakirgb changed the title Fix #1502 Fix #1502 Add triggers support Mar 3, 2025
@meiji163
Copy link
Contributor

meiji163 commented Mar 3, 2025

Thanks for opening this backport, LGTM!

@meiji163 meiji163 merged commit 0263a20 into github:master Mar 3, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meiji163 meiji163 meiji163 approved these changes

@rashiq rashiq Awaiting requested review from rashiq rashiq is a code owner

@timvaillancourt timvaillancourt Awaiting requested review from timvaillancourt timvaillancourt is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Backport openark/gh-ost/pull/30 to this repo - add triggers support
3 participants
@yakirgb @meiji163 @Yakir-Taboola