From 7f89c0b7b147493e0c56a61b4216a961666821d4 Mon Sep 17 00:00:00 2001 From: Dan Norman Date: Tue, 9 Aug 2022 12:48:14 -0700 Subject: [PATCH 1/4] new check added to streams sql _adapt_column_type and comments to merge_sql_types --- singer_sdk/streams/sql.py | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/singer_sdk/streams/sql.py b/singer_sdk/streams/sql.py index dfafa2e3e..725f9bd38 100644 --- a/singer_sdk/streams/sql.py +++ b/singer_sdk/streams/sql.py @@ -719,14 +719,19 @@ def merge_sql_types( if len(sql_types) == 1: return sql_types[0] + # Convert the two types given into a sorted list + # containing the best conversion classes sql_types = self._sort_types(sql_types) + # If greater than two evaluate the first pair then on down the line if len(sql_types) > 2: return self.merge_sql_types( [self.merge_sql_types([sql_types[0], sql_types[1]])] + sql_types[2:] ) assert len(sql_types) == 2 + + # Get the generic type class generic_type = type(sql_types[0].as_generic()) if isinstance(generic_type, type): if issubclass( @@ -825,9 +830,33 @@ def _adapt_column_type( Raises: NotImplementedError: if altering columns is not supported. """ - current_type = self._get_column_type(full_table_name, column_name) + current_type: sqlalchemy.types.TypeEngine = self._get_column_type( + full_table_name, column_name + ) + + # Check if the existing column type and the sql type are the same + if isinstance(sql_type, type(current_type)): + # The current column and sql type are the same + # Nothing to do + return + # Check if the existing column generic type + # and sql generic type are the same + elif isinstance(sql_type.as_generic(), type(current_type.as_generic())): + # The current column and sql generic types are the same + # Nothing to do + return + # Check if the new type is a subclass of the current type + elif issubclass(type(sql_type.as_generic()), type(current_type)): + # Since it is a sub class of the current type + # a conversion on insert will happen + # Nothing to do + return + + # Not the same type, generic type or compatible types + # calling merge_sql_types for assistnace compatible_sql_type = self.merge_sql_types([current_type, sql_type]) - if current_type == compatible_sql_type: + + if compatible_sql_type == current_type: # Nothing to do return From 4e941ba8207f0cc8dc3cfc5f27ad752bdbc99294 Mon Sep 17 00:00:00 2001 From: Dan Norman Date: Mon, 15 Aug 2022 09:21:28 -0700 Subject: [PATCH 2/4] Removed issubclass check did work as intended --- singer_sdk/streams/sql.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/singer_sdk/streams/sql.py b/singer_sdk/streams/sql.py index 725f9bd38..9df3ea8cb 100644 --- a/singer_sdk/streams/sql.py +++ b/singer_sdk/streams/sql.py @@ -845,12 +845,6 @@ def _adapt_column_type( # The current column and sql generic types are the same # Nothing to do return - # Check if the new type is a subclass of the current type - elif issubclass(type(sql_type.as_generic()), type(current_type)): - # Since it is a sub class of the current type - # a conversion on insert will happen - # Nothing to do - return # Not the same type, generic type or compatible types # calling merge_sql_types for assistnace From 836bf4e97cd6bf3a68cc8f449fda050ceb6f2780 Mon Sep 17 00:00:00 2001 From: Dan Norman Date: Thu, 18 Aug 2022 13:48:30 -0700 Subject: [PATCH 3/4] Changed to a single string to string check --- singer_sdk/streams/sql.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/singer_sdk/streams/sql.py b/singer_sdk/streams/sql.py index 9df3ea8cb..400e56b2b 100644 --- a/singer_sdk/streams/sql.py +++ b/singer_sdk/streams/sql.py @@ -835,16 +835,10 @@ def _adapt_column_type( ) # Check if the existing column type and the sql type are the same - if isinstance(sql_type, type(current_type)): + if str(sql_type) == str(current_type): # The current column and sql type are the same # Nothing to do return - # Check if the existing column generic type - # and sql generic type are the same - elif isinstance(sql_type.as_generic(), type(current_type.as_generic())): - # The current column and sql generic types are the same - # Nothing to do - return # Not the same type, generic type or compatible types # calling merge_sql_types for assistnace From 0e7bc5e722f76f851fed8d65859ea5d5f8d650d4 Mon Sep 17 00:00:00 2001 From: Dan Norman Date: Fri, 19 Aug 2022 09:11:26 -0700 Subject: [PATCH 4/4] update of merge_sql_types to match _adapt_column_type changes --- singer_sdk/streams/sql.py | 51 +++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/singer_sdk/streams/sql.py b/singer_sdk/streams/sql.py index 400e56b2b..1d236d856 100644 --- a/singer_sdk/streams/sql.py +++ b/singer_sdk/streams/sql.py @@ -719,6 +719,17 @@ def merge_sql_types( if len(sql_types) == 1: return sql_types[0] + # Gathering Type to match variables + # sent in _adapt_column_type + current_type = sql_types[0] + # sql_type = sql_types[1] + + # Getting the length of each type + # current_type_len: int = getattr(sql_types[0], "length", 0) + sql_type_len: int = getattr(sql_types[1], "length", 0) + if sql_type_len is None: + sql_type_len = 0 + # Convert the two types given into a sorted list # containing the best conversion classes sql_types = self._sort_types(sql_types) @@ -730,21 +741,31 @@ def merge_sql_types( ) assert len(sql_types) == 2 - # Get the generic type class - generic_type = type(sql_types[0].as_generic()) - if isinstance(generic_type, type): - if issubclass( - generic_type, - (sqlalchemy.types.String, sqlalchemy.types.Unicode), - ): - return sql_types[0] - - elif isinstance( - generic_type, - (sqlalchemy.types.String, sqlalchemy.types.Unicode), - ): - return sql_types[0] + for opt in sql_types: + # Get the length + opt_len: int = getattr(opt, "length", 0) + generic_type = type(opt.as_generic()) + + if isinstance(generic_type, type): + if issubclass( + generic_type, + (sqlalchemy.types.String, sqlalchemy.types.Unicode), + ): + # If length None or 0 then is varchar max ? + if (opt_len is None) or (opt_len == 0): + return opt + elif isinstance( + generic_type, + (sqlalchemy.types.String, sqlalchemy.types.Unicode), + ): + # If length None or 0 then is varchar max ? + if (opt_len is None) or (opt_len == 0): + return opt + # If best conversion class is equal to current type + # return the best conversion class + elif str(opt) == str(current_type): + return opt raise ValueError( f"Unable to merge sql types: {', '.join([str(t) for t in sql_types])}" @@ -844,7 +865,7 @@ def _adapt_column_type( # calling merge_sql_types for assistnace compatible_sql_type = self.merge_sql_types([current_type, sql_type]) - if compatible_sql_type == current_type: + if str(compatible_sql_type) == str(current_type): # Nothing to do return