MySQL Check binlog metadata setting during schema validation#35910
Conversation
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
ggevay
left a comment
There was a problem hiding this comment.
Approving to unblock the fixing of main, but would be good if someone from @MaterializeInc/sources-sinks would also take a look.
(Could you please rebase on the latest main? #35909 fixed another test fail that CI ran into here.)
martykulma
left a comment
There was a problem hiding this comment.
lgtm - I kicked off the MySQL nightly tests to validate the fix. The comments need updating for determine_compatibility but doesn't block.
src/mysql-util/src/desc.rs
Outdated
| @@ -95,16 +99,26 @@ impl MySqlTableDesc { | |||
| // ignore extra columns from `other.columns`. | |||
There was a problem hiding this comment.
This comment should be updated, and also the one for the method signature. Ok to do in a follow on PR to unblock.
aebf705 to
48c2706
Compare
6b7d777 to
ecc98a3
Compare
|
I'm fairly certain this PR has actually just exposed a race condition that already existed when handling MySQL restores, so I'm going to disable the commented out test for right now so that this PR can be merged to unblock main |
Addresses a gap where certain schema changes were being considered valid even though their validity is contingent on the
binlog_row_metadata = FULL;mysql system variable setting.Fixes this broken test: https://buildkite.com/materialize/nightly/builds/15965/steps/canvas?jid=019d6a48-8cf3-44a0-933f-451cda3adff1&tab=output