From e79b19e6b5252f6a03605a43caf449d01acc36f7 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Fri, 26 Apr 2019 13:01:02 -0400 Subject: [PATCH 1/4] Replace deprecated adapter methods with relations --- README.md | 4 ++-- .../models/sql/test_nullcheck_table.sql | 2 +- macros/schema_tests/equality.sql | 6 +----- macros/sql/get_tables_by_prefix.sql | 20 +++++++++++++------ macros/sql/get_tables_by_prefix_sql.sql | 4 ++-- macros/sql/nullcheck_table.sql | 6 +++--- macros/sql/star.sql | 8 +------- macros/sql/union.sql | 8 +------- macros/sql/unpivot.sql | 8 +------- 9 files changed, 26 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index aa734e06..79affb14 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,7 @@ from {{ref('my_model')}} ``` #### union_tables ([source](macros/sql/union.sql)) -This macro implements an "outer union." The list of tables provided to this macro will be unioned together, and any columns exclusive to a subset of these tables will be filled with `null` where not present. The `column_override` argument is used to explicitly assign the column type for a set of columns. The `source_column_name` argument is used to change the name of the`_dbt_source_table` field. +This macro implements an "outer union." The list of relations provided to this macro will be unioned together, and any columns exclusive to a subset of these tables will be filled with `null` where not present. The `column_override` argument is used to explicitly assign the column type for a set of columns. The `source_column_name` argument is used to change the name of the`_dbt_source_table` field. Usage: ``` @@ -440,7 +440,7 @@ models: project-name: post-hook: "grant select on {{ this }} to db_reader" ``` -A useful workaround is to change the above post-hook to: +A useful workaround is to change the above post-hook t o: ```yaml post-hook: "grant select on {{ this.schema }}.{{ this.name }} to db_reader" ``` diff --git a/integration_tests/models/sql/test_nullcheck_table.sql b/integration_tests/models/sql/test_nullcheck_table.sql index 6c49b19d..ea66c8ec 100644 --- a/integration_tests/models/sql/test_nullcheck_table.sql +++ b/integration_tests/models/sql/test_nullcheck_table.sql @@ -7,7 +7,7 @@ with nulled as ( - {{ dbt_utils.nullcheck_table(tbl.schema, tbl.name) }} + {{ dbt_utils.nullcheck_table(tbl) }} ) diff --git a/macros/schema_tests/equality.sql b/macros/schema_tests/equality.sql index 7f97bbdd..48f8af5b 100644 --- a/macros/schema_tests/equality.sql +++ b/macros/schema_tests/equality.sql @@ -9,11 +9,7 @@ {% endif %} -- setup - -{% set schema = model.schema %} -{% set model_a_name = model.name %} - -{% set dest_columns = adapter.get_columns_in_table(schema, model_a_name) %} +{% set dest_columns = adapter.get_columns_in_relation(model) %} {% set dest_cols_csv = dest_columns | map(attribute='quoted') | join(', ') %} with a as ( diff --git a/macros/sql/get_tables_by_prefix.sql b/macros/sql/get_tables_by_prefix.sql index 648c13dd..1664190b 100644 --- a/macros/sql/get_tables_by_prefix.sql +++ b/macros/sql/get_tables_by_prefix.sql @@ -1,16 +1,24 @@ -{% macro get_tables_by_prefix(schema, prefix, exclude='') %} +{% macro get_tables_by_prefix(schema, prefix, exclude='', database=target.database) %} - {%- call statement('tables', fetch_result=True) %} + {%- call statement('get_tables', fetch_result=True) %} {{ dbt_utils.get_tables_by_prefix_sql(schema, prefix, exclude) }} {%- endcall -%} - {%- set table_list = load_result('tables') -%} + {%- set table_list = load_result('get_tables') -%} - {%- if table_list and table_list['data'] -%} - {%- set tables = table_list['data'] | map(attribute=0) | list %} - {{ return(tables) }} + {%- if table_list and table_list['table'] -%} + {%- set tbl_relations = [] -%} + {%- for row in table_list['table'] -%} + {%- set tbl_relation = adapter.get_relation(database, row.table_schema, row.table_name) -%} + {%- if not tbl_relation -%} + {%- set tbl_relation = api.Relation.create(database, row.table_schema, row.table_name) -%} + {%- endif -%} + {%- do tbl_relations.append(tbl_relation) -%} + {%- endfor -%} + + {{ return(tbl_relations) }} {%- else -%} {{ return([]) }} {%- endif -%} diff --git a/macros/sql/get_tables_by_prefix_sql.sql b/macros/sql/get_tables_by_prefix_sql.sql index 0ee7f806..7239e727 100644 --- a/macros/sql/get_tables_by_prefix_sql.sql +++ b/macros/sql/get_tables_by_prefix_sql.sql @@ -5,7 +5,7 @@ {% macro default__get_tables_by_prefix_sql(schema, prefix, exclude='') %} select distinct - table_schema || '.' || table_name as ref + table_schema, table_name from information_schema.tables where table_schema = '{{ schema }}' and table_name ilike '{{ prefix }}%' @@ -17,7 +17,7 @@ {% macro bigquery__get_tables_by_prefix_sql(schema, prefix, exclude='') %} select distinct - concat(dataset_id, '.', table_id) as ref + dataset_id as table_schema, table_id as table_name from {{schema}}.__TABLES_SUMMARY__ where dataset_id = '{{schema}}' diff --git a/macros/sql/nullcheck_table.sql b/macros/sql/nullcheck_table.sql index bd6e34a0..e77df3d5 100644 --- a/macros/sql/nullcheck_table.sql +++ b/macros/sql/nullcheck_table.sql @@ -1,8 +1,8 @@ -{% macro nullcheck_table(schema, table) %} +{% macro nullcheck_table(relation) %} - {% set cols = adapter.get_columns_in_table(schema, table) %} + {% set cols = adapter.get_columns_in_relation(relation) %} select {{ dbt_utils.nullcheck(cols) }} - from {{schema}}.{{table}} + from {{relation}} {% endmacro %} diff --git a/macros/sql/star.sql b/macros/sql/star.sql index a72877a4..3dfd17a6 100644 --- a/macros/sql/star.sql +++ b/macros/sql/star.sql @@ -5,14 +5,8 @@ {{ return('') }} {% endif %} - {%- if from.name -%} - {%- set schema_name, table_name = from.schema, from.name -%} - {%- else -%} - {%- set schema_name, table_name = (from | string).split(".") -%} - {%- endif -%} - {%- set include_cols = [] %} - {%- set cols = adapter.get_columns_in_table(schema_name, table_name) -%} + {%- set cols = adapter.get_columns_in_relation(from) -%} {%- for col in cols -%} {%- if col.column not in except -%} diff --git a/macros/sql/union.sql b/macros/sql/union.sql index 23fe73d9..b46518ce 100644 --- a/macros/sql/union.sql +++ b/macros/sql/union.sql @@ -16,13 +16,7 @@ {%- set _ = table_columns.update({table: []}) %} - {%- if table.name -%} - {%- set schema, table_name = table.schema, table.name -%} - {%- else -%} - {%- set schema, table_name = (table | string).split(".") -%} - {%- endif -%} - - {%- set cols = adapter.get_columns_in_table(schema, table_name) %} + {%- set cols = adapter.get_columns_in_relation(table) %} {%- for col in cols -%} {%- if col.column not in exclude %} diff --git a/macros/sql/unpivot.sql b/macros/sql/unpivot.sql index 1d0a0351..29e3fe25 100644 --- a/macros/sql/unpivot.sql +++ b/macros/sql/unpivot.sql @@ -19,13 +19,7 @@ Arguments: {%- set _ = table_columns.update({table: []}) %} - {%- if table.name -%} - {%- set schema, table_name = table.schema, table.name -%} - {%- else -%} - {%- set schema, table_name = (table | string).split(".") -%} - {%- endif -%} - - {%- set cols = adapter.get_columns_in_table(schema, table_name) %} + {%- set cols = adapter.get_columns_in_relation(table) %} {%- for col in cols -%} {%- if col.column.lower() not in exclude|map('lower') -%} From c315cfc6dc2e9b42a9c157992716d3fea9526a08 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Fri, 26 Apr 2019 18:20:10 -0400 Subject: [PATCH 2/4] Handle snowflake info schema casing --- macros/sql/get_tables_by_prefix_sql.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macros/sql/get_tables_by_prefix_sql.sql b/macros/sql/get_tables_by_prefix_sql.sql index 7239e727..242e9f11 100644 --- a/macros/sql/get_tables_by_prefix_sql.sql +++ b/macros/sql/get_tables_by_prefix_sql.sql @@ -5,7 +5,7 @@ {% macro default__get_tables_by_prefix_sql(schema, prefix, exclude='') %} select distinct - table_schema, table_name + table_schema as "table_schema", table_name as "table_name" from information_schema.tables where table_schema = '{{ schema }}' and table_name ilike '{{ prefix }}%' From 0ce0b44a8f197a3c2b3b9aaa56e313c7b45dc2d6 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 30 Apr 2019 09:23:43 -0400 Subject: [PATCH 3/4] Add _is_relation test. Consistent contracts --- README.md | 2 +- macros/cross_db_utils/_is_relation.sql | 5 +++++ macros/schema_tests/equality.sql | 1 + macros/sql/get_tables_by_prefix.sql | 7 ++----- macros/sql/get_tables_by_prefix_sql.sql | 10 +++++----- macros/sql/nullcheck_table.sql | 1 + macros/sql/star.sql | 2 ++ macros/sql/union.sql | 1 + macros/sql/unpivot.sql | 3 ++- 9 files changed, 20 insertions(+), 12 deletions(-) create mode 100644 macros/cross_db_utils/_is_relation.sql diff --git a/README.md b/README.md index 79affb14..92025e06 100644 --- a/README.md +++ b/README.md @@ -440,7 +440,7 @@ models: project-name: post-hook: "grant select on {{ this }} to db_reader" ``` -A useful workaround is to change the above post-hook t o: +A useful workaround is to change the above post-hook to: ```yaml post-hook: "grant select on {{ this.schema }}.{{ this.name }} to db_reader" ``` diff --git a/macros/cross_db_utils/_is_relation.sql b/macros/cross_db_utils/_is_relation.sql new file mode 100644 index 00000000..8c52d42f --- /dev/null +++ b/macros/cross_db_utils/_is_relation.sql @@ -0,0 +1,5 @@ +{% macro _is_relation(arg, macro) %} + {%- if execute and arg.name is none -%} + {%- do exceptions.raise_compiler_error("Macro " ~ macro ~ " expected a Relation but received the value: " ~ arg) -%} + {%- endif -%} +{% endmacro %} diff --git a/macros/schema_tests/equality.sql b/macros/schema_tests/equality.sql index 48f8af5b..519dd198 100644 --- a/macros/schema_tests/equality.sql +++ b/macros/schema_tests/equality.sql @@ -9,6 +9,7 @@ {% endif %} -- setup +{%- do dbt_utils._is_relation(model, 'test_equality') -%} {% set dest_columns = adapter.get_columns_in_relation(model) %} {% set dest_cols_csv = dest_columns | map(attribute='quoted') | join(', ') %} diff --git a/macros/sql/get_tables_by_prefix.sql b/macros/sql/get_tables_by_prefix.sql index 1664190b..b7ce7f48 100644 --- a/macros/sql/get_tables_by_prefix.sql +++ b/macros/sql/get_tables_by_prefix.sql @@ -2,7 +2,7 @@ {%- call statement('get_tables', fetch_result=True) %} - {{ dbt_utils.get_tables_by_prefix_sql(schema, prefix, exclude) }} + {{ dbt_utils.get_tables_by_prefix_sql(schema, prefix, exclude, database) }} {%- endcall -%} @@ -11,10 +11,7 @@ {%- if table_list and table_list['table'] -%} {%- set tbl_relations = [] -%} {%- for row in table_list['table'] -%} - {%- set tbl_relation = adapter.get_relation(database, row.table_schema, row.table_name) -%} - {%- if not tbl_relation -%} - {%- set tbl_relation = api.Relation.create(database, row.table_schema, row.table_name) -%} - {%- endif -%} + {%- set tbl_relation = api.Relation.create(database, row.table_schema, row.table_name) -%} {%- do tbl_relations.append(tbl_relation) -%} {%- endfor -%} diff --git a/macros/sql/get_tables_by_prefix_sql.sql b/macros/sql/get_tables_by_prefix_sql.sql index 242e9f11..c26d06e5 100644 --- a/macros/sql/get_tables_by_prefix_sql.sql +++ b/macros/sql/get_tables_by_prefix_sql.sql @@ -1,12 +1,12 @@ -{% macro get_tables_by_prefix_sql(schema, prefix, exclude='') %} +{% macro get_tables_by_prefix_sql(schema, prefix, exclude='', database=target.database) %} {{ adapter_macro('dbt_utils.get_tables_by_prefix_sql', schema, prefix, exclude) }} {% endmacro %} -{% macro default__get_tables_by_prefix_sql(schema, prefix, exclude='') %} +{% macro default__get_tables_by_prefix_sql(schema, prefix, exclude='', database=target.database) %} select distinct table_schema as "table_schema", table_name as "table_name" - from information_schema.tables + from {{adapter.quote(database)}}.information_schema.tables where table_schema = '{{ schema }}' and table_name ilike '{{ prefix }}%' and table_name not ilike '{{ exclude }}' @@ -14,12 +14,12 @@ {% endmacro %} -{% macro bigquery__get_tables_by_prefix_sql(schema, prefix, exclude='') %} +{% macro bigquery__get_tables_by_prefix_sql(schema, prefix, exclude='', database=target.database) %} select distinct dataset_id as table_schema, table_id as table_name - from {{schema}}.__TABLES_SUMMARY__ + from {{adapter.quote(database)}}.{{schema}}.__TABLES_SUMMARY__ where dataset_id = '{{schema}}' and lower(table_id) like lower ('{{prefix}}%') and lower(table_id) not like lower ('{{exclude}}') diff --git a/macros/sql/nullcheck_table.sql b/macros/sql/nullcheck_table.sql index e77df3d5..e79c12ed 100644 --- a/macros/sql/nullcheck_table.sql +++ b/macros/sql/nullcheck_table.sql @@ -1,5 +1,6 @@ {% macro nullcheck_table(relation) %} + {%- do dbt_utils._is_relation(relation, 'nullcheck_table') -%} {% set cols = adapter.get_columns_in_relation(relation) %} select {{ dbt_utils.nullcheck(cols) }} diff --git a/macros/sql/star.sql b/macros/sql/star.sql index 3dfd17a6..d53e8282 100644 --- a/macros/sql/star.sql +++ b/macros/sql/star.sql @@ -1,4 +1,6 @@ {% macro star(from, relation_alias=False, except=[]) -%} + + {%- do dbt_utils._is_relation(from, 'star') -%} {#-- Prevent querying of db in parsing mode. This works because this macro does not create any new refs. #} {%- if not execute -%} diff --git a/macros/sql/union.sql b/macros/sql/union.sql index b46518ce..5fcde0cd 100644 --- a/macros/sql/union.sql +++ b/macros/sql/union.sql @@ -16,6 +16,7 @@ {%- set _ = table_columns.update({table: []}) %} + {%- do dbt_utils._is_relation(table, 'union_tables') -%} {%- set cols = adapter.get_columns_in_relation(table) %} {%- for col in cols -%} diff --git a/macros/sql/unpivot.sql b/macros/sql/unpivot.sql index 29e3fe25..efe24c01 100644 --- a/macros/sql/unpivot.sql +++ b/macros/sql/unpivot.sql @@ -4,7 +4,7 @@ Pivot values from columns to rows. Example Usage: {{ dbt_utils.unpivot(table=ref('users'), cast_to='integer', exclude=['id','created_at']) }} Arguments: - table: Table name, required. + table: Relation object, required. cast_to: The datatype to cast all unpivoted columns to. Default is varchar. exclude: A list of columns to exclude from the unpivot operation. Default is none. #} @@ -19,6 +19,7 @@ Arguments: {%- set _ = table_columns.update({table: []}) %} + {%- do dbt_utils._is_relation(table, 'unpivot') -%} {%- set cols = adapter.get_columns_in_relation(table) %} {%- for col in cols -%} From 358f94de3dff0fea6fcc8e7a212156e10e921734 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 30 Apr 2019 11:57:36 -0400 Subject: [PATCH 4/4] Better _is_relation logic --- macros/cross_db_utils/_is_relation.sql | 6 +++--- macros/sql/get_tables_by_prefix_sql.sql | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/macros/cross_db_utils/_is_relation.sql b/macros/cross_db_utils/_is_relation.sql index 8c52d42f..bee6db97 100644 --- a/macros/cross_db_utils/_is_relation.sql +++ b/macros/cross_db_utils/_is_relation.sql @@ -1,5 +1,5 @@ -{% macro _is_relation(arg, macro) %} - {%- if execute and arg.name is none -%} - {%- do exceptions.raise_compiler_error("Macro " ~ macro ~ " expected a Relation but received the value: " ~ arg) -%} +{% macro _is_relation(obj, macro) %} + {%- if not (obj is mapping and obj.get('metadata', {}).get('type', '').endswith('Relation')) -%} + {%- do exceptions.raise_compiler_error("Macro " ~ macro ~ " expected a Relation but received the value: " ~ obj) -%} {%- endif -%} {% endmacro %} diff --git a/macros/sql/get_tables_by_prefix_sql.sql b/macros/sql/get_tables_by_prefix_sql.sql index c26d06e5..3c27c825 100644 --- a/macros/sql/get_tables_by_prefix_sql.sql +++ b/macros/sql/get_tables_by_prefix_sql.sql @@ -6,7 +6,7 @@ select distinct table_schema as "table_schema", table_name as "table_name" - from {{adapter.quote(database)}}.information_schema.tables + from {{database}}.information_schema.tables where table_schema = '{{ schema }}' and table_name ilike '{{ prefix }}%' and table_name not ilike '{{ exclude }}'