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 mysql2 support #18

Merged
merged 5 commits into from
Oct 27, 2023
Merged

Add mysql2 support #18

merged 5 commits into from
Oct 27, 2023

Conversation

prog-supdex
Copy link
Contributor

@prog-supdex prog-supdex commented Oct 19, 2023

What is the purpose of this pull request?

Added mysql2 support

Is there anything you'd like reviewers to focus on?

Added a new Mysql Adapter
Mysql doesn't support ON CONFLICT (but has ON DUPLICATE KEY UPDATE) and doesn't have RETURNING ID

So, I used ActiveRecord::Base.connection.update(sql) for getting amount of updated rows
Also, Mysql doesn't understand EXCLUDED, so we need to use some method wrap_column_name, which will be defined in every adapter. Because, Mysql understands VALUES(here_column_names), but Postgresql understands EXCLUDED.here_column_name

Checklist

  • [- ] I've added tests for this change
  • [+] I've added a Changelog entry
  • [+] I've updated a documentation

@prog-supdex prog-supdex marked this pull request as draft October 20, 2023 17:53
@prog-supdex prog-supdex force-pushed the ci_mysql branch 8 times, most recently from 94eda5f to 6096ad7 Compare October 23, 2023 09:34
@prog-supdex prog-supdex changed the title Add running CI with mysql2 adapter Add mysql2 support Oct 23, 2023
@prog-supdex prog-supdex marked this pull request as ready for review October 23, 2023 09:57

def initialize(klass, current_adapter_name)
@klass = klass
@current_adapter_name = current_adapter_name
Copy link
Member

Choose a reason for hiding this comment

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

We don't this to be a part of the adapter state; we only use it in the apply? method, so let's pass current adapter as an argument to #apply? instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks
got it!
done!

@klass = klass
@current_adapter_name = current_adapter_name
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@palkan
Copy link
Member

palkan commented Oct 27, 2023

Thanks!

@palkan palkan merged commit 500a35e into evilmartians:master Oct 27, 2023
10 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.

2 participants