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!: correctly use execution time on new rows for scd type 2 by column #3192

Merged
merged 2 commits into from
Sep 27, 2024

Conversation

eakmanrq
Copy link
Contributor

This corrects the SCD Type 2 By Column default behavior to use execution time for new rows after the table is initially loaded. This matches what the documentation example says it should behave. This does mean there is a behavior change for current users but the current behavior seems to be a bug. There is not a way for a user to maintain old behavior if they wanted it since we consider it a bug.

From reviewing the code, truncate is a great way to identify if a table is being loaded for the first time. This also means that if a user issues a restatement on the table and wants to rebuild from scratch then it would also be set to truncate and would mimic loading the rows for the first time.

@eakmanrq eakmanrq force-pushed the eakmanrq/fix_scd_type_2_column_behavior branch from f889b61 to f960fb6 Compare September 27, 2024 00:15
@@ -1364,6 +1364,7 @@ def test_scd_type_2_by_time(ctx: TestContext):
execution_time="2023-01-01 00:00:00",
updated_at_as_valid_from=False,
columns_to_types=input_schema,
truncate=True,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This properly mimics how the adapter will be used by truncating on first load and then not truncating on follow up load.

@@ -1633,7 +1637,7 @@ def test_scd_type_2_by_column(ctx: TestContext):
"id": 5,
"name": "e",
"status": "inactive",
"valid_from": "1970-01-01 00:00:00",
"valid_from": "2023-01-05 00:00:00",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The prior value was a bug and this is it being fixed.

@eakmanrq eakmanrq force-pushed the eakmanrq/fix_scd_type_2_column_behavior branch from f960fb6 to 75f5b78 Compare September 27, 2024 00:18
@eakmanrq eakmanrq requested a review from a team September 27, 2024 00:38
@eakmanrq eakmanrq merged commit 31b96aa into main Sep 27, 2024
23 checks passed
@eakmanrq eakmanrq deleted the eakmanrq/fix_scd_type_2_column_behavior branch September 27, 2024 15: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