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

Fix the renamed relations code against 1.8 #719

Conversation

VersusFacit
Copy link
Contributor

@VersusFacit VersusFacit commented Feb 24, 2024

resolves #693
docs dbt-labs/docs.getdbt.com/#

Problem

There are too many BEGINs and COMMITs in the logs. This is due to how we're nesting macros. This is a problem when drops and renames need to operate together in a single transaction.

This is a red herring. The real problem is renamed_relations is not evaluating right on instantiation and causing drops where there shouldn't be any.

Solution

Change instantiation semantics in core and here. Add test.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

Copy link
Contributor

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 dbt-redshift contributing guide.

@VersusFacit VersusFacit force-pushed the ADAP-1077/restore_drop_rename_table_in_same_transaction branch from 8051de7 to 3c3edce Compare February 24, 2024 02:56
@VersusFacit VersusFacit force-pushed the ADAP-1077/restore_drop_rename_table_in_same_transaction branch from 3c3edce to 5c9a3fe Compare February 24, 2024 04:38
@cla-bot cla-bot bot added the cla:yes label Feb 24, 2024
@VersusFacit VersusFacit force-pushed the ADAP-1077/restore_drop_rename_table_in_same_transaction branch from 8c14348 to 8a31975 Compare February 24, 2024 05:06
@VersusFacit VersusFacit changed the title Remove dispatch to stop excessive transaction bracing. Fix the downstream code Feb 27, 2024
@VersusFacit VersusFacit changed the title Fix the downstream code Fix the renamed relations code Feb 27, 2024
@VersusFacit VersusFacit reopened this Feb 27, 2024
@VersusFacit VersusFacit changed the base branch from main to 1.7.latest February 27, 2024 19:20
@VersusFacit VersusFacit changed the base branch from 1.7.latest to main February 27, 2024 19:21
@mikealfare mikealfare changed the base branch from main to 1.7.latest February 28, 2024 03:11
@mikealfare mikealfare changed the base branch from 1.7.latest to main February 28, 2024 03:11
@VersusFacit VersusFacit changed the title Fix the renamed relations code Fix the renamed relations code against 1.7 Feb 28, 2024
dev-requirements.txt Outdated Show resolved Hide resolved
@VersusFacit VersusFacit force-pushed the ADAP-1077/restore_drop_rename_table_in_same_transaction branch from f8e710a to 71ca351 Compare February 28, 2024 18:39
@mikealfare mikealfare changed the title Fix the renamed relations code against 1.7 Fix the renamed relations code against 1.8 Mar 21, 2024
@mikealfare
Copy link
Contributor

This looks like it might be a dupe. I'll come back after merging the other PR and clean this up if that's the case.

@mikealfare
Copy link
Contributor

After fixing rename_relations, this PR has been reduced to the changelog, hence I'm closing it as a dupe. Please refer to dbt-labs/dbt-core#9681 for more information.

@mikealfare mikealfare closed this Mar 21, 2024
@VersusFacit VersusFacit deleted the ADAP-1077/restore_drop_rename_table_in_same_transaction branch March 27, 2024 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants