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

refactor: Backfill materialized columns by column reference instead of property #26742

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

tkaemming
Copy link
Contributor

@tkaemming tkaemming commented Dec 6, 2024

Problem

Some properties will have multiple columns associated with them during the transition period when rematerializing columns as nullable, so simply providing a property source column and property name will no longer be specific enough to identify a particular column that needs to be backfilled.

More context available in #26651.

Changes

  • Make backfill_materialized_columns take Iterable[MaterializedColumn] instead of list[tuple[PropertyName, TableColumn]] and update all relevant call sites.
  • Also updated materialize to return a MaterializedColumn instead of ColumnName | None to get rid of some annoying workarounds to appease the type checker.

Does this work well for both Cloud and self-hosted?

N/A

How did you test this code?

Updated tests.

@tkaemming tkaemming force-pushed the refactor-materialized-column-backfill branch from 4abd569 to a8ce35d Compare December 6, 2024 23:14
@tkaemming tkaemming requested a review from a team December 6, 2024 23:43
@tkaemming tkaemming marked this pull request as ready for review December 6, 2024 23:43
@tkaemming tkaemming merged commit 694d9c4 into master Dec 9, 2024
91 checks passed
@tkaemming tkaemming deleted the refactor-materialized-column-backfill branch December 9, 2024 19:17
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