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.

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

Update replace.sql #186

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

rongfengliang
Copy link

@rongfengliang rongfengliang commented Apr 25, 2024

resolves: #187

{% set is_replaceable = existing_relation.type == target_relation_type and existing_relation.can_be_replaced %}

maybe

{% set is_replaceable = existing_relation.type == target_relation.type and existing_relation.can_be_replaced %}

{% set is_replaceable = existing_relation.type == target_relation_type and existing_relation.can_be_replaced %}

maybe 

{% set is_replaceable = existing_relation.type == target_relation.type and existing_relation.can_be_replaced %}
Copy link

cla-bot bot commented Apr 25, 2024

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: @rongfengliang

Copy link

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@cla-bot cla-bot bot added the cla:yes label Apr 25, 2024
@rongfengliang rongfengliang requested a review from a team as a code owner May 14, 2024 17:48
Copy link
Contributor

@mikealfare mikealfare left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @rongfengliang! We'll need to test this against concrete adapters to make sure that the functionality we're effectively turning on here works. In the meantime, if you could update the changelog body, then this will be ready to merge pending those tests.

.changes/unreleased/Fixes-20240425-133610.yaml Outdated Show resolved Hide resolved
@adrianburusdbt
Copy link
Contributor

@mikealfare functional tests performed on:

  • snowflake
  • bq
  • redshift
    No issues seen, unless other testing is needed it can be merged.

@mikealfare mikealfare added the community This PR is from a community member label Dec 11, 2024
@VersusFacit VersusFacit self-requested a review December 18, 2024 19:51
Copy link
Contributor

@VersusFacit VersusFacit left a comment

Choose a reason for hiding this comment

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

Thanks for including reference to concrete testing of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes community This PR is from a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_replace_sql is_replaceable check error
6 participants