-
Notifications
You must be signed in to change notification settings - Fork 29
Detect missing database and recreate & Fix enum conversion uppercase to lowercase etc. #121
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
a6e476c
Fix enum
jaredmdobson 40fba71
Fix enum parser
jaredmdobson aad9364
Fix formatting
jaredmdobson 883f426
Fix
jaredmdobson 727c19f
Fix
jaredmdobson 017e48e
Fix bug
jaredmdobson 53e4018
Add in enum translation fix
jaredmdobson 2fbec2e
[skip ci]
jaredmdobson fcf167a
[skip ci]
jaredmdobson 202c8c0
Check for temp as well
jaredmdobson f458ffd
Remove unsafe
jaredmdobson e9e9a2d
Remove unused
jaredmdobson d83f2de
Fix test
jaredmdobson a7d4ea7
Fix test
jaredmdobson 8ac2900
Fix test
jaredmdobson b3223cd
Fix test
jaredmdobson 037bdb0
Fix spec
jaredmdobson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| from .parser import parse_mysql_enum, is_enum_type | ||
| from .converter import EnumConverter | ||
| from .utils import find_enum_definition_end, extract_field_components | ||
| from .ddl_parser import ( | ||
| find_enum_or_set_definition_end, | ||
| parse_enum_or_set_field, | ||
| extract_enum_or_set_values, | ||
| strip_value | ||
| ) | ||
|
|
||
| __all__ = [ | ||
| 'parse_mysql_enum', | ||
| 'is_enum_type', | ||
| 'EnumConverter', | ||
| 'find_enum_definition_end', | ||
| 'extract_field_components', | ||
| 'find_enum_or_set_definition_end', | ||
| 'parse_enum_or_set_field', | ||
| 'extract_enum_or_set_values', | ||
| 'strip_value' | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| from typing import List, Union, Optional, Any | ||
| from logging import getLogger | ||
|
|
||
| # Create a single module-level logger | ||
| logger = getLogger(__name__) | ||
|
|
||
| class EnumConverter: | ||
| """Class to handle conversion of enum values between MySQL and ClickHouse""" | ||
|
|
||
| @staticmethod | ||
| def convert_mysql_to_clickhouse_enum( | ||
| value: Any, | ||
| enum_values: List[str], | ||
| field_name: str = "unknown" | ||
| ) -> Optional[Union[str, int]]: | ||
| """ | ||
| Convert a MySQL enum value to the appropriate ClickHouse representation | ||
|
|
||
| Args: | ||
| value: The MySQL enum value (can be int, str, None) | ||
| enum_values: List of possible enum string values | ||
| field_name: Name of the field (for better error reporting) | ||
|
|
||
| Returns: | ||
| The properly converted enum value for ClickHouse | ||
| """ | ||
| # Handle NULL values | ||
| if value is None: | ||
| return None | ||
|
|
||
| # Handle integer values (index-based) | ||
| if isinstance(value, int): | ||
| # Check if the value is 0 | ||
| if value == 0: | ||
| # Return 0 as-is - let ClickHouse handle it according to the field's nullability | ||
| logger.debug(f"ENUM CONVERSION: Found enum index 0 for field '{field_name}'. Keeping as 0.") | ||
| return 0 | ||
|
|
||
| # Validate that the enum index is within range | ||
| if value < 1 or value > len(enum_values): | ||
| # Log the issue | ||
| logger.error(f"ENUM CONVERSION: Invalid enum index {value} for field '{field_name}' " | ||
| f"with values {enum_values}") | ||
| # Return the value unchanged | ||
| return value | ||
| else: | ||
| # Convert to the string representation (lowercase to match our new convention) | ||
| return enum_values[int(value)-1].lower() | ||
|
|
||
| # Handle string values | ||
| elif isinstance(value, str): | ||
| # Validate that the string value exists in enum values | ||
| # First check case-sensitive, then case-insensitive | ||
| if value in enum_values: | ||
| return value.lower() | ||
|
|
||
| # Try case-insensitive match | ||
| lowercase_enum_values = [v.lower() for v in enum_values] | ||
| if value.lower() in lowercase_enum_values: | ||
| return value.lower() | ||
|
|
||
| # Value not found in enum values | ||
| logger.error(f"ENUM CONVERSION: Invalid enum value '{value}' not in {enum_values} " | ||
| f"for field '{field_name}'") | ||
| # Return the value unchanged | ||
| return value | ||
|
|
||
| # Handle any other unexpected types | ||
| else: | ||
| logger.error(f"ENUM CONVERSION: Unexpected type {type(value)} for enum field '{field_name}'") | ||
| # Return the value unchanged | ||
| return value |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could not remove this check.
We could start replication, and then interrupt until it finished and then continue. It will start from zero instead of trying to continue.
We should probably check for both target_database and target_database_tmp missing, and only in this case we are good to go with restarting.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay i modified it, what do you think now?
My only problem with it is that on boot if the database was not there, it would just continue as if it was 🤷🏿
I'm also available to do a call as well if needed. I'm on discord at: @jaredmdobson
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!