DBZ-3643 Fix to support NOBLOBs among binlog_row_image attributes. #5319
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. |
Bue-von-hon
force-pushed
the
DBZ-3643
branch
from
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.
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.
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
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