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

DBZ-8689 Provide config to override all date/datetime cols to nullable #228

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

twthorn
Copy link
Contributor

@twthorn twthorn commented Feb 12, 2025

Ticket https://issues.redhat.com/browse/DBZ-8689

For nullable cols, we send null when we receive a zero date. For not nullable columns, we send the epoch value. It is impossible for consumer to differentiate between true epoch and zero value. Provide a setting to override these columns to nullable so that a special value (null) can be sent when a zero date is encountered that is distinct from the epoch value.

Considered using a custom converter, but it appeared to be an overly complex solution. Since these are temporal types, they conversion logic is dependent on the temporal mode eg for date or datetime. The converter would need to read in all these configs, and duplicate the logic (since the value conversion should be the same). So code is no longer dry and have potential for drift. All we need is to set the schema to be optional (but want an identical converter). I didn't see a good way to do this (nor a good way to read in those temporal configs in a custom converter), so I opted for a separate config that is read in when building the schema & value converters inVitessValueConverter. Please let me know if there's a better way to do this.

This config seems more akin to time.precision.mode setting which could technically be done with a custom converter but not without significant downside, so instead we have the connectors' main value converter read it in and handle the conversion logic there.

@twthorn
Copy link
Contributor Author

twthorn commented Feb 12, 2025

@jpechane this is also ready for review when you get a free moment, thanks!

@jpechane jpechane merged commit 10d8e6c into debezium:main Feb 24, 2025
4 checks passed
@jpechane
Copy link
Contributor

@twthorn Applied, thanks! Could you please send a PR with docs update too?

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