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

Remove SQL driver auto-detection. #142

Open
jmalloc opened this issue Mar 21, 2023 · 0 comments
Open

Remove SQL driver auto-detection. #142

jmalloc opened this issue Mar 21, 2023 · 0 comments
Milestone

Comments

@jmalloc
Copy link
Member

jmalloc commented Mar 21, 2023

I'm referring to this logic.

It probes the database to work out which driver to use (MySQL, PostgreSQL, etc). It does this by making SQL queries that only work on specific databases. This means some queries fail under normal operation which has caused confusion in the past.

I am suggesting that the choice of driver should be explicit. There are several ways we could approach this:

  1. Replace New() with database-specific alternatives
  2. Always pass a driver to New()
  3. Add a Driver() method to the sqlprojection.MessageHandler interface

1 and 2 are much the same from a usability standpoint. Of these two I'd prefer 2, as it allows the user to supply a third-party driver implementation.

3 has the benefit of keeping the driver selection alongside the handler implementation. IMO this option makes the most sense because this where the application's database-specific code is.

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

No branches or pull requests

1 participant