Use model alias for the CTE identifier generated during ephemeral materialization #236
Conversation
|
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @jeancochrane |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
Two places in the product docs that we could update to explain the effect of the |
|
Thanks for the thorough review @dbeatty10! I think the changeset is now a lot simpler after e5c1b46. Would you like me to make a docs PR to tweak the sections you mentioned above? I'd be happy to do so, but am also down to leave it to your team if you'd prefer to have editorial control over this one. |
🎉 Nice work @jeancochrane !
That would be awesome if you are willing to raise a PR in docs.getdbt.com. If you tag me in it, I'll give a review along with one of my colleagues on the documentation team. |
jeancochrane
changed the title
jeancochrane
changed the title
## What are you changing in this pull request and why? This PR updates the docs to reflect the bugfix made in dbt-labs/dbt-core#10290 and dbt-labs/dbt-adapters#236 to bring the identifiers used for CTEs in line with the identifiers used for tables and views.⚠️ Note that these changes should **not** be deployed until those two PRs have been merged and released, since these docs will not accurately reflect the behavior of the packages until they have been patched. I've included this prerequisite in the checklist below. ## 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. - [x] For [docs versioning](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#about-versioning), review how to [version a whole page](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#adding-a-new-version) and [version a block of content](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#versioning-blocks-of-content). - [x] Add a checklist item for anything that needs to happen before this PR is merged, such as "needs technical review" or "change base branch." - [ ] Merge and release dbt-labs/dbt-core#10290 and dbt-labs/dbt-adapters#236 --------- Co-authored-by: Matt Shaver <60105315+matthewshaver@users.noreply.github.com>
ChenyuLInx
force-pushed
the
allow-control-over-ephemeral-cte-identifier
branch
from
8b41bc2
to
4f512d7
Compare
There was a problem hiding this comment.
Thanks @jeancochrane for the thoughtful PR!
This change looks good to me!
|
Closing and reopening PR to make sure all needed checks are running |
Partially resolves dbt-labs/dbt-core#5273. Reimplementation of dbt-labs/dbt-core#5488 to match the new split between adapter and core repos.
Problem
Ephemeral model materializations do not currently support using special characters like dots in file names, even if these special characters are sanitized using model aliases.
Solution
Prioritize using the model alias (if specified) via
model.identifierwhen creating a node identifier for ephemeral models.This change requires a corresponding change to
dbt-coreto fix the bug in the special case of unit testing, which will be implemented in a separate PR.Checklist