Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override list_relations_without_caching #428

Merged
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
8f92274
add support for datashared objects and external tables for redshift l…
jiezhen-chen Apr 7, 2023
29e5a5c
changie
jiezhen-chen Apr 7, 2023
34fd237
Merge branch 'main' into fix_get_relation
jiezhen-chen Apr 7, 2023
8cf25a9
Merge branch 'main' into fix_get_relation
jiezhen-chen Apr 11, 2023
b86a826
Merge branch 'main' into fix_get_relation
dataders Apr 12, 2023
dc98d12
refactor inner joins to cte
jiezhen-chen Apr 13, 2023
7bf1978
shorten list_relations_without_caching query
jiezhen-chen Apr 13, 2023
0958f28
Merge branch 'fix_get_relation' of github.com:jiezhen-chen/dbt-redshi…
jiezhen-chen Apr 13, 2023
c69e651
add database filter to get_columns_in_relation to only query inbound …
jiezhen-chen Apr 13, 2023
bce3e88
leverage redshift_conn method for list_schemas
jiezhen-chen Apr 14, 2023
451c60d
add list_schemas method
jiezhen-chen Apr 14, 2023
4353a38
migrate from macros to redshift driver
jiezhen-chen Apr 28, 2023
b5494a6
Merge branch 'main' into redshift_list_schemas
jiezhen-chen Apr 28, 2023
4295477
merge with main
jiezhen-chen Apr 29, 2023
d408c06
add database_metadata_current_db_only to unit tests
jiezhen-chen Apr 29, 2023
404401b
override get_columns_in_relation
jiezhen-chen May 2, 2023
e127e38
cast relation types to lower case
jiezhen-chen May 2, 2023
c64fe20
add functional test to ensure list_relations_without_chacing works
jiezhen-chen May 4, 2023
4402fb1
revert changes in macros adapter.sql
jiezhen-chen May 4, 2023
62c3c54
revert changes in macros adapter.sql
jiezhen-chen May 4, 2023
462d58a
revert changes in macros adapter.sql
jiezhen-chen May 4, 2023
d5c7794
Merge branch 'main' into override_list_relations_without_caching
jiezhen-chen May 5, 2023
48c7584
update test_adapter_methods with project test
jiezhen-chen May 5, 2023
0cd00ac
Merge branch 'dbt-labs:main' into override_list_relations_without_cac…
jiezhen-chen May 15, 2023
3ccc47a
Merge branch 'main' into override_list_relations_without_caching
jiezhen-chen May 15, 2023
2748550
Merge branch 'dbt-labs:main' into override_list_relations_without_cac…
jiezhen-chen May 23, 2023
b6bee40
Merge branch 'main' into override_list_relations_without_caching
jiezhen-chen May 24, 2023
a171443
Merge branch 'main' into override_list_relations_without_caching
jiezhen-chen Jun 13, 2023
9e4955f
remove snowflake comment in test_adapter_methods
jiezhen-chen Jun 13, 2023
85b20bd
unpack driver metadata results with dictionary
jiezhen-chen Jun 13, 2023
3e5790e
refactor parse_relation_results to its own method
jiezhen-chen Jun 13, 2023
6b4ff5a
replace dict[] with Dict[]
jiezhen-chen Jun 15, 2023
1bfe915
Merge branch 'main' into override_list_relations_without_caching
jiezhen-chen Jun 15, 2023
2c861b9
correct unit tests and functional tests
jiezhen-chen Jun 15, 2023
292bdde
remove irrelevant comment
jiezhen-chen Jun 15, 2023
4be74af
Merge branch 'main' into override_list_relations_without_caching
colin-rogers-dbt Jun 15, 2023
81b47e3
Merge branch 'main' into override_list_relations_without_caching
colin-rogers-dbt Jun 20, 2023
0ca4d35
set current_db_only to default true
jiezhen-chen Jun 21, 2023
1de8fcb
Merge branch 'main' into override_list_relations_without_caching
jiezhen-chen Jun 28, 2023
9539df3
remove unused import
jiezhen-chen Jun 28, 2023
14dfea2
Merge branch 'main' into override_list_relations_without_caching
colin-rogers-dbt Jul 7, 2023
764e481
Merge branch 'main' into override_list_relations_without_caching
dataders Jul 10, 2023
ced9cd1
remove overriding list_relations_without_caching and rewrite the sql …
jiezhen-chen Jul 12, 2023
2175a04
Merge branch 'override_list_relations_without_caching' of github.com:…
jiezhen-chen Jul 12, 2023
4e3ed0d
update unit tests
jiezhen-chen Jul 12, 2023
781dfe3
remove overriding list_relations_without_caching and rewrite the sql …
jiezhen-chen Jul 12, 2023
7b2f375
remove unused tests
jiezhen-chen Jul 12, 2023
0ea7496
add schema filter
jiezhen-chen Jul 12, 2023
bb80ac6
Merge branch 'main' into override_list_relations_without_caching
colin-rogers-dbt Jul 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/unreleased/Features-20230407-104723.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: Features
time: 2023-04-07T10:47:23.105369-07:00
custom:
Author: jiezhen-chen
Issue: 17 179 217
1 change: 1 addition & 0 deletions dbt/adapters/redshift/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from dbt.adapters.redshift.connections import RedshiftConnectionManager # noqa
from dbt.adapters.redshift.connections import RedshiftCredentials

