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

Allow connections to newer MariaDB databases #227

Closed
wants to merge 1 commit into from

Conversation

Joo200
Copy link
Contributor

@Joo200 Joo200 commented Oct 8, 2023

With MariaDB it's not possible anymore to connect to the database with the mysql database connector.

I already have the mariadb connector added to my server so I don't need it in LWC. It can be added and shaded if needed.

This PR allows to specify the new connection type "mariadb", since mariadb and mysql has the same capabilities there is no other change needed.

@pop4959
Copy link
Owner

pop4959 commented Nov 6, 2023

Thank you for the PR, but I'm not really a huge fan of this for a few reasons. One is that this does not include the MariaDB connector anyway, so generally useless outside of cases where you include it yourself as mentioned. At the same time I am not fond of shading in this connector specifically for the few users that use it. Bukkit only includes the connector for SQLite and MySQL, and LWC has never explicitly supported MariaDB to begin with. I don't believe that hacking this on makes much sense, as this shift implies that MariaDB plans to make further breaking changes, in which case, an entirely separate implementation catered to MariaDB makes far more sense rather than piggybacking off of the mess that is PhysDB.

That said, I don't really see it happening right now, however that could change in the future (for example, if Bukkit decides to change the connectors they include, etc).

Will have to see. 👀

@pop4959 pop4959 closed this Nov 6, 2023
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.

2 participants