Skip to content

Fix(mssql): Use truncate+insert for FULL models instead of merge #4531

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

Merged
merged 1 commit into from
May 26, 2025

Conversation

erindru
Copy link
Collaborator

@erindru erindru commented May 26, 2025

Addresses issue #4472

This PR:

  • Overrides delete_from() to use TRUNCATE if there is no WHERE clause. This works because unlike many other databases, truncate works within a transaction on MSSQL and is more efficient than using DELETE
  • Uses the DELETE+INSERT strategy instead of MERGE if _insert_overwrite_by_condition is called without a condition. _insert_overwrite_by_condition is used by replace_table() which is how the FULL materialization strategy is implemented.

The net result is that full data replacements in existing tables become a lot more efficient

if not where or where == exp.true():
# this is a full table replacement, call the base strategy to do DELETE+INSERT
# which will result in TRUNCATE+INSERT due to how we have overridden self.delete_from()
return EngineAdapter._insert_overwrite_by_condition(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

super(EngineAdapter, self) threw an exception and super(EngineAdapterWithIndexSupport, self) returned the _insert_overwrite_by_condition method from InsertOverwriteWithMergeMixin instead of from EngineAdapter.

So I worked around this by making a direct call to EngineAdapter

@erindru erindru merged commit 01f7af8 into main May 26, 2025
23 checks passed
@erindru erindru deleted the erin/mssql-full-strategy branch May 26, 2025 19:56
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