Various MySQL Bug Fixes

[patrickwwbutler]

Fixes https://github.com/MaterializeInc/database-issues/issues/11312 by changing the decoding logic - previously with binlog_row_metadata = MINIMAL, excluded columns were not being skipped in the binlog, resulting in the wrong column, because we were filtering out excluded columns from the desc columns before zipping. Also changes logic for replication when binlog_row_metadata = FULL by building a vec of pairs by locating the matching index in the binlog and grabbing the value there.

Fixes https://github.com/MaterializeInc/database-issues/issues/11313 by filtering out excluded columns when checking compatibility.

Fixes https://github.com/MaterializeInc/database-issues/issues/11315 by checking for version before both of the binlog_row_metadata checks.

[patrickwwbutler]

@def- I clauded up a test for the version check - but unfortunately it seems hard to actually run as we don't have images for lower mysql versions:

$ docker compose up --detach --wait materialized mysql
[+] Running 0/1
 ⠦ mysql Pulling                                                                                                                                                                                                                                                              0.6s 
no matching manifest for linux/arm64/v8 in the manifest list entries

Do we have a good way to test this?

[def-]

With this test addition:

diff --git a/test/mysql-cdc/binlog-backward-compat.td b/test/mysql-cdc/binlog-backward-compat.td
index e4113285a8..765c66f0bd 100644
--- a/test/mysql-cdc/binlog-backward-compat.td
+++ b/test/mysql-cdc/binlog-backward-compat.td
@@ -102,6 +102,33 @@ DELETE FROM bar WHERE a = 4;
 > DROP SOURCE mysql_src2 CASCADE;


+$ mysql-execute name=mysql
+SET GLOBAL binlog_row_metadata = FULL;
+USE public;
+CREATE TABLE baz (a INT, b INT, c INT);
+INSERT INTO baz VALUES (1, 2, 3), (4, 5, 6);
+
+> CREATE SOURCE mysql_src3 FROM MYSQL CONNECTION mysql_conn;
+
+> CREATE TABLE baz FROM SOURCE mysql_src3 (REFERENCE public.baz) WITH (EXCLUDE COLUMNS (b));
+
+> SELECT * FROM baz;
+1 3
+4 6
+
+$ mysql-execute name=mysql
+SET GLOBAL binlog_row_metadata = MINIMAL;
+ALTER TABLE baz ADD COLUMN d INT DEFAULT NULL;
+INSERT INTO baz (a, b, c) VALUES (7, 8, 9);
+
+> SELECT * FROM baz;
+1 3
+4 6
+7 9
+
+> DROP SOURCE mysql_src3 CASCADE;
+
+
 # Restore to a clean state so other tests are unaffected.
 $ mysql-execute name=mysql
 SET GLOBAL binlog_row_metadata = FULL;

Do we expect to get "incompatible schema change" when MySQL switches from FULL to MINIMAL and you run any DDL? Because it fails like that for me:

binlog-backward-compat.td:136:1: error: executing query failed: db error: ERROR: Source error: source must be dropped and recreated due to failure: incompatible schema change: column c in table baz has been altered
     |
  15 | > CREATE SECRET mysq ... [rest of line truncated for security]
  20 |     PASSWORD SECRET  ... [rest of line truncated for security]
  23 | $ mysql-connect name ... [rest of line truncated for security]
 135 |
 136 | > SELECT * FROM baz;
     | ^
^^^ +++
+++ !!! Error Report
1 errors were encountered during execution
files involved: binlog-backward-compat.td

[martykulma]

Thanks @patrickwwbutler! Left some comments, one of them is a nit, but the other one we need to nail down.

@maheshwarip - just a heads up as this may be unexpected - MySQL 5.7 can't be supported with source versioning as it doesn't have support for full row metadata.

[martykulma]

this is also a problem - any error that becomes a definite error can't change the string it renders to, so this needs to be "extra column description"

[patrickwwbutler]

okay I know you explained this yesterday but can you go over it again? this is so that we can retract the error successfully?

[martykulma]

np - updates will come in pairs - pre-image/post-image

consider this set of updates where B fails to decode:

t : pre  : post
--:------:-----
1 : NULL : A    row with A is inserted     -> emit A:diff=1
6 :   A  : B    row with A is updated to B -> emit A:diff=-1, Err:diff=1
9 :   B  : C    row with B is updated to C -> emit Err:diff=-1, C:diff=1

If the Err at ts=6 is different from ts=9, it will have issued a retraction for a row that was never inserted.

In the recent issue, the subsource stalled and couldn't make any progress due to a bug, so recreating was inevitable. In some other cases, might not be the case, so we have to maintain the discipline until this behavior is changed.

[patrickwwbutler]

Ah we need to retract the exact same error message in order to actually "undo" the error, right. Wonder if there's some other way to do this, but anything I can think of basically transposes into the upsert problem, which we would obviously not want to implement for regular sources