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

MDEV-35905: Revert CONC-710 induced mysqlbinlog FIXME #3782

Open
wants to merge 1 commit into
base: 11.4
Choose a base branch
from

Conversation

bnestere
Copy link
Contributor

@bnestere bnestere commented Jan 22, 2025

mariadb-corporation/mariadb-connector-c@1a2ed3f
and
mariadb-corporation/mariadb-connector-c@78e56a7
of CONC-710 mistakenly moved Item_result from mariadb_com.h to
mariadb_rpl.h thinking it was only used by Connector/C's binlog API;
however it is also used by mysqlbinlog in the server. This broke
server compilation.

e41145f
provided a temporary fix to re-define Item_result in mysqlbinlog.cc
to get the build working

mariadb-corporation/mariadb-connector-c@75d381f
reverted the change from CONC-710 in libmariadb to move Item_result
back to mariadb_com.h

This patch updates the server libmariadb version to include the
latest connector-c fix, and reverts the old temporary fix, so
mysqlbinlog uses Item_result from mariadb_com.h again.

It was initially tested locally by compiling and running MTR with an
updated libmariadb submodule, and then verified via CI runs,
e.g. this build

Copy link
Contributor

@dr-m dr-m left a comment

Choose a reason for hiding this comment

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

@vaintroub raised some concerns in MDEV-35905, which may need to be addressed first.

You’re not using the pull request template. In this case, it would have been interesting to read how this was tested, such as, pointing to a CI run that includes an up-to-date libmariadb submodule.

The commit message as well as the pull request description should refer to the changes in other GitHub repositories in the usual GitHub notation, for example, mariadb-corporation/mariadb-connector-c@75d381f (mariadb-corporation/mariadb-connector-c@75d381ffb2bd183912759819bd60d685c015eeb9).

mariadb-corporation/mariadb-connector-c@1a2ed3f
and
mariadb-corporation/mariadb-connector-c@78e56a7
of CONC-710 mistakenly moved Item_result from mariadb_com.h to
mariadb_rpl.h thinking it was only used by Connector/C's binlog API;
however it is also used by mysqlbinlog in the server. This broke
server compilation.

e41145f
provided a temporary fix to re-define Item_result in mysqlbinlog.cc
to get the build working

mariadb-corporation/mariadb-connector-c@75d381f
reverted the change from CONC-710 in libmariadb to move Item_result
back to mariadb_com.h

This patch updates the server libmariadb version to include the
latest connector-c fix, and reverts the old temporary fix, so
mysqlbinlog uses Item_result from mariadb_com.h again.

Reviewed By:
============
Marko Mäkelä <[email protected]>
@bnestere
Copy link
Contributor Author

The commit message as well as the pull request description should refer to the changes in other GitHub repositories in the usual GitHub notation,

Ah, I didn't know about that, thanks for the pointer! I've re-written the commit message (and will edit the PR description accordingly, along with test info)

@vuvova
Copy link
Member

vuvova commented Jan 26, 2025

it's already fixed in bb-11.4-branch

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

Successfully merging this pull request may close these issues.

4 participants