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

Pass errors to the sink trait instead of dropping the batch #69

Closed
wants to merge 2 commits into from

Conversation

passcod
Copy link
Contributor

@passcod passcod commented Nov 30, 2024

Background: this is something I did in my branch while implementing a custom Sink. My target database had types that pg_replicate didn't handle at the time, as well as some custom types which pg_replicate can't handle at all. Debugging this efficiently was hindered by pg_replicate crashing immediately when it encountered a type it couldn't handle.

More recently the unknown_types_to_bytes feature has been introduced which probably renders this change unnecessary at least for decoding errors, but I want to propose it anyway; however I understand if it's rejected.

  • Change the Sink trait to accept Results in the write_table_row, write_table_rows, and write_cdc_event methods.
  • The pipeline, instead of erroring early on encountering an error while processing an event/row batch, and dropping the events/rows, now passes the entire results directly to the trait.
  • The trait implementation is then responsible for handling those errors as it wishes, including ignoring errored rows.

@imor
Copy link
Contributor

imor commented Dec 1, 2024

Thanks for the PR, I'll review tomorrow.

@imor
Copy link
Contributor

imor commented Dec 2, 2024

unknown_types_to_bytes was kind of a stopgap solution and would probably be removed. Now that we've switched the table copies as well the CDC stream to the text format the fallback type is a string. So any unsupported type would become a string with its value being whatever Postgres's text format output for that type is. But I actually liked the fail early approach because it surfaced the error instead of silently converting its type to string.

I think a much more useful feature would be to add the ability for the consumers of pg_replicate to add support for their custom types in their own crates. Do you think this will work for you with your custom types?

@passcod
Copy link
Contributor Author

passcod commented Dec 2, 2024

Absolutely. That sounds like a much better solution. I'll close this then.

@passcod passcod closed this Dec 2, 2024
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