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

Add function to parse 'extradata’ in rows event #817

Merged

Conversation

chungeun-choi
Copy link
Contributor

@chungeun-choi chungeun-choi commented Aug 31, 2023

Hello, I am a developer contributing to py-mysql-replication.

While checking the source code, we developed it because the parsing-related part of 'rows event' to 'extra data' was not developed in the go-mysql package
(This feature has been added since Mysql 8.0.16)

Please leave a comment if there are any modifications or additions to the code!

Developed

  • Added function 'decodeExtraData' that in file 'replication.row_event.go’

  • The function is executed when the DecodeHeader function is executed and the dataLen value is greater than 2

  • The extra data to be parsed is as follows

    	  +----------+--------------------------------------+
            |type_code |        extra_row_ndb_info            |
            +--- ------+--------------------------------------+
            | NDB      |Len of ndb_info |Format |ndb_data     |
            | 1 byte   |1 byte          |1 byte |len - 2 byte |
            +----------+----------------+-------+-------------+
    
            In case of INSERT/DELETE
            +-----------+----------------+
            | type_code | partition_info |
            +-----------+----------------+
            |   PART    |  partition_id  |
            | (1 byte)  |     2 byte     |
            +-----------+----------------+
    
            In case of UPDATE
            +-----------+------------------------------------+
            | type_code |        partition_info              |
            +-----------+--------------+---------------------+
            |   PART    | partition_id | source_partition_id |
            | (1 byte)  |    2 byte    |       2 byte        |
            +-----------+--------------+---------------------+

Copy link
Collaborator

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

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

Hi, can you mimic

func TestTableMapOptMetaVisibility(t *testing.T) {
to dump some binlog content and test the new fields work as expected?

@@ -878,7 +878,13 @@ type RowsEvent struct {
Flags uint16

// if version == 2
ExtraData []byte
// Use when DataLen value is greater than 2
NdbLength uint8
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems it's complaining that the type of variables are not vertically aligned.

@chungeun-choi
Copy link
Contributor Author

Thank you for your review! @lance6716
And I'll add a test code

@chungeun-choi
Copy link
Contributor Author

chungeun-choi commented Sep 4, 2023

Hello , @lance6716

I've made some significant improvements to codebase to ensure the proper parsing of 'extradata' within the 'rows event' functionality. Here's what this PR includes:

  • Fix Incorrect Byte Parsing in NDB Info from Extra Data
  • Add Test Code to Validate 'extradata' Parsing in 'rows event'

Please review the changes, and your feedback is highly appreciated.

Copy link
Collaborator

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

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

rest lgtm

replication/row_event.go Outdated Show resolved Hide resolved
replication/row_event.go Outdated Show resolved Hide resolved
replication/row_event.go Outdated Show resolved Hide resolved
@chungeun-choi
Copy link
Contributor Author

I appreciate your review. and I can fix the three parts you mentioned!
Please check it again after modifying it.

@chungeun-choi
Copy link
Contributor Author

fixed the three parts you mentioned :)

pos += 1
e.NdbData = data[pos : pos+ndbLength-2]
case ENUM_EXTRA_ROW_INFO_TYPECODE_PARTITION:
if e.eventType == UPDATE_ROWS_EVENTv2 || e.eventType == PARTIAL_UPDATE_ROWS_EVENT {
Copy link
Collaborator

@lance6716 lance6716 Sep 7, 2023

Choose a reason for hiding this comment

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

I'm still not sure about it. MySQL enables it for UPDATE_ROWS_EVENT, UPDATE_ROWS_EVENT_V1 and PARTIAL_UPDATE_ROWS_EVENT, and we do it for UPDATE_ROWS_EVENTv2 and PARTIAL_UPDATE_ROWS_EVENT. I remember UPDATE_ROWS_EVENT and UPDATE_ROWS_EVENT_V1 has been deprecated so we really don't need to care about them. I'll check it later.

And it's helpful that you can also help me check and provide some reference 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

As you says in PR description, this feature is added in v8.0.16, and log_bin_use_v1_row_events is deprecated in v8.0.18, so we should also consider UPDATE_ROWS_EVENT_V1.

And MySQL's UPDATE_ROWS_EVENT (31) is our UPDATE_ROWS_EVENTv2

@lance6716 lance6716 merged commit ad9a6e6 into go-mysql-org:master Sep 8, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants