Conversation
MySQL TIME is a signed duration. The leading bit of the packed TIME2 payload carries that sign, and MySQL stores a negative value as the two's complement of the whole payload, fractional bytes included. The deserializer sliced hour, minute and second straight out of the raw value, so every negative TIME came back as a large positive one: -00:00:01 decoded to 3686643000 rather than -1000. Take the sign off the whole payload before slicing any field, and negate the result. Positive values, including the ones above 24 hours that TIME allows, are unchanged. Fixes osheroff#154.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #154.
MySQL
TIMEis a signed duration in-838:59:59 .. 838:59:59. The leading bit of the packed TIME2 payload carries that sign — the comment indeserializeTimeV2already documents it — but the code sliced hour, minute and second straight out of the raw value:So every negative
TIMEcame back as a large positive one.-00:00:01decoded to3686643000, which is the number in #154.What the fix does
MySQL stores a negative TIME as the two's complement of the whole packed payload, fractional bytes included, so the sign has to come off before any field is sliced out and the fractional bytes have to be part of that same complement. This reads all
3 + (meta + 1) / 2bytes at once, takes the complement when the leading bit is clear, slices, and negates the result.That the fraction shares the complement is not a detail I could take on faith, so I checked it against a live server.
mysql:8.0.46, row-format binlog, one table withTIME,TIME(3)andTIME(6)columns:The integer part is
fbin the fractional rows andfcin the plain one — MySQL'sintpart++ / frac -= 0x1<<nfor a negative value with a fraction. Complementing only the first three bytes would get those rows wrong; complementing the whole payload gets them right.Tests
Two cases in
AbstractRowsEventDataDeserializerTest, using payloads taken from that binlog rather than hand-derived, covering both signs and all three fractional widths:16:08:0481 02 04100:08:0486 42 0400:00:0080 00 00-00:00:017f ff ff-16:08:047e fd fc-16:08:04.2507e fd fb f6 3c-16:08:04.2500007e fd fb fc 2f 70Reverting the change fails both new tests with
expected [-1000] but found [3686643000]andexpected [-58084250] but found [3628565303], and leaves the existingtestFromgreen.mvn test— 69 tests, all passing.Positive values are untouched, including those above 24 hours, which
TIMEallows and which already decoded correctly.Scope
Debezium is not affected:
RowDeserializersindebezium-connector-binlogoverrides all three row deserializers with its own TIME2 implementation that already handles the sign. That is most likely why this issue stalled — the reporter was running this library's own tests while the check in the thread was made through Debezium.