Skip to content
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

[mysql-cdc] Fix table metadata field type 0 when parsing binlog event #2785

Closed
wants to merge 6 commits into from

Conversation

EchoLee5
Copy link
Contributor

@EchoLee5 EchoLee5 commented Nov 30, 2023

As described by #2784, in specific scenarios will appear table metadata field type 0 when parsing binlog event
When reading the sign bit of a numerical type, use the packed to determine the number of bytes to read to avoid the possibility of misreading due to errors in calculations.

@leonardBang
Copy link
Contributor

Thanks @EchoLee5 for the contribution, the analysis looks good to me, could you fix the CI failure firstly ?

Copy link
Contributor

@ruanhang1993 ruanhang1993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Could you please add some tests to cover this?

@EchoLee5
Copy link
Contributor Author

EchoLee5 commented Dec 4, 2023

Thanks @EchoLee5 for the contribution, the analysis looks good to me, could you fix the CI failure firstly ?

I've solved it, thank you.

@EchoLee5
Copy link
Contributor Author

EchoLee5 commented Dec 4, 2023

Thanks for the contribution. Could you please add some tests to cover this?

I have added unit tests.

Copy link
Contributor

@ruanhang1993 ruanhang1993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Left some comments.

* packed.
*
* <p>Line 123: The length of the {@link TableMapEventMetadataDeserializer#readBooleanList} is
* determined by the length * of the packet header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct this description. There is a useless * in it.

TableMapEventMetadata tableMapEventMetadata =
metadataDeserializer.deserialize(new ByteArrayInputStream(data), 10, 8);

assertThat(tableMapEventMetadata.getSignedness().get(1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result is not checked here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add some ITCase to reproduce the problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruanhang1993 thank you for your review, I've modified the description and unit tests, the bytes used in my test case can be reproduced before modifying the code. I also mentioned the conditions for reproduction in #2784

@ruanhang1993
Copy link
Contributor

Duplicate to #2682

@ruanhang1993
Copy link
Contributor

@EchoLee5 Thanks for your contribution.
Your changes have been merged into commit f970444 and merged into master branch. Close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants