From 81d91f35f11679351bf772f1c65728ea213d4a58 Mon Sep 17 00:00:00 2001 From: Hal Ali Date: Mon, 5 Aug 2024 19:52:16 -0400 Subject: [PATCH] fix(targets): Quote add-column-ddl with column starting with `_` (underscore) based on engine (#2583) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix column quoting and add tests * remove mypy ignore --------- Co-authored-by: Edgar Ramírez Mondragón <16805946+edgarrmondragon@users.noreply.github.com> --- singer_sdk/connectors/sql.py | 2 +- .../camelcase_complex_schema.singer | 2 +- tests/samples/test_target_sqlite.py | 18 ++++++++++++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 283fe183fe..f482226406 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -998,7 +998,7 @@ def get_column_add_ddl( column_type, ), ) - compiled = create_column_clause.compile(self._engine) + compiled = create_column_clause.compile(self._engine).string return sa.DDL( "ALTER TABLE %(table_name)s ADD COLUMN %(create_column_clause)s", { diff --git a/singer_sdk/testing/target_test_streams/camelcase_complex_schema.singer b/singer_sdk/testing/target_test_streams/camelcase_complex_schema.singer index 03330c7246..8c5d66fb6d 100644 --- a/singer_sdk/testing/target_test_streams/camelcase_complex_schema.singer +++ b/singer_sdk/testing/target_test_streams/camelcase_complex_schema.singer @@ -1,2 +1,2 @@ {"type": "SCHEMA", "stream": "ForecastingTypeToCategory", "schema": {"properties": {"Id": {"type": "string"}, "IsDeleted": {"type": ["null", "boolean"]}, "CreatedDate": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "CreatedById": {"type": ["null", "string"]}, "LastModifiedDate": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "LastModifiedById": {"type": ["null", "string"]}, "SystemModstamp": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "ForecastingTypeId": {"type": ["null", "string"]}, "ForecastingItemCategory": {"type": ["null", "string"]}, "DisplayPosition": {"type": ["null", "integer"]}, "IsAdjustable": {"type": ["null", "boolean"]}, "IsOwnerAdjustable": {"type": ["null", "boolean"]}}, "type": "object", "additionalProperties": false}, "key_properties": ["Id"]} -{"type": "SCHEMA", "stream": "ForecastingTypeToCategory", "schema": {"properties": {"Id": {"type": "string"}, "IsDeleted": {"type": ["null", "boolean"]}, "CreatedDate": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "CreatedById": {"type": ["null", "string"]}, "LastModifiedDate": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "LastModifiedById": {"type": ["null", "string"]}, "SystemModstamp": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "ForecastingTypeId": {"type": ["null", "string"]}, "ForecastingItemCategory": {"type": ["null", "string"]}, "DisplayPosition": {"type": ["null", "integer"]}, "IsAdjustable": {"type": ["null", "boolean"]}, "IsOwnerAdjustable": {"type": ["null", "boolean"]}, "age": {"type": "integer"}, "NewCamelCasedAttribute": {"type": "string"}}, "type": "object", "additionalProperties": false}, "key_properties": ["Id"]} +{"type": "SCHEMA", "stream": "ForecastingTypeToCategory", "schema": {"properties": {"Id": {"type": "string"}, "IsDeleted": {"type": ["null", "boolean"]}, "CreatedDate": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "CreatedById": {"type": ["null", "string"]}, "LastModifiedDate": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "LastModifiedById": {"type": ["null", "string"]}, "SystemModstamp": {"anyOf": [{"type": "string", "format": "date-time"}, {"type": ["string", "null"]}]}, "ForecastingTypeId": {"type": ["null", "string"]}, "ForecastingItemCategory": {"type": ["null", "string"]}, "DisplayPosition": {"type": ["null", "integer"]}, "IsAdjustable": {"type": ["null", "boolean"]}, "IsOwnerAdjustable": {"type": ["null", "boolean"]}, "age": {"type": "integer"}, "NewCamelCasedAttribute": {"type": "string"}, "_attribute_startswith_underscore": {"type": "string"}}, "type": "object", "additionalProperties": false}, "key_properties": ["Id"]} diff --git a/tests/samples/test_target_sqlite.py b/tests/samples/test_target_sqlite.py index edf88ee926..4f6d54e603 100644 --- a/tests/samples/test_target_sqlite.py +++ b/tests/samples/test_target_sqlite.py @@ -182,7 +182,9 @@ def test_sqlite_column_addition(sqlite_sample_target: SQLTarget): props_a: dict[str, dict] = {"col_a": th.StringType().to_dict()} props_b = deepcopy(props_a) props_b["col_b"] = th.IntegerType().to_dict() - schema_msg_a, schema_msg_b = ( + props_c = deepcopy(props_b) + props_c["_col_c"] = th.IntegerType().to_dict() + schema_msg_a, schema_msg_b, schema_msg_c = ( { "type": "SCHEMA", "stream": test_tbl, @@ -191,7 +193,7 @@ def test_sqlite_column_addition(sqlite_sample_target: SQLTarget): "properties": props, }, } - for props in [props_a, props_b] + for props in [props_a, props_b, props_c] ) tap_output_a = "\n".join( json.dumps(msg) @@ -211,8 +213,20 @@ def test_sqlite_column_addition(sqlite_sample_target: SQLTarget): }, ] ) + tap_output_c = "\n".join( + json.dumps(msg) + for msg in [ + schema_msg_c, + { + "type": "RECORD", + "stream": test_tbl, + "record": {"col_a": "samplerow2", "col_b": 2, "_col_c": 3}, + }, + ] + ) target_sync_test(sqlite_sample_target, input=StringIO(tap_output_a), finalize=True) target_sync_test(sqlite_sample_target, input=StringIO(tap_output_b), finalize=True) + target_sync_test(sqlite_sample_target, input=StringIO(tap_output_c), finalize=True) def test_sqlite_activate_version(