from dbt.adapters.redshift.column import RedshiftColumn # noqa
from dbt.adapters.redshift.relation import RedshiftRelation # noqa: F401
from dbt.adapters.redshift.impl import RedshiftAdapter
Expand Down
5 changes: 5 additions & 0 deletions dbt/adapters/redshift/connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
from dbt.dataclass_schema import FieldEncoder, dbtClassMixin, StrEnum
from dbt.helper_types import Port


logger = AdapterLogger("Redshift")


drop_lock: Lock = dbt.flags.MP_CONTEXT.Lock() # type: ignore


IAMDuration = NewType("IAMDuration", int)


Expand Down Expand Up @@ -77,6 +80,7 @@ class RedshiftCredentials(Credentials):
sslmode: Optional[str] = None
retries: int = 1
region: Optional[str] = None # if not provided, will be determined from host
current_db_only: Optional[bool] = False
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
autocommit: Optional[bool] = False

_ALIASES = {"dbname": "database", "pass": "password"}
Expand Down Expand Up @@ -127,6 +131,7 @@ def get_connect_method(self):
"db_groups": self.credentials.db_groups,
"region": self.credentials.region,
"timeout": self.credentials.connect_timeout,
"database_metadata_current_db_only": self.credentials.current_db_only,
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
}
if kwargs["region"] is None:
logger.debug("No region provided, attempting to determine from host.")
Expand Down
45 changes: 42 additions & 3 deletions dbt/adapters/redshift/impl.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
from dataclasses import dataclass
from typing import Optional, Set, Any, Dict, Type
from typing import Optional, Set, Any, Dict, Type, List
from collections import namedtuple

from dbt.adapters.base import PythonJobHelper
from dbt.adapters.base.impl import AdapterConfig, ConstraintSupport
from dbt.adapters.sql import SQLAdapter
from dbt.adapters.base.meta import available
from dbt.contracts.connection import AdapterResponse
from dbt.contracts.graph.nodes import ConstraintType
from dbt.events import AdapterLogger
from dbt.adapters.base.relation import BaseRelation


import dbt.exceptions

Expand All @@ -30,7 +31,7 @@ class RedshiftConfig(AdapterConfig):


class RedshiftAdapter(SQLAdapter):
Relation = RedshiftRelation
Relation = RedshiftRelation # type: ignore
ConnectionManager = RedshiftConnectionManager
connections: RedshiftConnectionManager
Column = RedshiftColumn # type: ignore
Expand Down Expand Up @@ -114,6 +115,44 @@ def valid_incremental_strategies(self):
def timestamp_add_sql(self, add_to: str, number: int = 1, interval: str = "hour") -> str:
return f"{add_to} + interval '{number} {interval}'"

