Skip to content

Pull for issue #233 take two - #242

Closed
sodabrew wants to merge 6 commits into
brianmario:masterfrom
sodabrew:master
Closed

sodabrew wants to merge 6 commits into
brianmario:masterfrom
sodabrew:master

Conversation

@sodabrew

Copy link
Copy Markdown
Collaborator

In the previous pull request, I had blown up my repo and it wanted to pull every changeset. Recloned and committed the changesets cleanly.

This adds :local_infile as an option to Mysql2::Client.new()
Refactors three calls to mysql_options around a single utility function.
Exposes a wrapper around mysql_options as a Client instance method.

@thbar

thbar commented Feb 29, 2012

Copy link
Copy Markdown

@sodabrew thank you so much for taking the time to research and package this!

@brianmario I can confirm this pull request allowed me to achieve bulk load while using Mysql2::Client.default_query_options[:connect_flags] |= Mysql2::Client::LOCAL_FILES did not work for me (packet errors).

@sodabrew

Copy link
Copy Markdown
Collaborator Author

This changeset refactors the mysql_options calls to reduce code duplicate, and adds one more option, :local_infile (which would have been a lot of duplicated code until I refactored it).

The new option is used like this:
@mysql = Mysql2::Client.new(:host => @host, :username => @username, :database => @Schema, :local_infile => true)

@brianmario

Copy link
Copy Markdown
Owner

could you update or resubmit this so it's mergable?

Conflicts:
	ext/mysql2/client.h
@sodabrew

Copy link
Copy Markdown
Collaborator Author

Merged your latest code, the one conflict was trivial whitespace at the end of the file ext/mysql2/client.h

Thanks for looking into my pull! Much appreciated!

@thbar

thbar commented Feb 29, 2012

Copy link
Copy Markdown

@thbar

thbar commented Feb 29, 2012

Copy link
Copy Markdown

Question on this one for Rails 3.0.x compatibility (which I try to achieve in adapter_extensions): if this one get merged, will there be a release of mysql2 < 0.3 including the fix?

@brianmario

Copy link
Copy Markdown
Owner

Yeah I try to push out releases for 0.2.x as well as 0.3.x

@thbar

thbar commented Feb 29, 2012

Copy link
Copy Markdown

Great to hear :) thanks!

@sparkertime

Copy link
Copy Markdown

Just wanted to say that we switched to this fork of mysql2 and it has resolved our local_infile issues as described in greater detail in #43 . Looking forward to mysql2 pulling it in!

@allentsai

Copy link
Copy Markdown

sodabrew, thank you for this one. We forked brianmario, and merged this pull in, and our issues have been resolved as well.

@sodabrew

Copy link
Copy Markdown
Collaborator Author

Cool. I've been pretty busy recently, but per Brian's request I'm going to work on adding some more unit tests to make sure this patch is good to go upstream.

@sodabrew

sodabrew commented Apr 4, 2012

Copy link
Copy Markdown
Collaborator Author

I cleaned up the code and moved it to a branch. Can't see where to change the branch in this pull request (probably because it has comments on it already?) so I'll open a new pull request.

@sodabrew sodabrew closed this Apr 4, 2012
@sodabrew

sodabrew commented Apr 4, 2012

Copy link
Copy Markdown
Collaborator Author

(See pull request #252 now.)

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.

5 participants