Skip to content

Commit

Permalink
Fix the example for api.Relation.create() (#5249)
Browse files Browse the repository at this point in the history
[Preview](https://docs-getdbt-com-git-dbeatty10-patch-3-dbt-labs.vercel.app/guides/create-new-materializations?step=2#update-the-relation-cache)

## What are you changing in this pull request and why?

### Problem

The code example within the guide for [custom
materializations](https://docs.getdbt.com/guides/create-new-materializations?step=2#update-the-relation-cache)
related to
[`api.Relation.create`](https://docs.getdbt.com/reference/dbt-classes#using-relations)
doesn't work as-is and yields a confusing error message:

```
23:15:15    Field "path" of type Path in PostgresRelation has invalid value {'database': 'my_database', 'schema': 'my_schema', 'identifier': Undefined}
```

### Solution

For some reason, `identifier` needs to be prefixed with `this.` whereas
`database` and `schema` don't 🤷

To make the code example most readable and least confusing, I'm opting
to add this prefix to all three.

### Alternative solutions

#### Alternative 1
Alternatively, we could have switched to using the approach in the
current
[table](https://github.com/dbt-labs/dbt-adapters/blob/7392debf7ff1aa5d58c0fe26eb76ac8ade4fe15b/dbt/include/global_project/macros/materializations/models/table.sql#L4)
and
[view](https://github.com/dbt-labs/dbt-adapters/blob/7392debf7ff1aa5d58c0fe26eb76ac8ade4fe15b/dbt/include/global_project/macros/materializations/models/view.sql#L4)
that uses `this.incorporate` instead:

```sql
target_relation = this.incorporate(type='...')
```

#### Alternative 2

Only replace `identifier=identifier` with `identifier=this.identifier`

#### Alternative 3

Only replace `identifier=identifier` with `identifier=model['alias']`

#### Alternative 4

Only add `{%- set identifier = model['alias'] -%}` earlier in the macro.

I opted against all the alternatives in order to use a consistent
pattern across `database`, `schema`, and `identifier` and also to make
the fewest changes possible.

## Checklist
- [x] Review the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md)
so my content adheres to these guidelines.

Co-authored-by: Mirna Wong <[email protected]>
  • Loading branch information
dbeatty10 and mirnawong1 authored Apr 9, 2024
1 parent c844562 commit 89754d5
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions website/docs/guides/create-new-materializations.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Materializations should [return](/reference/dbt-jinja-functions/return) the list
{%- materialization my_view, default -%}

{%- set target_relation = api.Relation.create(
identifier=identifier, schema=schema, database=database,
identifier=this.identifier, schema=this.schema, database=this.database,
type='view') -%}

-- ... setup database ...
Expand Down Expand Up @@ -187,4 +187,4 @@ Specific materializations can be selected by using the dot-notation when selecti

We recommend _not_ overriding materialization names directly, and instead using a prefix or suffix to denote that the materialization changes the behavior of the default implementation (eg. my_project_incremental).

</div>
</div>

0 comments on commit 89754d5

Please sign in to comment.