diff --git a/.changes/unreleased/Fixes-20241104-104610.yaml b/.changes/unreleased/Fixes-20241104-104610.yaml new file mode 100644 index 000000000..c512d0bdd --- /dev/null +++ b/.changes/unreleased/Fixes-20241104-104610.yaml @@ -0,0 +1,7 @@ +kind: Fixes +body: 'Performance fixes for snowflake microbatch strategy: use temp view instead + of table, remove unnecessary ''using'' clause' +time: 2024-11-04T10:46:10.005317-05:00 +custom: + Author: michelleark + Issue: "1228" diff --git a/.changes/unreleased/Under the Hood-20241106-113249.yaml b/.changes/unreleased/Under the Hood-20241106-113249.yaml new file mode 100644 index 000000000..0437a8c88 --- /dev/null +++ b/.changes/unreleased/Under the Hood-20241106-113249.yaml @@ -0,0 +1,6 @@ +kind: Under the Hood +body: remove SnowflakeAdapterResponse in favor of updated AdapterResponse in base +time: 2024-11-06T11:32:49.503467-08:00 +custom: + Author: colin-rogers-dbt + Issue: "1233" diff --git a/dbt/adapters/snowflake/connections.py b/dbt/adapters/snowflake/connections.py index 10bee30f0..fc2c09c19 100644 --- a/dbt/adapters/snowflake/connections.py +++ b/dbt/adapters/snowflake/connections.py @@ -84,11 +84,6 @@ def snowflake_private_key(private_key: RSAPrivateKey) -> bytes: ) -@dataclass -class SnowflakeAdapterResponse(AdapterResponse): - query_id: str = "" - - @dataclass class SnowflakeCredentials(Credentials): account: str @@ -447,17 +442,17 @@ def cancel(self, connection): logger.debug("Cancel query '{}': {}".format(connection_name, res)) @classmethod - def get_response(cls, cursor) -> SnowflakeAdapterResponse: + def get_response(cls, cursor) -> AdapterResponse: code = cursor.sqlstate if code is None: code = "SUCCESS" - - return SnowflakeAdapterResponse( + query_id = str(cursor.sfqid) if cursor.sfqid is not None else None + return AdapterResponse( _message="{} {}".format(code, cursor.rowcount), rows_affected=cursor.rowcount, code=code, - query_id=cursor.sfqid, + query_id=query_id, ) # disable transactional logic by default on Snowflake diff --git a/dbt/include/snowflake/macros/materializations/incremental.sql b/dbt/include/snowflake/macros/materializations/incremental.sql index d73525d6d..dbb79de02 100644 --- a/dbt/include/snowflake/macros/materializations/incremental.sql +++ b/dbt/include/snowflake/macros/materializations/incremental.sql @@ -20,7 +20,7 @@ The append strategy can use a view because it will run a single INSERT statement. - When unique_key is none, the delete+insert strategy can use a view beacuse a + When unique_key is none, the delete+insert and microbatch strategies can use a view beacuse a single INSERT statement is run with no DELETES as part of the statement. Otherwise, play it safe by using a temporary table. #} */ @@ -32,10 +32,10 @@ ) %} {% endif %} - {% if strategy == "delete+insert" and tmp_relation_type is not none and tmp_relation_type != "table" and unique_key is not none %} + {% if strategy in ["delete+insert", "microbatch"] and tmp_relation_type is not none and tmp_relation_type != "table" and unique_key is not none %} {% do exceptions.raise_compiler_error( "In order to maintain consistent results when `unique_key` is not none, - the `delete+insert` strategy only supports `table` for `tmp_relation_type` but " + the `" ~ strategy ~ "` strategy only supports `table` for `tmp_relation_type` but " ~ tmp_relation_type ~ " was specified." ) %} @@ -49,7 +49,7 @@ {{ return("view") }} {% elif strategy in ("default", "merge", "append") %} {{ return("view") }} - {% elif strategy == "delete+insert" and unique_key is none %} + {% elif strategy in ["delete+insert", "microbatch"] and unique_key is none %} {{ return("view") }} {% else %} {{ return("table") }} diff --git a/dbt/include/snowflake/macros/materializations/merge.sql b/dbt/include/snowflake/macros/materializations/merge.sql index 57c58afdd..c8ac8d6fd 100644 --- a/dbt/include/snowflake/macros/materializations/merge.sql +++ b/dbt/include/snowflake/macros/materializations/merge.sql @@ -66,7 +66,6 @@ {% do arg_dict.update({'incremental_predicates': incremental_predicates}) %} delete from {{ target }} DBT_INTERNAL_TARGET - using {{ source }} where ( {% for predicate in incremental_predicates %} {%- if not loop.first %}and {% endif -%} {{ predicate }} diff --git a/tests/unit/test_snowflake_adapter.py b/tests/unit/test_snowflake_adapter.py index 32e73eb45..aa580aad2 100644 --- a/tests/unit/test_snowflake_adapter.py +++ b/tests/unit/test_snowflake_adapter.py @@ -60,8 +60,10 @@ def setUp(self): self.handle = mock.MagicMock(spec=snowflake_connector.SnowflakeConnection) self.cursor = self.handle.cursor.return_value self.mock_execute = self.cursor.execute + self.mock_execute.return_value = mock.MagicMock(sfqid="42") self.patcher = mock.patch("dbt.adapters.snowflake.connections.snowflake.connector.connect") self.snowflake = self.patcher.start() + self.snowflake.connect.cursor.return_value = mock.MagicMock(sfqid="42") # Create the Manifest.state_check patcher @mock.patch("dbt.parser.manifest.ManifestLoader.build_manifest_state_check") @@ -90,7 +92,6 @@ def _mock_state_check(self): self.qh_patch = mock.patch.object(self.adapter.connections.query_header, "add") self.mock_query_header_add = self.qh_patch.start() self.mock_query_header_add.side_effect = lambda q: "/* dbt */\n{}".format(q) - self.adapter.acquire_connection() inject_adapter(self.adapter, SnowflakePlugin)