def _get_cursor(self):
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
return self.connections.get_thread_connection().handle.cursor()

def _get_tables(self, database: Optional[str], schema: Optional[str]):
cursor = self._get_cursor()
results = []
for table in cursor.get_tables(
catalog=database,
schema_pattern=schema,
table_name_pattern=None,
types=["VIEW", "TABLE"],
):
results.append([table[0], table[1], table[2], table[3]])
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
# kinda feel like it might not be a good thing to do this by index
return results

def list_relations_without_caching( # type: ignore
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
self, schema_relation: BaseRelation
) -> List[RedshiftRelation]:
results = self._get_tables(schema_relation.database, schema_relation.schema)
relations = []
quote_policy = {"database": True, "schema": True, "identifier": True}
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
for _database, _schema, name, _type in results:
try:
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
_type = self.Relation.get_relation_type(_type.lower())
except ValueError:
_type = self.Relation.External
relations.append(
self.Relation.create(
database=_database,
schema=_schema,
identifier=name,
quote_policy=quote_policy,
type=_type,
)
)
return relations

def _link_cached_database_relations(self, schemas: Set[str]):
"""
:param schemas: The set of schemas that should have links added.
Expand Down
5 changes: 0 additions & 5 deletions dbt/include/redshift/macros/adapters.sql
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,6 @@
{% endmacro %}


{% macro redshift__list_relations_without_caching(schema_relation) %}
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
{{ return(postgres__list_relations_without_caching(schema_relation)) }}
{% endmacro %}


{% macro redshift__information_schema_name(database) -%}
{{ return(postgres__information_schema_name(database)) }}
{%- endmacro %}
Expand Down
125 changes: 125 additions & 0 deletions tests/functional/adapter/test_adapter_methods.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import pytest

from dbt.tests.util import run_dbt, check_relations_equal
from dbt.tests.fixtures.project import write_project_files


tests__get_relation_invalid = """
{% set upstream = ref('upstream') %}
{% set relations = adapter.get_relation(database=upstream.database, schema=upstream.schema, identifier="doesnotexist") %}
{% set limit_query = 0 %}
{% if relations.identifier %}
{% set limit_query = 1 %}
{% endif %}

select 1 as id limit {{ limit_query }}

"""

models__upstream_sql = """
select 1 as id

"""

models__expected_sql = """
select 1 as valid_relation

"""

models__model_sql = """

{% set upstream = ref('upstream') %}

select * from {{ upstream }}

"""

models__call_get_relation = """

{% set model = ref('model') %}

{% set relation = adapter.get_relation(database=model.database, schema=model.schema, identifier=model.identifier) %}
{% if relation.identifier == model.identifier %}

select 1 as valid_relation

{% else %}

select 0 as valid_relation

{% endif %}

"""

models__get_relation_type = """

{% set base_view = ref('base_view') %}

{% set relation = adapter.get_relation(database=base_view.database, schema=base_view.schema, identifier=base_view.identifier) %}
{% if relation.type == 'view' %}

select 1 as valid_type

{% else %}

select 0 as valid_type

{% endif %}

