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.

Sign up for GitHub

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

More consistent results from get_columns_in_relation #355

Merged
merged 4 commits into from
May 16, 2022
Merged

More consistent results from get_columns_in_relation #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9dc40c
More consistent results from get_columns_in_relation
jtcohen6 May 11, 2022
b3011ee
Not dispatched, full name
jtcohen6 May 11, 2022
be2f121
Add changelog entry
jtcohen6 May 11, 2022
44ade9c
Merge branch 'main' into fix/get-columns-in-relation-consistency
jtcohen6 May 16, 2022
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
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
## dbt-spark 1.1.0 (TBD)
## dbt-spark 1.2.0 (April 28, 2022)

### Fixes
- `adapter.get_columns_in_relation` (method) and `get_columns_in_relation` (macro) now return identical responses. The previous behavior of `get_columns_in_relation` (macro) is now represented by a new macro, `get_columns_in_relation_raw` ([#354](https://github.com/dbt-labs/dbt-spark/issues/354), [#355](https://github.com/dbt-labs/dbt-spark/pull/355))

## dbt-spark 1.1.0 (April 28, 2022)

### Features
- Add session connection method ([#272](https://github.com/dbt-labs/dbt-spark/issues/272), [#279](https://github.com/dbt-labs/dbt-spark/pull/279))
Expand Down
6 changes: 4 additions & 2 deletions dbt/adapters/spark/impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

logger = AdapterLogger("Spark")

GET_COLUMNS_IN_RELATION_MACRO_NAME = "get_columns_in_relation"
GET_COLUMNS_IN_RELATION_RAW_MACRO_NAME = "spark__get_columns_in_relation_raw"
LIST_SCHEMAS_MACRO_NAME = "list_schemas"
LIST_RELATIONS_MACRO_NAME = "list_relations_without_caching"
DROP_RELATION_MACRO_NAME = "drop_relation"
Expand Down Expand Up @@ -225,7 +225,9 @@ def get_columns_in_relation(self, relation: Relation) -> List[SparkColumn]:
# use get_columns_in_relation spark macro
# which would execute 'describe extended tablename' query
try:
rows: List[agate.Row] = super().get_columns_in_relation(relation)
rows: List[agate.Row] = self.execute_macro(
GET_COLUMNS_IN_RELATION_RAW_MACRO_NAME, kwargs={"relation": relation}
)
columns = self.parse_describe_extended(relation, rows)
except dbt.exceptions.RuntimeException as e:
# spark would throw error when table doesn't exist, where other
Expand Down
10 changes: 7 additions & 3 deletions dbt/include/spark/macros/adapters.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,15 @@
{%- endcall -%}
{% endmacro %}

{% macro spark__get_columns_in_relation(relation) -%}
{% call statement('get_columns_in_relation', fetch_result=True) %}
{% macro spark__get_columns_in_relation_raw(relation) -%}
Copy link
Contributor

@ueshin ueshin May 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@jtcohen6 Is it possible to make it dispatch pattern? We might want to override this. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I submitted a PR #365, could you take a look at it? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @ueshin -- utilizing the dispatch pattern makes good sense. 👍

{% call statement('get_columns_in_relation_raw', fetch_result=True) %}
describe extended {{ relation.include(schema=(schema is not none)) }}
{% endcall %}
{% do return(load_result('get_columns_in_relation').table) %}
{% do return(load_result('get_columns_in_relation_raw').table) %}
{% endmacro %}

{% macro spark__get_columns_in_relation(relation) -%}
{{ return(adapter.get_columns_in_relation(relation)) }}
{% endmacro %}

{% macro spark__list_relations_without_caching(relation) %}
Expand Down