-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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-3643 Fix to support NOBLOBs among binlog_row_image attributes. #5319
base: main
Are you sure you want to change the base?
Conversation
Hi @Bue-von-hon. Thank you for your valuable contribution. |
Welcome as a new contributor to Debezium, @Bue-von-hon. Reviewers, please add missing author name(s) and alias name(s) to the COPYRIGHT.txt and Aliases.txt respectively. |
dc396a0
to
79faf46
Compare
Hi @Bue-von-hon. Thank you for your valuable contribution. |
Hi @Bue-von-hon, thanks. I commented on your Jira about the test case coverages, do you think you could add some test cases as I described in the Jira, thanks. |
/** | ||
* Determines whether the binlog format used by the database server is {@code binlog_row_image='NOBLOB'}. | ||
* | ||
* @return {@code true} if the {@code binlog_row_image} is set to {@code NOBLOB}, {@code false} otherwise | ||
*/ | ||
public boolean isBinlogRowImageNoblob() { | ||
try { | ||
final String rowImage = queryAndMap("SHOW GLOBAL VARIABLES LIKE 'binlog_row_image'", rs -> { | ||
if (rs.next()) { | ||
return rs.getString(2); | ||
} | ||
// This setting was introduced in MySQL 5.6+ with default of 'FULL'. | ||
// For older versions, assume 'FULL'. | ||
return "FULL"; | ||
}); | ||
LOGGER.debug("binlog_row_image={}", rowImage); | ||
return "NOBLOB".equalsIgnoreCase(rowImage); | ||
} | ||
catch (SQLException e) { | ||
throw new DebeziumException("Unexpected error while connecting to the database and looking at BINLOG_ROW_IMAGE mode: ", e); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't care for the fact we're introducing a second method that does nearly the identical behavior as isBinlogRowImageFull
. Could we introduce a protected/private method like the following and then delegate both isBinlogRowImageFull()
and isBinlogRowImageNoblob()
to this:
public boolean isBinlogRowImageFull() {
return isBinlogRowImage("FULL");
}
public boolean isBinlogRowImageNoblob() {
return isBinlogRowImage("NOBLOB");
}
protected boolean isBinlogRowImage(String expectedType) {
try {
final String rowImage = queryAndMap("SHOW GLOBAL VARIABLES LIKE 'binlog_row_image'", rs -> {
if (rs.next()) {
return rs.getString(2);
}
// This setting was introduced in MySQL 5.6+ with default of 'FULL'.
// For older versions, assume 'FULL'.
return "FULL";
});
LOGGER.debug("binlog_row_image={}", rowImage);
return expectedType.equalsIgnoreCase(rowImage);
}
catch (SQLException e) {
throw new DebeziumException("Unexpected error while connecting to the database and looking at BINLOG_ROW_IMAGE mode: ", e);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, do we need to call SHOW GLOBAL VARIABLES LIKE
twice? IMHO only one method is enough, just the name should be updated like isBinlogRowImageFullOrNoBlob
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure i will! |
Hi @Bue-von-hon. Thank you for your valuable contribution. |
2 similar comments
Hi @Bue-von-hon. Thank you for your valuable contribution. |
Hi @Bue-von-hon. Thank you for your valuable contribution. |
…move text and blob columns from create queries if noblob
…ed during snapshot in noblob mode
FYI p.s. I've added a new docker environment for testing, so please understand. |
debezium-connector-mysql/src/test/java/io/debezium/connector/mysql/MySqlConnectorNoblobT.java
Outdated
Show resolved
Hide resolved
@Bue-von-hon Could the whole testsuite pass when running against the new MySQL container image? |
Hi @Bue-von-hon, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
…nnector/mysql/MySqlConnectorNoblobT.java Co-authored-by: Jiri Pechanec <jpechane@redhat.com>
@Naros @jpechane I've resolved all the conflicts. I probably need to change the account information, but the MySqlUniqueDatabase class was just added, so I don't have a reference for it. |
Modification: - Fix(BinlogSnapshotChangeEventSource): Move existing work to a new package. - Fix(BinlogSourceTask): Add noblob condition. - Fix(MySqlConnectorNoBlobIT): Fix name and Add Implement abstract methods to use the UniqueDatabase class.
Hi @Bue-von-hon, thanks for your contribution. Please prefix the commit message(s) with the DBZ-xxx JIRA issue key. |
Hi @Bue-von-hon and @jpechane, I have a radical idea here, and I'm curious what you both think of this. So my understanding here is that when Could we tackle this from the opposite side, rather than trying to manipulate the To me this seems to accomplish the same outcome but without overcomplicating the column mapping process between the SQL statements we get and the in-memory relational models maintained by Debezium. |
@Naros Clever idea! Yes, let's do it this way. |
Hi @Naros @jpechane leave information. It looks like there's going to be a major modification to the EventDispatcher class. (io.debezium.pipeline.EventDispatcher)
However, the event dispatcher class or method being changed must have the noblob configuration information. p.s. of course, inside sourceInfo there is no relevant information yet, I need to add it. |
@Bue-von-hon Do you need to modify the |
resolves DBZ-3643
PTAL @gunnarmorling @Naros