Repository: meltano/sdk
Pull request #2843 (2025-01-28 17:01:33)

fix: Use a SQLAlchemy to generate an insert statement
## Summary by Sourcery

Update insert statements to use SQLAlchemy executables.

Bug Fixes:
- Fix insert statements to use SQLAlchemy to prevent SQL injection vulnerabilities.

Enhancements:
- Refactor SQL insert statement generation to use SQLAlchemy.

Tests:
- Update tests to reflect the changes in insert statement generation.

<!-- readthedocs-preview meltano-sdk start -->
----
📚 Documentation preview 📚: https://meltano-sdk--2843.org.readthedocs.build/en/2843/

<!-- readthedocs-preview meltano-sdk end -->

sourcery-ai[bot]: <!-- Generated by sourcery-ai[bot]: start review_guide -->

🧙 Sourcery is reviewing your pull request!

---

<details>
<summary>Tips and commands</summary>

#### Interacting with Sourcery

- **Trigger a new review:** Comment `@sourcery-ai review` on the pull request.
- **Continue discussions:** Reply directly to Sourcery's review comments.
- **Generate a GitHub issue from a review comment:** Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with `@sourcery-ai issue` to create an issue from it.
- **Generate a pull request title:** Write `@sourcery-ai` anywhere in the pull
  request title to generate a title at any time. You can also comment
  `@sourcery-ai title` on the pull request to (re-)generate the title at any time.
- **Generate a pull request summary:** Write `@sourcery-ai summary` anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment `@sourcery-ai summary` on the pull request to
  (re-)generate the summary at any time.
- **Generate reviewer's guide:** Comment `@sourcery-ai guide` on the pull
  request to (re-)generate the reviewer's guide at any time.
- **Resolve all Sourcery comments:** Comment `@sourcery-ai resolve` on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
- **Dismiss all Sourcery reviews:** Comment `@sourcery-ai dismiss` on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  `@sourcery-ai review` to trigger a new review!
- **Generate a plan of action for an issue:** Comment `@sourcery-ai plan` on
  an issue to generate a plan of action for it.

#### Customizing Your Experience

Access your [dashboard](https://app.sourcery.ai) to:
- Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
- Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.

#### Getting Help

- [Contact our support team](mailto:support@sourcery.ai) for questions or feedback.
- Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information.
- Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).

</details>

