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

Mysqlconnector3919: Fix auto update issues that arise in newer versions of MariaDb #4003

Merged
merged 17 commits into from
Jul 1, 2023

Conversation

alissa0
Copy link
Contributor

@alissa0 alissa0 commented May 13, 2023

When setting up a new dev environment recently I encountered the same issue described in #3919

Some cursory research lead me to a discussion about this on Stack Overflow

I decided to implement MySqlConnector as an alternative to MySql.Data, but it wasn't a drop-in replacement as it lacks support for the MySqlScript methods (including the StatementExecuted event) so some light refactoring had to be done.

@gmriggs
Copy link
Collaborator

gmriggs commented May 13, 2023

Is this a widely-recognized solution, or just switching to another library that doesn't happen to have the issue?

IIRC, I had an issue within the past year when upgrading an existing ACE DB to a newer MariaDB version. I think it was maybe 10.6, but it was over a year ago, and I can't remember the exact version. Anyway, there was an issue with utf8mb4 where it couldn't handle the older table format anymore. I mostly got utf8mb4 errors, but I think I might have seen a similar DBNull error in the mix as well.

I'm not sure if it's the same issue, but I ask because I found a bunch of varied solutions at the time. Some of them involved adjusting or switching MySqlConnector versions, but the cleanest solution I found was to simply upgrade my existing DB to the newer utf8mb4 format.

@gmriggs gmriggs requested a review from Mag-nus May 13, 2023 18:27
@alissa0
Copy link
Contributor Author

alissa0 commented May 13, 2023

I spent a little bit of time trying to find additional information about this but I haven't really been able to find any extensive discussion around this issue aside from the stack overflow thread I linked initially :(
I was setting this up under a fresh archlinux installation in WSL2. As far as I can tell, it's using utf8mb4 across the board and I'm running into the issue around collations having NULL ids with the current driver.

So in short I don't think this is a widely recognized solution but the package does seem to be a widely utilized open source alternative to the one provided by Oracle. However, I can understand the hesitation surrounding changing something like this so if there's any additional info I can provide or research avenues I can explore let me know and I'll be happy to help!

@gmriggs
Copy link
Collaborator

gmriggs commented May 14, 2023

Okay, this sounds like a different issue than the upgrade one I had last year then.

@Mag-nus
Copy link
Member

Mag-nus commented May 18, 2023

I haven't spent enough time to give a proper review, but it just feels wrong going from what is the de facto standard for .NET to a 3rd party lib to fix an issue.

What I also found interesting is what we have in there now:

<Target Name="ChangeAliasesOfStrongNameAssemblies" BeforeTargets="FindReferenceAssembliesForReferences;ResolveReferences">
    <ItemGroup>
      <ReferencePath Condition="'%(FileName)' == 'MySqlConnector'">
        <Aliases>MySqlConnectorAlias</Aliases>
      </ReferencePath>
    </ItemGroup>
  </Target>

That also is worth a review.

Copy link
Member

@LtRipley36706 LtRipley36706 left a comment

Choose a reason for hiding this comment

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

Confirmed this change fixes issue with MariaDB 10.10+ as well as appears to work with latest MySQL.

Both database engines perform correct out of box setup and installation.

@LtRipley36706 LtRipley36706 merged commit 974e3ac into ACEmulator:master Jul 1, 2023
2 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

4 participants