From 28e58624a5bc8e3e1890da8a8f4dc1f3034ac30c Mon Sep 17 00:00:00 2001 From: Luciano Vitti Date: Fri, 7 Jun 2024 09:46:26 -0300 Subject: [PATCH 1/6] fix config change detection not working for multiple sortkey --- .../relation_configs/materialized_view.py | 15 ++- .../redshift/relation_configs/sort.py | 28 +++-- .../relations/materialized_view/describe.sql | 22 +++- tests/unit/test_materialized_view.py | 56 ++++++++++ tests/unit/test_relation.py | 104 +++++++++++++++++- 5 files changed, 208 insertions(+), 17 deletions(-) diff --git a/dbt/adapters/redshift/relation_configs/materialized_view.py b/dbt/adapters/redshift/relation_configs/materialized_view.py index f6d93754e..0743fbf37 100644 --- a/dbt/adapters/redshift/relation_configs/materialized_view.py +++ b/dbt/adapters/redshift/relation_configs/materialized_view.py @@ -167,6 +167,15 @@ def parse_relation_results(cls, relation_results: RelationResults) -> Dict: "query": agate.Table( agate.Row({"definition": "")} ), + "column_descriptor": agate.Table( + agate.Row({ + "schema": "", + "table": "", + "column": "", + "is_sort_key": any(true, false), + "is_dist_key: any(true, false), + }) + ), } Additional columns in either value is fine, as long as `sortkey` and `sortstyle` are available. @@ -197,10 +206,10 @@ def parse_relation_results(cls, relation_results: RelationResults) -> Dict: {"dist": RedshiftDistConfig.parse_relation_results(materialized_view)} ) - # TODO: this only shows the first column in the sort key - if materialized_view.get("sortkey1"): + column_descriptor = relation_results.get("column_descriptor") + if column_descriptor and len(column_descriptor.rows) > 0: config_dict.update( - {"sort": RedshiftSortConfig.parse_relation_results(materialized_view)} + {"sort": RedshiftSortConfig.parse_relation_results(column_descriptor.rows)} ) return config_dict diff --git a/dbt/adapters/redshift/relation_configs/sort.py b/dbt/adapters/redshift/relation_configs/sort.py index 91152615e..ac3582670 100644 --- a/dbt/adapters/redshift/relation_configs/sort.py +++ b/dbt/adapters/redshift/relation_configs/sort.py @@ -2,7 +2,7 @@ from dbt.adapters.contracts.relation import RelationConfig from typing import Optional, FrozenSet, Set, Dict, Any -import agate +from agate import MappedSequence from dbt.adapters.relation_configs import ( RelationConfigChange, RelationConfigChangeAction, @@ -136,7 +136,7 @@ def parse_relation_config(cls, relation_config: RelationConfig) -> Dict[str, Any return config_dict @classmethod - def parse_relation_results(cls, relation_results_entry: agate.Row) -> dict: + def parse_relation_results(cls, relation_results_entry: MappedSequence) -> dict: """ Translate agate objects from the database into a standard dictionary. @@ -145,20 +145,24 @@ def parse_relation_results(cls, relation_results_entry: agate.Row) -> dict: Processing of `sortstyle` has been omitted here, which means it's the default (compound). Args: - relation_results_entry: the description of the sortkey and sortstyle from the database in this format: - - agate.Row({ - ..., - "sortkey1": "", - ... - }) + relation_results_entry: The list of rows that contains the sortkey in this format: + [ + agate.Row({ + ..., + "column": "", + "is_sort_key": True, + ... + }), + ] Returns: a standard dictionary describing this `RedshiftSortConfig` instance """ - if sortkey := relation_results_entry.get("sortkey1"): - return {"sortkey": {sortkey}} - return {} + sort_config = [] + for column in relation_results_entry: + if column.get("is_sort_key"): + sort_config.append(column.get("column")) + return {"sortkey": sort_config} @dataclass(frozen=True, eq=True, unsafe_hash=True) class RedshiftSortConfigChange(RelationConfigChange, RelationConfigValidationMixin): diff --git a/dbt/include/redshift/macros/relations/materialized_view/describe.sql b/dbt/include/redshift/macros/relations/materialized_view/describe.sql index a56d0756e..77dd0e708 100644 --- a/dbt/include/redshift/macros/relations/materialized_view/describe.sql +++ b/dbt/include/redshift/macros/relations/materialized_view/describe.sql @@ -24,6 +24,22 @@ {%- endset %} {% set _materialized_view = run_query(_materialized_view_sql) %} + {%- set _column_descriptor_sql -%} + SELECT + n.nspname AS schema, + c.relname AS table, + a.attname as column, + a.attisdistkey as is_dist_key, + a.attsortkeyord > 0 as is_sort_key + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + JOIN pg_attribute a ON a.attrelid = c.oid + WHERE + n.nspname ilike '{{ relation.schema }}' + AND c.relname LIKE 'mv_tbl__{{ relation.identifier }}__%' + {%- endset %} + {% set _column_descriptor = run_query(_column_descriptor_sql) %} + {%- set _query_sql -%} select vw.definition @@ -34,6 +50,10 @@ {%- endset %} {% set _query = run_query(_query_sql) %} - {% do return({'materialized_view': _materialized_view, 'query': _query}) %} + {% do return({ + 'materialized_view': _materialized_view, + 'query': _query, + 'column_descriptor': _column_descriptor, + })%} {% endmacro %} diff --git a/tests/unit/test_materialized_view.py b/tests/unit/test_materialized_view.py index 8e4f6ca3e..fb077d412 100644 --- a/tests/unit/test_materialized_view.py +++ b/tests/unit/test_materialized_view.py @@ -1,5 +1,6 @@ from unittest.mock import Mock +import agate import pytest from dbt.adapters.redshift.relation_configs import RedshiftMaterializedViewConfig @@ -53,3 +54,58 @@ def test_redshift_materialized_view_config_throws_expected_exception_with_invali ) with pytest.raises(ValueError): config.parse_relation_config(model_node) + + +def test_redshift_materialized_view_parse_relation_results_handles_multiples_sort_key(): + materialized_view = agate.Table.from_object( + [], + [ + "database", + "schema", + "table", + "diststyle", + "sortkey1", + "autorefresh", + ], + ) + + column_descriptor = agate.Table.from_object( + [ + { + "schema": "my_schema", + "table": "my_table", + "column": "my_column", + "is_dist_key": True, + "is_sort_key": True, + }, + { + "schema": "my_schema", + "table": "my_table", + "column": "my_column2", + "is_dist_key": True, + "is_sort_key": True, + }, + ], + ) + + query = agate.Table.from_object( + [ + { + "definition": "create materialized view my_view as (select 1 as my_column, 'value' as my_column2)" + } + ] + ) + + relation_results = { + "materialized_view": materialized_view, + "column_descriptor": column_descriptor, + "query": query, + } + + config_dict = RedshiftMaterializedViewConfig.parse_relation_results( + relation_results + ) + + assert isinstance(config_dict["sort"], dict) + assert config_dict["sort"]["sortkey"][0] == "my_column" + assert config_dict["sort"]["sortkey"][1] == "my_column2" diff --git a/tests/unit/test_relation.py b/tests/unit/test_relation.py index 4817ab100..d6c4f483c 100644 --- a/tests/unit/test_relation.py +++ b/tests/unit/test_relation.py @@ -1,5 +1,15 @@ +from unittest.mock import Mock + +import agate +import pytest + from dbt.adapters.redshift.relation import RedshiftRelation -from dbt.adapters.contracts.relation import RelationType +from dbt.adapters.contracts.relation import ( + RelationType, + RelationConfig, +) + +from dbt.adapters.redshift.relation_configs.sort import RedshiftSortStyle def test_renameable_relation(): @@ -15,3 +25,95 @@ def test_renameable_relation(): RelationType.Table, } ) + + +@pytest.fixture +def materialized_view_multiple_sort_key_from_db(): + materialized_view = agate.Table.from_object( + [ + { + "database": "my_db", + "schema": "my_schema", + "table": "my_table", + } + ], + ) + + column_descriptor = agate.Table.from_object( + [ + { + "schema": "my_schema", + "table": "my_table", + "column": "my_column", + "is_dist_key": True, + "is_sort_key": True, + }, + { + "schema": "my_schema", + "table": "my_table", + "column": "my_column2", + "is_dist_key": True, + "is_sort_key": True, + }, + ], + ) + + query = agate.Table.from_object( + [ + { + "definition": "create materialized view my_view as (select 1 as my_column, 'value' as my_column2)" + } + ] + ) + + relation_results = { + "materialized_view": materialized_view, + "column_descriptor": column_descriptor, + "query": query, + } + return relation_results + + +@pytest.fixture +def materialized_view_multiple_sort_key_config(): + relation_config = Mock(spec=RelationConfig) + + relation_config.database = "my_db" + relation_config.identifier = "my_table" + relation_config.schema = "my_schema" + relation_config.config = Mock() + relation_config.config.extra = { + "sort_type": RedshiftSortStyle.compound, + "sort": ["my_column", "my_column2"], + } + relation_config.config.sort = "" + relation_config.compiled_code = "create materialized view my_view as (select 1 as my_column, 'value' as my_column2)" + return relation_config + + +def test_materialized_view_config_changeset_multiple_sort_key_without_changes( + materialized_view_multiple_sort_key_from_db, + materialized_view_multiple_sort_key_config, +): + + change_set = RedshiftRelation.materialized_view_config_changeset( + materialized_view_multiple_sort_key_from_db, + materialized_view_multiple_sort_key_config, + ) + + assert change_set is None + + +def test_materialized_view_config_changeset_multiple_sort_key_with_changes( + materialized_view_multiple_sort_key_from_db, + materialized_view_multiple_sort_key_config, +): + materialized_view_multiple_sort_key_config.config.extra["sort"].append("my_column3") + + change_set = RedshiftRelation.materialized_view_config_changeset( + materialized_view_multiple_sort_key_from_db, + materialized_view_multiple_sort_key_config, + ) + + assert change_set is not None + assert change_set.sort.context.sortkey == frozenset({"my_column", "my_column2", "my_column3"}) From 48f6229e2235032e1a765dc4258c281347e7ac13 Mon Sep 17 00:00:00 2001 From: Luciano Vitti Date: Fri, 7 Jun 2024 13:08:04 -0300 Subject: [PATCH 2/6] add test --- .../relation_configs/materialized_view.py | 10 +-- tests/unit/test_relation.py | 71 ++++++++++++------- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/dbt/adapters/redshift/relation_configs/materialized_view.py b/dbt/adapters/redshift/relation_configs/materialized_view.py index 0743fbf37..1c418159c 100644 --- a/dbt/adapters/redshift/relation_configs/materialized_view.py +++ b/dbt/adapters/redshift/relation_configs/materialized_view.py @@ -207,10 +207,12 @@ def parse_relation_results(cls, relation_results: RelationResults) -> Dict: ) column_descriptor = relation_results.get("column_descriptor") - if column_descriptor and len(column_descriptor.rows) > 0: - config_dict.update( - {"sort": RedshiftSortConfig.parse_relation_results(column_descriptor.rows)} - ) + if column_descriptor: + sort_columns = [row for row in column_descriptor.rows if row.get("is_sort_key")] + if sort_columns: + config_dict.update( + {"sort": RedshiftSortConfig.parse_relation_results(sort_columns)} + ) return config_dict diff --git a/tests/unit/test_relation.py b/tests/unit/test_relation.py index d6c4f483c..f2d8a5373 100644 --- a/tests/unit/test_relation.py +++ b/tests/unit/test_relation.py @@ -28,7 +28,7 @@ def test_renameable_relation(): @pytest.fixture -def materialized_view_multiple_sort_key_from_db(): +def materialized_view_without_sort_key_from_db(): materialized_view = agate.Table.from_object( [ { @@ -39,24 +39,7 @@ def materialized_view_multiple_sort_key_from_db(): ], ) - column_descriptor = agate.Table.from_object( - [ - { - "schema": "my_schema", - "table": "my_table", - "column": "my_column", - "is_dist_key": True, - "is_sort_key": True, - }, - { - "schema": "my_schema", - "table": "my_table", - "column": "my_column2", - "is_dist_key": True, - "is_sort_key": True, - }, - ], - ) + column_descriptor = agate.Table.from_object([]) query = agate.Table.from_object( [ @@ -75,22 +58,62 @@ def materialized_view_multiple_sort_key_from_db(): @pytest.fixture -def materialized_view_multiple_sort_key_config(): +def materialized_view_without_sort_key_config(): relation_config = Mock(spec=RelationConfig) relation_config.database = "my_db" relation_config.identifier = "my_table" relation_config.schema = "my_schema" relation_config.config = Mock() - relation_config.config.extra = { - "sort_type": RedshiftSortStyle.compound, - "sort": ["my_column", "my_column2"], - } + relation_config.config.extra = {} relation_config.config.sort = "" relation_config.compiled_code = "create materialized view my_view as (select 1 as my_column, 'value' as my_column2)" return relation_config +@pytest.fixture +def materialized_view_multiple_sort_key_from_db(materialized_view_without_sort_key_from_db): + materialized_view_without_sort_key_from_db["column_descriptor"] = agate.Table.from_object( + [ + { + "schema": "my_schema", + "table": "my_table", + "column": "my_column", + "is_dist_key": True, + "is_sort_key": True, + }, + { + "schema": "my_schema", + "table": "my_table", + "column": "my_column2", + "is_dist_key": True, + "is_sort_key": True, + }, + ], + ) + return materialized_view_without_sort_key_from_db + + +@pytest.fixture +def materialized_view_multiple_sort_key_config(materialized_view_without_sort_key_config): + materialized_view_without_sort_key_config.config.extra = { + "sort_type": RedshiftSortStyle.compound, + "sort": ["my_column", "my_column2"], + } + + return materialized_view_without_sort_key_config + +def test_materialized_view_config_changeset_without_sort_key_empty_changes( + materialized_view_without_sort_key_from_db, + materialized_view_without_sort_key_config, +): + change_set = RedshiftRelation.materialized_view_config_changeset( + materialized_view_without_sort_key_from_db, + materialized_view_without_sort_key_config, + ) + + assert change_set is None + def test_materialized_view_config_changeset_multiple_sort_key_without_changes( materialized_view_multiple_sort_key_from_db, materialized_view_multiple_sort_key_config, From 2c470bd8cb42804716c6234071c29fbf0bdaf0d2 Mon Sep 17 00:00:00 2001 From: Luciano Vitti Date: Mon, 10 Jun 2024 10:30:24 -0300 Subject: [PATCH 3/6] Signed CLA From 862ec394bcc219852977f9faa2f00876a84a9b3a Mon Sep 17 00:00:00 2001 From: Luciano Vitti Date: Mon, 10 Jun 2024 10:41:55 -0300 Subject: [PATCH 4/6] changelog --- .changes/unreleased/Fixes-20240610-104114.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20240610-104114.yaml diff --git a/.changes/unreleased/Fixes-20240610-104114.yaml b/.changes/unreleased/Fixes-20240610-104114.yaml new file mode 100644 index 000000000..d220288f8 --- /dev/null +++ b/.changes/unreleased/Fixes-20240610-104114.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Fix config change detection not working for multiple sortkey in materialized views +time: 2024-06-10T10:41:14.721411-03:00 +custom: + Author: lvitti + Issue: "838" From 4c42f9915ace551f67d5efcff549377077e17725 Mon Sep 17 00:00:00 2001 From: Luciano Vitti Date: Fri, 9 Aug 2024 16:22:06 -0300 Subject: [PATCH 5/6] Requested changes in the PR --- .../relation_configs/materialized_view.py | 11 +++--- .../redshift/relation_configs/sort.py | 20 ++++++----- .../relations/materialized_view/describe.sql | 6 ++-- tests/unit/test_materialized_view.py | 14 +++----- tests/unit/test_relation.py | 35 +++++++++++++------ 5 files changed, 46 insertions(+), 40 deletions(-) diff --git a/dbt/adapters/redshift/relation_configs/materialized_view.py b/dbt/adapters/redshift/relation_configs/materialized_view.py index 286b02dbf..e65a7cf1c 100644 --- a/dbt/adapters/redshift/relation_configs/materialized_view.py +++ b/dbt/adapters/redshift/relation_configs/materialized_view.py @@ -169,12 +169,10 @@ def parse_relation_results(cls, relation_results: RelationResults) -> Dict: "query": agate.Table( agate.Row({"definition": "")} ), - "column_descriptor": agate.Table( + "columns": agate.Table( agate.Row({ - "schema": "", - "table": "", "column": "", - "is_sort_key": any(true, false), + "sort_key_position": , "is_dist_key: any(true, false), }) ), @@ -208,9 +206,8 @@ def parse_relation_results(cls, relation_results: RelationResults) -> Dict: {"dist": RedshiftDistConfig.parse_relation_results(materialized_view)} ) - column_descriptor = relation_results.get("column_descriptor") - if column_descriptor: - sort_columns = [row for row in column_descriptor.rows if row.get("is_sort_key")] + if columns:= relation_results.get("columns"): + sort_columns = [row for row in columns.rows if row.get("sort_key_position") > 0] if sort_columns: config_dict.update( {"sort": RedshiftSortConfig.parse_relation_results(sort_columns)} diff --git a/dbt/adapters/redshift/relation_configs/sort.py b/dbt/adapters/redshift/relation_configs/sort.py index 8e986bf82..5bdb4c936 100644 --- a/dbt/adapters/redshift/relation_configs/sort.py +++ b/dbt/adapters/redshift/relation_configs/sort.py @@ -1,8 +1,7 @@ from dataclasses import dataclass from dbt.adapters.contracts.relation import RelationConfig -from typing import Optional, FrozenSet, Set, Dict, Any, TYPE_CHECKING +from typing import Optional, Set, Dict, Any, TYPE_CHECKING, Tuple -from agate import MappedSequence from dbt.adapters.relation_configs import ( RelationConfigChange, RelationConfigChangeAction, @@ -47,7 +46,7 @@ class RedshiftSortConfig(RedshiftRelationConfigBase, RelationConfigValidationMix """ sortstyle: Optional[RedshiftSortStyle] = None - sortkey: Optional[FrozenSet[str]] = None + sortkey: Optional[Tuple[str]] = None def __post_init__(self): # maintains `frozen=True` while allowing for a variable default on `sort_type` @@ -104,7 +103,7 @@ def validation_rules(self) -> Set[RelationConfigValidationRule]: def from_dict(cls, config_dict) -> Self: kwargs_dict = { "sortstyle": config_dict.get("sortstyle"), - "sortkey": frozenset(column for column in config_dict.get("sortkey", {})), + "sortkey": tuple(column for column in config_dict.get("sortkey", {})), } sort: Self = super().from_dict(kwargs_dict) # type: ignore return sort # type: ignore @@ -134,12 +133,13 @@ def parse_relation_config(cls, relation_config: RelationConfig) -> Dict[str, Any if isinstance(sortkey, str): sortkey = [sortkey] - config_dict.update({"sortkey": set(sortkey)}) + config_dict.update({"sortkey": tuple(sortkey)}) return config_dict + @classmethod - def parse_relation_results(cls, relation_results_entry: MappedSequence) -> dict: + def parse_relation_results(cls, relation_results_entry: "agate.MappedSequence") -> dict: """ Translate agate objects from the database into a standard dictionary. @@ -153,7 +153,7 @@ def parse_relation_results(cls, relation_results_entry: MappedSequence) -> dict: agate.Row({ ..., "column": "", - "is_sort_key": True, + "sort_key_position": , ... }), ] @@ -161,8 +161,10 @@ def parse_relation_results(cls, relation_results_entry: MappedSequence) -> dict: Returns: a standard dictionary describing this `RedshiftSortConfig` instance """ sort_config = [] - for column in relation_results_entry: - if column.get("is_sort_key"): + + sorted_columns = sorted(relation_results_entry, key=lambda x: x["sort_key_position"]) + for column in sorted_columns: + if column.get("sort_key_position"): sort_config.append(column.get("column")) return {"sortkey": sort_config} diff --git a/dbt/include/redshift/macros/relations/materialized_view/describe.sql b/dbt/include/redshift/macros/relations/materialized_view/describe.sql index 77dd0e708..3f038b9ad 100644 --- a/dbt/include/redshift/macros/relations/materialized_view/describe.sql +++ b/dbt/include/redshift/macros/relations/materialized_view/describe.sql @@ -26,11 +26,9 @@ {%- set _column_descriptor_sql -%} SELECT - n.nspname AS schema, - c.relname AS table, a.attname as column, a.attisdistkey as is_dist_key, - a.attsortkeyord > 0 as is_sort_key + a.attsortkeyord as sort_key_position FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace JOIN pg_attribute a ON a.attrelid = c.oid @@ -53,7 +51,7 @@ {% do return({ 'materialized_view': _materialized_view, 'query': _query, - 'column_descriptor': _column_descriptor, + 'columns': _column_descriptor, })%} {% endmacro %} diff --git a/tests/unit/test_materialized_view.py b/tests/unit/test_materialized_view.py index fb077d412..13d4341df 100644 --- a/tests/unit/test_materialized_view.py +++ b/tests/unit/test_materialized_view.py @@ -72,18 +72,14 @@ def test_redshift_materialized_view_parse_relation_results_handles_multiples_sor column_descriptor = agate.Table.from_object( [ { - "schema": "my_schema", - "table": "my_table", "column": "my_column", "is_dist_key": True, - "is_sort_key": True, + "sort_key_position": 1, }, { - "schema": "my_schema", - "table": "my_table", "column": "my_column2", "is_dist_key": True, - "is_sort_key": True, + "sort_key_position": 2, }, ], ) @@ -98,13 +94,11 @@ def test_redshift_materialized_view_parse_relation_results_handles_multiples_sor relation_results = { "materialized_view": materialized_view, - "column_descriptor": column_descriptor, + "columns": column_descriptor, "query": query, } - config_dict = RedshiftMaterializedViewConfig.parse_relation_results( - relation_results - ) + config_dict = RedshiftMaterializedViewConfig.parse_relation_results(relation_results) assert isinstance(config_dict["sort"], dict) assert config_dict["sort"]["sortkey"][0] == "my_column" diff --git a/tests/unit/test_relation.py b/tests/unit/test_relation.py index f2d8a5373..0985c62c8 100644 --- a/tests/unit/test_relation.py +++ b/tests/unit/test_relation.py @@ -51,7 +51,7 @@ def materialized_view_without_sort_key_from_db(): relation_results = { "materialized_view": materialized_view, - "column_descriptor": column_descriptor, + "columns": column_descriptor, "query": query, } return relation_results @@ -67,27 +67,25 @@ def materialized_view_without_sort_key_config(): relation_config.config = Mock() relation_config.config.extra = {} relation_config.config.sort = "" - relation_config.compiled_code = "create materialized view my_view as (select 1 as my_column, 'value' as my_column2)" + relation_config.compiled_code = ( + "create materialized view my_view as (select 1 as my_column, 'value' as my_column2)" + ) return relation_config @pytest.fixture def materialized_view_multiple_sort_key_from_db(materialized_view_without_sort_key_from_db): - materialized_view_without_sort_key_from_db["column_descriptor"] = agate.Table.from_object( + materialized_view_without_sort_key_from_db["columns"] = agate.Table.from_object( [ { - "schema": "my_schema", - "table": "my_table", "column": "my_column", "is_dist_key": True, - "is_sort_key": True, + "sort_key_position": 1, }, { - "schema": "my_schema", - "table": "my_table", "column": "my_column2", "is_dist_key": True, - "is_sort_key": True, + "sort_key_position": 2, }, ], ) @@ -103,6 +101,7 @@ def materialized_view_multiple_sort_key_config(materialized_view_without_sort_ke return materialized_view_without_sort_key_config + def test_materialized_view_config_changeset_without_sort_key_empty_changes( materialized_view_without_sort_key_from_db, materialized_view_without_sort_key_config, @@ -114,6 +113,7 @@ def test_materialized_view_config_changeset_without_sort_key_empty_changes( assert change_set is None + def test_materialized_view_config_changeset_multiple_sort_key_without_changes( materialized_view_multiple_sort_key_from_db, materialized_view_multiple_sort_key_config, @@ -139,4 +139,19 @@ def test_materialized_view_config_changeset_multiple_sort_key_with_changes( ) assert change_set is not None - assert change_set.sort.context.sortkey == frozenset({"my_column", "my_column2", "my_column3"}) + assert change_set.sort.context.sortkey == ("my_column", "my_column2", "my_column3") + + +def test_materialized_view_config_changeset_multiple_sort_key_with_changes_in_order_column( + materialized_view_multiple_sort_key_from_db, + materialized_view_multiple_sort_key_config, +): + materialized_view_multiple_sort_key_config.config.extra["sort"] = ["my_column2", "my_column"] + + change_set = RedshiftRelation.materialized_view_config_changeset( + materialized_view_multiple_sort_key_from_db, + materialized_view_multiple_sort_key_config, + ) + + assert change_set is not None + assert change_set.sort.context.sortkey == ("my_column2", "my_column") From e4e8d526758a78a69046cacb3b51a7132ab58393 Mon Sep 17 00:00:00 2001 From: Luciano Vitti Date: Fri, 9 Aug 2024 18:01:33 -0300 Subject: [PATCH 6/6] fix linting. fix sort_key_position for non sort_key column --- dbt/adapters/redshift/relation_configs/materialized_view.py | 4 ++-- dbt/adapters/redshift/relation_configs/sort.py | 4 ++-- tests/unit/test_materialized_view.py | 5 +++++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dbt/adapters/redshift/relation_configs/materialized_view.py b/dbt/adapters/redshift/relation_configs/materialized_view.py index e65a7cf1c..a01185f22 100644 --- a/dbt/adapters/redshift/relation_configs/materialized_view.py +++ b/dbt/adapters/redshift/relation_configs/materialized_view.py @@ -206,8 +206,8 @@ def parse_relation_results(cls, relation_results: RelationResults) -> Dict: {"dist": RedshiftDistConfig.parse_relation_results(materialized_view)} ) - if columns:= relation_results.get("columns"): - sort_columns = [row for row in columns.rows if row.get("sort_key_position") > 0] + if columns := relation_results.get("columns"): + sort_columns = [row for row in columns.rows if row.get("sort_key_position", 0) > 0] if sort_columns: config_dict.update( {"sort": RedshiftSortConfig.parse_relation_results(sort_columns)} diff --git a/dbt/adapters/redshift/relation_configs/sort.py b/dbt/adapters/redshift/relation_configs/sort.py index 5bdb4c936..f38d5a1e1 100644 --- a/dbt/adapters/redshift/relation_configs/sort.py +++ b/dbt/adapters/redshift/relation_configs/sort.py @@ -137,7 +137,6 @@ def parse_relation_config(cls, relation_config: RelationConfig) -> Dict[str, Any return config_dict - @classmethod def parse_relation_results(cls, relation_results_entry: "agate.MappedSequence") -> dict: """ @@ -161,7 +160,7 @@ def parse_relation_results(cls, relation_results_entry: "agate.MappedSequence") Returns: a standard dictionary describing this `RedshiftSortConfig` instance """ sort_config = [] - + sorted_columns = sorted(relation_results_entry, key=lambda x: x["sort_key_position"]) for column in sorted_columns: if column.get("sort_key_position"): @@ -169,6 +168,7 @@ def parse_relation_results(cls, relation_results_entry: "agate.MappedSequence") return {"sortkey": sort_config} + @dataclass(frozen=True, eq=True, unsafe_hash=True) class RedshiftSortConfigChange(RelationConfigChange, RelationConfigValidationMixin): context: RedshiftSortConfig diff --git a/tests/unit/test_materialized_view.py b/tests/unit/test_materialized_view.py index 13d4341df..322b134f8 100644 --- a/tests/unit/test_materialized_view.py +++ b/tests/unit/test_materialized_view.py @@ -81,6 +81,11 @@ def test_redshift_materialized_view_parse_relation_results_handles_multiples_sor "is_dist_key": True, "sort_key_position": 2, }, + { + "column": "my_column5", + "is_dist_key": False, + "sort_key_position": 0, + }, ], )