<!-- Generated by sourcery-ai[bot]: end review_guide -->
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/meltano/sdk/pull/2843?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) Report
Attention: Patch coverage is `88.88889%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 90.52%. Comparing base [(`e0deb8f`)](https://app.codecov.io/gh/meltano/sdk/commit/e0deb8f3e2eccefb660cd1bc535adafb494a08ae?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) to head [(`4b216a0`)](https://app.codecov.io/gh/meltano/sdk/commit/4b216a0c113924d6c08927b04a2764ec1eee63aa?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano).
> Report is 2 commits behind head on main.

| [Files with missing lines](https://app.codecov.io/gh/meltano/sdk/pull/2843?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) | Patch % | Lines |
|---|---|---|
| [singer\_sdk/sinks/sql.py](https://app.codecov.io/gh/meltano/sdk/pull/2843?src=pr&el=tree&filepath=singer_sdk%2Fsinks%2Fsql.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano#diff-c2luZ2VyX3Nkay9zaW5rcy9zcWwucHk=) | 80.00% | [1 Missing :warning: ](https://app.codecov.io/gh/meltano/sdk/pull/2843?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano) |

:x: Your patch check has failed because the patch coverage (88.88%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2843      +/-   ##
==========================================
- Coverage   91.31%   90.52%   -0.79%     
==========================================
  Files          62       62              
  Lines        5206     5205       -1     
  Branches      672      672              
==========================================
- Hits         4754     4712      -42     
- Misses        319      352      +33     
- Partials      133      141       +8     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/meltano/sdk/pull/2843?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=meltano).

codspeed-hq[bot]: <!-- __CODSPEED_PERFORMANCE_REPORT_COMMENT__ -->
<!-- __CODSPEED_INSTRUMENTATION_PERFORMANCE_REPORT_COMMENT__ -->

## [CodSpeed Performance Report](https://codspeed.io/meltano/sdk/branches/edgarrmondragon%2Ffix%2Fsqlalchemy-insert-statement)

### Merging #2843 will **not alter performance**

<sub>Comparing <code>edgarrmondragon/fix/sqlalchemy-insert-statement</code> (4b216a0) with <code>main</code> (57085dd)</sub>



### Summary

`✅ 7` untouched benchmarks  




edgarrmondragon: @sourcery-ai review

Patch:
diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py
index 1b0f646c..5e451ec9 100644
--- a/singer_sdk/connectors/sql.py
+++ b/singer_sdk/connectors/sql.py
@@ -840,6 +840,10 @@ class SQLConnector:  # noqa: PLR0904
                 pool_pre_ping=True,
             )
 
+    @deprecated(
+        "This method is deprecated. Use or override `FullyQualifiedName` instead.",
+        category=SingerSDKDeprecationWarning,
+    )
     def quote(self, name: str) -> str:
         """Quote a name if it needs quoting, using '.' as a name-part delimiter.
 
@@ -853,7 +857,7 @@ class SQLConnector:  # noqa: PLR0904
         Returns:
             str: The quoted name.
         """
-        return ".".join(
+        return ".".join(  # pragma: no cover
             [
                 self._dialect.identifier_preparer.quote(name_part)
                 for name_part in name.split(".")
diff --git a/singer_sdk/sinks/sql.py b/singer_sdk/sinks/sql.py
index 5bb64afa..855ee351 100644
--- a/singer_sdk/sinks/sql.py
+++ b/singer_sdk/sinks/sql.py
@@ -4,12 +4,12 @@ from __future__ import annotations
 
 import re
 import typing as t
+import warnings
 from collections import defaultdict
 from copy import copy
-from textwrap import dedent
 
 import sqlalchemy as sa
-from sqlalchemy.sql import quoted_name
+from sqlalchemy.sql import insert
 from sqlalchemy.sql.expression import bindparam
 
 from singer_sdk.connectors import SQLConnector
@@ -282,19 +282,26 @@ class SQLSink(BatchSink, t.Generic[_C]):
         Returns:
             An insert statement.
         """
-        property_names = list(self.conform_schema(schema)["properties"].keys())
-        column_identifiers = [
-            self.connector.quote(quoted_name(name, quote=True))
-            for name in property_names
-        ]
-        statement = dedent(
-            f"""\
-            INSERT INTO {full_table_name}
-            ({", ".join(column_identifiers)})
-            VALUES ({", ".join([f":{name}" for name in property_names])})
-            """,
+        conformed_schema = self.conform_schema(schema)
+        property_names = list(conformed_schema["properties"])
+
+        _, schema_name, table_name = self.connector.parse_full_table_name(
+            full_table_name
         )
-        return statement.rstrip()
+
+        table = sa.Table(
+            table_name,
+            sa.MetaData(),
+            *[
+                sa.Column(
+                    name, sa.String
+                )  # Assuming all columns are of type String for simplicity  # noqa: E501
+                for name in property_names
+            ],
+            schema=schema_name,
+        )
+
+        return insert(table)
 
     def bulk_insert_records(
         self,
@@ -321,7 +328,13 @@ class SQLSink(BatchSink, t.Generic[_C]):
             full_table_name,
             schema,
         )
-        if isinstance(insert_sql, str):
+        if isinstance(insert_sql, str):  # pragma: no cover
+            warnings.warn(
+                "Generating a SQL insert statement as a string is deprecated. "
+                "Please return an SQLAlchemy Executable object instead.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
             insert_sql = sa.text(insert_sql)
 
         conformed_records = [self.conform_record(record) for record in records]


Test patch:
diff --git a/tests/core/sinks/test_sql_sink.py b/tests/core/sinks/test_sql_sink.py
index c20be40f..d1c921ec 100644
--- a/tests/core/sinks/test_sql_sink.py
+++ b/tests/core/sinks/test_sql_sink.py
@@ -1,9 +1,9 @@
 from __future__ import annotations
 
 import typing as t
-from textwrap import dedent
 
 import pytest
+from sqlalchemy.sql import Insert
 
 from samples.sample_duckdb import DuckDBConnector
 from singer_sdk.sinks.sql import SQLSink
@@ -55,10 +55,12 @@ class TestDuckDBSink:
 
     def test_generate_insert_statement(self, sink: DuckDBSink, schema: dict):
         """Test that the insert statement is generated correctly."""
-        expected = dedent(
-            """\
-            INSERT INTO foo
-            (id, col_ts, "table")
-            VALUES (:id, :col_ts, :table)"""
+        stmt = sink.generate_insert_statement("foo", schema=schema)
+        assert isinstance(stmt, Insert)
+        assert stmt.table.name == "foo"
+        assert stmt.table.columns.keys() == ["id", "col_ts", "table"]
+
+        # Rendered SQL should look like:
+        assert str(stmt) == (
+            'INSERT INTO foo (id, col_ts, "table") VALUES (:id, :col_ts, :table)'
         )
-        assert sink.generate_insert_statement("foo", schema=schema) == expected
diff --git a/tests/samples/test_target_sqlite.py b/tests/samples/test_target_sqlite.py
index 48e3a891..d1be34a2 100644
--- a/tests/samples/test_target_sqlite.py
+++ b/tests/samples/test_target_sqlite.py
@@ -646,12 +646,7 @@ def test_record_with_missing_properties(
                 },
             },
             [],
-            dedent(
-                """\
-                INSERT INTO test_stream
-                (id, name, "table")
-                VALUES (:id, :name, :table)""",
-            ),
+            'INSERT INTO test_stream (id, name, "table") VALUES (:id, :name, :table)',
         ),
     ],
     ids=[
@@ -676,7 +671,7 @@ def test_sqlite_generate_insert_statement(
         sink.full_table_name,
         sink.schema,
     )
-    assert dml == expected_dml
+    assert str(dml) == expected_dml
 
 
 def test_hostile_to_sqlite(