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

feat: introduce CDC generation for merge operations #2747

Merged
merged 3 commits into from
Aug 12, 2024
Merged

feat: introduce CDC generation for merge operations #2747

merged 3 commits into from
Aug 12, 2024

Conversation

rtyler
Copy link
Member

@rtyler rtyler commented Aug 10, 2024

it's been a slog y'all

This is the beginning of the work but represents working `delete`
`_change_type` information being produced on the merge operation
This does not have as much testing as it probably should, but
preliminary results are looking good!
@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Aug 10, 2024
@rtyler rtyler marked this pull request as ready for review August 10, 2024 22:17
@rtyler rtyler enabled auto-merge August 10, 2024 22:17
.and(col(TARGET_COLUMN).is_null()),
)?
.select(write_projection.clone())?
.with_column(CDC_COLUMN_NAME, lit("insert"))?,
Copy link
Collaborator

@ion-elgreco ion-elgreco Aug 10, 2024

Choose a reason for hiding this comment

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

Why do we create inserts? Isn't it cheaper to let the cdf_reader calculate what was inserted?

Hmm so while checking this action, it seems our cdf_reader doesn't properly do this

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rtyler I think we can add one optimization here, if we see it's insert only. So there is only a when_not_matched_insert_all statement, then we could avoid writing CDC as a whole.

Copy link
Collaborator

@ion-elgreco ion-elgreco left a comment

Choose a reason for hiding this comment

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

Nice work @rtyler 😄

@rtyler rtyler added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit 747603f Aug 12, 2024
18 checks passed
@rtyler rtyler deleted the merge-cdc branch August 12, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants