-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
ENH: 'to_sql()' add param 'method' to control insert statement (#21103) #21199
ENH: 'to_sql()' add param 'method' to control insert statement (#21103) #21199
Conversation
Hello @schettino72! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on May 30, 2018 at 03:40 Hours UTC |
For 200,000 rows, 20 columns, chunksize 1000. Running python 3.6, postgres:
|
Does In the past, we always have limited the pandas implementation to sqlachemy core constructs that are backend agnostic, within its limitations. |
No. I do not have enough knowledge of various DB systems. I just know it works with Postgresql and does NOT work with SQLite. I think the main point is to allow the insertion method to be modified without requiring any monkey-patching. I can drop the COPY handling from the patch if you guys wish... I included it to make sure the API was good enough to really use other methods. As the default and multi-value implementation are very similar. IMO it does not hurt to include some code for a specific backend as long as the user needs to explicitly choose to do so, but I understand you guys dont want the burden to support code for every different backend. |
Codecov Report
@@ Coverage Diff @@
## master #21199 +/- ##
==========================================
+ Coverage 91.84% 91.84% +<.01%
==========================================
Files 153 153
Lines 49506 49538 +32
==========================================
+ Hits 45467 45499 +32
Misses 4039 4039
Continue to review full report at Codecov.
|
So as I said on the issue (#21103 (comment)), I think a good start might be to have this option and allow users to pass a custom function, but not to actually implement such a 'copy' method ourselves (but it is a good test case). |
sure. no problems. I will remove the "copy" implementation. |
Be sure to keep it as a test case in the tests, and maybe also a good example for the documentation |
for 0.23.1 ? |
At least something, if not this, then a revert of the original PR. |
fair enough. there are 2 or 3 issues open in 0.23.1. pls adjust to what makes sense. |
I opened the PR to simply revert: #21355, we can keep this PR for 0.23.2 or 0.24.0 |
66cec62
to
abfac97
Compare
Sorry, I rebased this to master and force pushed... It seems github decided to automatically close this. Re-opened as #21401 |
Also revert default insert method to NOT use multi-value.
This is WIP. I would like to gather feedback on API change before going on.
Sample file for performance benchmarking
git diff upstream/master -u -- "*.py" | flake8 --diff