Skip to content

Commit

Permalink
Merge branch 'main' into feature/transient-dynamic-tables
Browse files Browse the repository at this point in the history
  • Loading branch information
colin-rogers-dbt authored Nov 15, 2024
2 parents 41ca020 + f6468f6 commit af122fc
Show file tree
Hide file tree
Showing 20 changed files with 282 additions and 32 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Fixes-20241104-104610.yaml
Original file line number Diff line number Diff line change
@@ -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"
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20241104-172349.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Iceberg quoting ignore fix.
time: 2024-11-04T17:23:49.706297-08:00
custom:
Author: versusfacit
Issue: "1227"
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20241016-035544.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Under the Hood
body: Add telemetry function
time: 2024-10-16T03:55:44.144174-07:00
custom:
Author: versusfacit
Issue: "301"
6 changes: 6 additions & 0 deletions .changes/unreleased/Under the Hood-20241106-113249.yaml
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 2 additions & 2 deletions .github/scripts/integration-test-matrix.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ module.exports = ({ context }) => {

if (labels.includes("test macos") || testAllLabel) {
include.push({
os: "macos-12",
os: "macos-14",
adapter,
"python-version": pythonVersion,
});
Expand Down Expand Up @@ -78,7 +78,7 @@ module.exports = ({ context }) => {
// additionally include runs for all adapters, on macos and windows,
// but only for the default python version
for (const adapter of supportedAdapters) {
for (const operatingSystem of ["windows-latest", "macos-12"]) {
for (const operatingSystem of ["windows-latest", "macos-14"]) {
include.push({
os: operatingSystem,
adapter: adapter,
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-12, windows-latest]
os: [ubuntu-latest, macos-14, windows-latest]
python-version: ['3.9', '3.10', '3.11', '3.12']
dist-type: ['whl', 'gz']

Expand Down
13 changes: 4 additions & 9 deletions dbt/adapters/snowflake/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions dbt/adapters/snowflake/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from dbt.adapters.base.impl import AdapterConfig, ConstraintSupport
from dbt.adapters.base.meta import available
from dbt.adapters.capability import CapabilityDict, CapabilitySupport, Support, Capability
from dbt.adapters.contracts.relation import RelationConfig
from dbt.adapters.sql import SQLAdapter
from dbt.adapters.sql.impl import (
LIST_SCHEMAS_MACRO_NAME,
Expand All @@ -25,6 +26,7 @@
SnowflakeRelationType,
TableFormat,
)

from dbt.adapters.snowflake import SnowflakeColumn
from dbt.adapters.snowflake import SnowflakeConnectionManager
from dbt.adapters.snowflake import SnowflakeRelation
Expand Down Expand Up @@ -261,6 +263,11 @@ def list_relations_without_caching(
# this can be collapsed once Snowflake adds is_iceberg to show objects
columns = ["database_name", "schema_name", "name", "kind", "is_dynamic"]
if self.behavior.enable_iceberg_materializations.no_warn:
# The QUOTED_IDENTIFIERS_IGNORE_CASE setting impacts column names like
# is_iceberg which is created by dbt, but it does not affect the case
# of column values in Snowflake's SHOW OBJECTS query! This
# normalization step ensures metadata queries are handled consistently.
schema_objects = schema_objects.rename(column_names={"IS_ICEBERG": "is_iceberg"})
columns.append("is_iceberg")

return [self._parse_list_relations_result(obj) for obj in schema_objects.select(columns)]
Expand Down Expand Up @@ -419,3 +426,18 @@ def valid_incremental_strategies(self):
def debug_query(self):
"""Override for DebugTask method"""
self.execute("select 1 as id")

@classmethod
def _get_adapter_specific_run_info(cls, config: RelationConfig) -> Dict[str, Any]:
table_format: Optional[str] = None
if (
config
and hasattr(config, "_extra")
and (relation_format := config._extra.get("table_format"))
):
table_format = relation_format

return {
"adapter_type": "snowflake",
"table_format": table_format,
}
3 changes: 1 addition & 2 deletions dbt/include/snowflake/macros/adapters.sql
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@
{% endmacro %}

{% macro snowflake__list_relations_without_caching(schema_relation, max_iter=10, max_results_per_iter=10000) %}

{%- set max_total_results = max_results_per_iter * max_iter -%}
{%- set sql -%}
{% if schema_relation is string %}
Expand All @@ -147,7 +146,7 @@
{# -- Gated for performance reason. If you don't want Iceberg, you shouldn't pay the
-- latency penalty. #}
{% if adapter.behavior.enable_iceberg_materializations.no_warn %}
select all_objects.*, is_iceberg as "is_iceberg"
select all_objects.*, is_iceberg
from table(result_scan(last_query_id(-1))) all_objects
left join INFORMATION_SCHEMA.tables as all_tables
on all_tables.table_name = all_objects."name"
Expand Down
8 changes: 4 additions & 4 deletions dbt/include/snowflake/macros/materializations/incremental.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#} */
Expand All @@ -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."
)
%}
Expand All @@ -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") }}
Expand Down
1 change: 0 additions & 1 deletion dbt/include/snowflake/macros/materializations/merge.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion dbt/include/snowflake/macros/relations/table/create.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{% macro snowflake__create_table_as(temporary, relation, compiled_code, language='sql') -%}

{%- if relation.is_iceberg_format and not adapter.behavior.enable_iceberg_materializations.no_warn %}
{% do exceptions.raise_compiler_error('Was unable to create model as Iceberg Table Format. Please set the `enable_iceberg_materializations` behavior flag to True in your dbt_project.yml. For more information, go to <url pending>.') %}
{% do exceptions.raise_compiler_error('Was unable to create model as Iceberg Table Format. Please set the `enable_iceberg_materializations` behavior flag to True in your dbt_project.yml. For more information, go to https://docs.getdbt.com/reference/resource-configs/snowflake-configs.') %}
{%- endif %}

{%- set materialization_prefix = relation.get_ddl_prefix_for_create(config.model.config, temporary) -%}
Expand Down
60 changes: 49 additions & 11 deletions tests/functional/adapter/list_relations_tests/test_show_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import pytest

from pathlib import Path

from dbt.adapters.factory import get_adapter_by_type
from dbt.adapters.snowflake import SnowflakeRelation

Expand Down Expand Up @@ -41,8 +43,32 @@
"""
)

_MODEL_ICEBERG = """
{{
config(
materialized = "table",
table_format="iceberg",
external_volume="s3_iceberg_snow",
)
}}
select 1
"""


class ShowObjectsBase:
@staticmethod
def list_relations_without_caching(project) -> List[SnowflakeRelation]:
my_adapter = get_adapter_by_type("snowflake")
schema = my_adapter.Relation.create(
database=project.database, schema=project.test_schema, identifier=""
)
with get_connection(my_adapter):
relations = my_adapter.list_relations_without_caching(schema)
return relations


class TestShowObjects:
class TestShowObjects(ShowObjectsBase):
views: int = 10
tables: int = 10
dynamic_tables: int = 10
Expand All @@ -66,16 +92,6 @@ def setup(self, project):
run_dbt(["seed"])
run_dbt(["run"])

@staticmethod
def list_relations_without_caching(project) -> List[SnowflakeRelation]:
my_adapter = get_adapter_by_type("snowflake")
schema = my_adapter.Relation.create(
database=project.database, schema=project.test_schema, identifier=""
)
with get_connection(my_adapter):
relations = my_adapter.list_relations_without_caching(schema)
return relations

def test_list_relations_without_caching(self, project):
relations = self.list_relations_without_caching(project)
assert len([relation for relation in relations if relation.is_view]) == self.views
Expand All @@ -87,3 +103,25 @@ def test_list_relations_without_caching(self, project):
len([relation for relation in relations if relation.is_dynamic_table])
== self.dynamic_tables
)


class TestShowIcebergObjects(ShowObjectsBase):
@pytest.fixture(scope="class")
def project_config_update(self):
return {"flags": {"enable_iceberg_materializations": True}}

@pytest.fixture(scope="class")
def models(self):
return {"my_model.sql": _MODEL_ICEBERG}

def test_quoting_ignore_flag_doesnt_break_iceberg_metadata(self, project):
"""https://github.com/dbt-labs/dbt-snowflake/issues/1227
The list relations function involves a metadata sub-query. Regardless of
QUOTED_IDENTIFIERS_IGNORE_CASE, this function will fail without proper
normalization within the encapsulating python function after the macro invocation
returns. This test verifies that normalization is working.
"""
run_dbt(["run"])

self.list_relations_without_caching(project)
Empty file.
90 changes: 90 additions & 0 deletions tests/functional/generic_test_tests/_files.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
SCHEMA__CONTROL = """
version: 2
models:
- name: colors
columns:
- name: color
data_tests:
- not_null
"""


SCHEMA__EXPLICIT_WAREHOUSE = """
version: 2
models:
- name: colors
columns:
- name: color
data_tests:
- not_null:
config:
snowflake_warehouse: DBT_TESTING_ALT
"""


SCHEMA__NOT_NULL = """
version: 2
models:
- name: facts
columns:
- name: value
data_tests:
- not_null:
config:
snowflake_warehouse: DBT_TESTING_ALT
"""


SCHEMA__RELATIONSHIPS = """
version: 2
models:
- name: facts
columns:
- name: color
data_tests:
- relationships:
to: ref('my_colors')
field: color
config:
snowflake_warehouse: DBT_TESTING_ALT
"""


SCHEMA__ACCEPTED_VALUES = """
version: 2
models:
- name: colors
columns:
- name: color
data_tests:
- accepted_values:
values: ['blue', 'red', 'green']
config:
snowflake_warehouse: DBT_TESTING_ALT
"""


SEED__COLORS = """
color
blue
green
red
yellow
""".strip()


# record 10 is missing a value
# record 7 has a color that's not on COLORS
SEED__FACTS = """
id,color,value
1,blue,10
2,red,20
3,green,30
4,yellow,40
5,blue,50
6,red,60
7,orange,70
8,green,80
9,yellow,90
10,blue,
""".strip()
Empty file.
Empty file.
Loading

0 comments on commit af122fc

Please sign in to comment.