"""


class RedshiftAdapterMethod:
@pytest.fixture(scope="class")
def tests(self):
return {"get_relation_invalid.sql": tests__get_relation_invalid}

@pytest.fixture(scope="class")
def models(self):
return {
"upstream.sql": models__upstream_sql,
"expected.sql": models__expected_sql,
"model.sql": models__model_sql,
"call_get_relation.sql": models__call_get_relation,
"base_view.sql": "{{ config(bind=True) }} select * from {{ ref('model') }}",
"get_relation_type.sql": models__get_relation_type,
"expected_type.sql": "select 1 as valid_type",
}

@pytest.fixture(scope="class")
def project_files(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this do anything differently than the tests and models fixture? I was under the impression that that's what project does as long as you call it in a fixture.

self,
project_root,
tests,
models,
):
write_project_files(project_root, "tests", tests)
write_project_files(project_root, "models", models)

@pytest.fixture(scope="class")
def project_config_update(self):
return {
"name": "adapter_methods",
}

# snowflake need all tables in CAP name
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
@pytest.fixture(scope="class")
def equal_tables(self):
return ["call_get_relation", "expected"]

@pytest.fixture(scope="class")
def equal_types(self):
jiezhen-chen marked this conversation as resolved.
Show resolved Hide resolved
return ["get_relation_type", "expected_type"]

def test_adapter_methods(self, project, equal_tables):
run_dbt(["compile"]) # trigger any compile-time issues
result = run_dbt()
assert len(result) == 7

run_dbt(["test"])
check_relations_equal(project.adapter, equal_tables)
check_relations_equal(project.adapter, ["get_relation_type", "expected_type"])


class TestRedshiftAdapterMethod(RedshiftAdapterMethod):
pass
38 changes: 38 additions & 0 deletions tests/unit/test_redshift_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def test_implicit_database_conn(self):
db_groups=[],
timeout=30,
region="us-east-1",
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand All @@ -92,6 +93,7 @@ def test_explicit_database_conn(self):
db_groups=[],
region="us-east-1",
timeout=30,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand All @@ -117,6 +119,7 @@ def test_explicit_iam_conn_without_profile(self):
profile=None,
timeout=30,
port=5439,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand Down Expand Up @@ -145,6 +148,7 @@ def test_explicit_iam_conn_with_profile(self):
profile="test",
timeout=30,
port=5439,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand All @@ -171,6 +175,7 @@ def test_explicit_iam_serverless_with_profile(self):
profile="test",
timeout=30,
port=5439,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand Down Expand Up @@ -199,6 +204,7 @@ def test_explicit_region(self):
profile="test",
timeout=30,
port=5439,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand Down Expand Up @@ -228,6 +234,7 @@ def test_explicit_region_failure(self):
profile="test",
timeout=30,
port=5439,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand Down Expand Up @@ -257,6 +264,7 @@ def test_explicit_invalid_region(self):
profile="test",
timeout=30,
port=5439,
database_metadata_current_db_only=False,
)

@mock.patch("redshift_connector.connect", Mock())
Expand Down Expand Up @@ -284,6 +292,7 @@ def test_serverless_iam_failure(self):
profile="test",
port=5439,
timeout=30,
database_metadata_current_db_only=False,
)
self.assertTrue("'host' must be provided" in context.exception.msg)

Expand Down Expand Up @@ -450,6 +459,35 @@ def test_add_query_success(self):
"select * from test3", True, bindings=None, abridge_sql_log=False
)

@mock.patch.object(
dbt.adapters.redshift.connections.SQLConnectionManager, "get_thread_connection"
)
def mock_cursor(self, mock_get_thread_conn):
conn = mock.MagicMock
mock_get_thread_conn.return_value = conn
mock_handle = mock.MagicMock
conn.return_value = mock_handle
mock_cursor = mock.MagicMock
mock_handle.return_value = mock_cursor
return mock_cursor

@mock.patch("dbt.adapters.redshift.impl.RedshiftAdapter._get_cursor")
def test_get_tables(self, mock_cursor):
mock_cursor.return_value.get_tables.return_value = [
("apple", "banana", "cherry", "orange")
]
results = self.adapter._get_tables(database="somedb", schema="someschema")
self.assertTrue(results == [["apple", "banana", "cherry", "orange"]])

@mock.patch("dbt.adapters.redshift.impl.RedshiftAdapter._get_tables")
def test_list_relations_without_caching(self, mock_get_tables):
mock_get_tables.return_value = [["somedb", "someschema", "sometb", "VIEW"]]
mock_schema = mock.MagicMock(database="somedb", schema="someschema")
results = self.adapter.list_relations_without_caching(mock_schema)
self.assertTrue(results[0].database == "somedb")
self.assertTrue(results[0].schema == "someschema")
self.assertTrue(results[0].identifier == "sometb")


class TestRedshiftAdapterConversions(TestAdapterConversions):
def test_convert_text_type(self):
Expand Down