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

Don't force "before" to exist in Debezium #18844

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

umanwizard
Copy link
Contributor

This was necessary pre-DEBEZIUM UPSERT, but isn't anymore.

@umanwizard umanwizard requested a review from a team as a code owner April 19, 2023 16:33
@umanwizard umanwizard requested a review from a team April 19, 2023 17:32
@morsapaes
Copy link
Contributor

Thanks for patching this, @umanwizard! I've pinned down documentation and integration testing dependencies. Leaving the approval to Petros and the Storage team.

Copy link
Contributor

@aljoscha aljoscha left a comment

Choose a reason for hiding this comment

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

Seems good! My only concern would be if we do want to use ENVELOPE DEBEZIUM for an efficient implementation in the future again. If we now relax the requirements of ENVELOPE DEBEZIUM we'd have to introduce a different name for that in the future.

Not sure that future will ever come, though ... 🤷‍♂️

@umanwizard
Copy link
Contributor Author

You're right, and I hope that day does someday come! But FWIW, the plan has always been to leave DEBEZIUM UPSERT as the default, and make such a mode be enabled by special syntax or a WITH option. So I don't think this is a regression on that front.

@umanwizard umanwizard merged commit e1e4454 into MaterializeInc:main Apr 20, 2023
@benesch
Copy link
Member

benesch commented Apr 20, 2023

I know I sound like a broken record, but: this needs a test and a release note.

@benesch
Copy link
Member

benesch commented Apr 20, 2023

Testing is probably somewhat tricky, but I think we can do it by configuring one of the Debezium PostgreSQL CDC tests to replicate from a table with REPLICA IDENTITY NOTHING.

@benesch
Copy link
Member

benesch commented Apr 21, 2023

I filed https://github.com/MaterializeInc/database-issues/issues/5580 to track the test.

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.

5 participants