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

chore: wrap truncate/insert in a transaction. #5

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

HassanJaveed84
Copy link
Contributor

@HassanJaveed84 HassanJaveed84 commented Jul 28, 2022

Starting with release 0.21.0 of dbt(dbt-labs/dbt-core#3510), transactions were turned off and autocommit has been turned on.

This re-enables transactions for our custom materialization by wrapping truncate/insert in a begin/commit.

The integration tests passed, also confirmed visually by viewing query history:
Screen Shot 2022-07-29 at 3 13 16 AM

@HassanJaveed84 HassanJaveed84 requested a review from pwnage101 July 28, 2022 22:01
Copy link
Member

@pwnage101 pwnage101 left a comment

Choose a reason for hiding this comment

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

Thanks!!!

Comment on lines +32 to +35
"begin",
"truncate table",
"insert into",
"commit",
Copy link
Member

Choose a reason for hiding this comment

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

Wow! That was simple!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, now it's part of the compiled sql.

Comment on lines -122 to +126
{{ run_hooks(post_hooks, inside_transaction=True) }}

-- `COMMIT` happens here
{{ adapter.commit() }}

{{ run_hooks(post_hooks, inside_transaction=False) }}
{{ run_hooks(post_hooks) }}
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that this now reverses the order of post_hooks & commit. Beforehand, the commit happened after post_hooks, but now it happens before post_hooks. I would guess the new behavior is more correct anyway.

@HassanJaveed84 HassanJaveed84 force-pushed the hassan/fix-transaction-issue branch from 8ceb752 to 7ef6952 Compare August 1, 2022 13:53
@HassanJaveed84 HassanJaveed84 merged commit 92b2ffd into main Aug 1, 2022
@HassanJaveed84 HassanJaveed84 deleted the hassan/fix-transaction-issue branch August 1, 2022 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants