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 support for MariaDB #9409

Merged
merged 17 commits into from
Jan 11, 2022
Merged

Add support for MariaDB #9409

merged 17 commits into from
Jan 11, 2022

Conversation

jakule
Copy link
Contributor

@jakule jakule commented Dec 14, 2021

This PR add support for MariaDB. Binary protocol is the same as MySQL, so there is no changes in the backend logic. Default client for MySQL and MariaDB from now on will be mariadb or mysql if the first one is not available in the system.
I also added tests for all CLI tools run by tsh db connect.

closes #8904

@jakule jakule force-pushed the jakule/mariadb-support branch from 461eb95 to 47fb525 Compare December 16, 2021 02:10
@jakule jakule marked this pull request as ready for review December 16, 2021 21:19
@jakule jakule requested review from r0mant and removed request for zmb3 and ptgott December 16, 2021 21:22
tool/tsh/db.go Outdated
// Check if mysql comes from Oracle or MariaDB
mysqlVer, err := c.exe.RunCommand(mysqlBin, "--version")
if err != nil {
// Looks like incorrect mysql installation or mysql binary is missing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think this comment will make more sense if placed in the getMySQLCommand since it is where the decision to go and use Oracle's version in case of error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As @r0mant suggested I changed the logic a bit. I changed the comment as now we have an explicit check for mysql.

tool/tsh/db.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

lgtm with a few nits

docs/pages/database-access/guides/rds.mdx Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
tool/tsh/db.go Outdated Show resolved Hide resolved
@jakule jakule force-pushed the jakule/mariadb-support branch from f7ff17a to a010afb Compare January 5, 2022 23:12
@jakule
Copy link
Contributor Author

jakule commented Jan 7, 2022

@klizhentas Can you take a look on the docs changes? Thanks.

@jakule jakule merged commit bae67b3 into master Jan 11, 2022
@jakule jakule deleted the jakule/mariadb-support branch January 11, 2022 01:12
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.

Self-hosted MariaDB support
4 participants