-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Adding (Insert or update if key exists) option to .to_sql #14553 #29636
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
Closed
Closed
Changes from all commits
Commits
Show all changes
125 commits
Select commit
Hold shift + click to select a range
848becc
updated .gitignore to ignore .python-version file
cvonsteg d9f7c23
skeleton framework
cvonsteg f0726b3
update docstring and add TODO
cvonsteg c742f47
scratch functions added
cvonsteg d2a0c2d
generator comment
cvonsteg 686930c
Squashed 'vendor/github.com/V0RT3X4/python_utils/' content from commi…
cvonsteg 1f47d45
Add "python_utils" from "[email protected]:V0RT3X4/python_utils.git@master"
cvonsteg 881a934
Add "python_utils" from "[email protected]:V0RT3X4/python_utils.git@master"
cvonsteg c95de1d
update scratch
cvonsteg 0082347
update scratch
cvonsteg 2d85c70
starting to modify sql.py adding workflow for insert method, and scra…
cvonsteg 0f32cd0
starting to modify sql.py adding workflow for insert method, and scra…
cvonsteg dc70c40
sql.py - methods to get pkey columns, pkey iterator, and almost finis…
cvonsteg a7b8e8e
sql.py - methods to get pkey columns, pkey iterator, and almost finis…
cvonsteg 1d936ef
changed upsert to if_exists option, beginning implementation
cvonsteg 9db2aaa
changed upsert to if_exists option, beginning implementation
cvonsteg 57a246d
all workflow present - now to debugging
cvonsteg c290a78
all workflow present - now to debugging
cvonsteg 35e0fc4
tidy up repo
cvonsteg 6bcd6c2
tidy up repo
cvonsteg 00e6319
Squashed 'vendor/github.com/V0RT3X4/python_utils/' content from commi…
cvonsteg 8d654ca
Add "python_utils" from "[email protected]:V0RT3X4/python_utils.git@master"
cvonsteg 966a95c
Add "python_utils" from "[email protected]:V0RT3X4/python_utils.git@master"
cvonsteg b214a68
added df argument to stop original df from getting deleted by upsert_…
cvonsteg 41938cd
added df argument to stop original df from getting deleted by upsert_…
cvonsteg 78f8e86
tidying up sql.py code and adding helpers to sql_scratch.py
cvonsteg 025b0ef
tidying up sql.py code and adding helpers to sql_scratch.py
cvonsteg bbcf92b
made upsert_delete mask index agnostic
cvonsteg 73fea73
made upsert_delete mask index agnostic
cvonsteg d8b7686
updated docuemntation for to_sql method
cvonsteg 75e16ff
updated docuemntation for to_sql method
cvonsteg 0dfe913
Added basic tests - need to figure out why postgres tests aren't working
cvonsteg 779818a
Added basic tests - need to figure out why postgres tests aren't working
cvonsteg 3fafc95
updated docstrings and added desription to
cvonsteg d79b970
updated docstrings and added desription to
cvonsteg c38f900
remove vendor
cvonsteg 0636332
Merge branch 'sql-upsert' of github.com:V0RT3X4/pandas into sql-upsert
cvonsteg a15fc2f
clean up for PR
cvonsteg 6c44506
wrapped whole insert workflow in transaction to avoid postgres freezing
cvonsteg e409bda
ENH: black file
cvonsteg d35e145
pep8 formatting
cvonsteg 8a57126
black formatting sql.py
cvonsteg 3c308d3
Merge branch 'master' into sql-upsert
cvonsteg d4764dc
Merge branch 'sql-upsert' of github.com:V0RT3X4/pandas into sql-upsert
cvonsteg 4396aa3
Merge branch 'master' into sql-upsert
cvonsteg 2b1c797
reformatted tests, added requested changes, and updated generic docst…
cvonsteg b23f528
reformatted tests, added requested changes, and updated generic docst…
cvonsteg 899da90
line-separated docstring in generc and removed unwatned pattern from …
cvonsteg 8ebc256
postgresql type-o and type-checking in sql tests
cvonsteg baad9e3
reformatting tests
cvonsteg 79ef9c0
remove quotations from postgres queries
cvonsteg d0eb251
postgres formatting
cvonsteg b838ef5
Merge branch 'master' into sql-upsert
cvonsteg 17a1d42
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg a25c2f3
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg f56b53f
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 247bec0
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 51a74a2
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 91c750b
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg f940b42
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 6dedb71
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg e3809a1
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 6d692cd
merging master
cvonsteg efd2382
removed temp=self.frame in _get_index_formatted_dataframe
cvonsteg c3a6a95
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg a0ce842
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 07bc8ca
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 4822ce0
Commenting out tests to confirm that stalled builds are caused by ups…
cvonsteg 0fba1b6
re-enabling to_sql_upsert_keep test
cvonsteg c230d16
re-enabling to_upsert_ignore, whilst disabling to_upsert_keep
cvonsteg 9a7ef9c
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 2a078f6
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg c3c6ed1
clean up flake8 version, re-activate test
cvonsteg 79becdc
remove SQLAlchemyConn tests to confirm hypothesis
cvonsteg 3b6ca76
relaunc TestMySQLAlchemyConn - remove TestMySQLAlchemy
cvonsteg 61d998f
added all MySQL test classes back
cvonsteg e454c35
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg b4c058c
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 2ed4d32
Merge branch 'sql-upsert' of github.com:V0RT3X4/pandas into sql-upsert
cvonsteg c33d536
fix: merge master, resolve conflicts, move inserts out of nested tran…
cvonsteg b3bfbcc
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg fafd646
Merge branch 'master' into sql-upsert
cvonsteg dbee26b
Merge branch 'sql-upsert' of github.com:V0RT3X4/pandas into sql-upsert
cvonsteg 3a04edd
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 0b03df7
Merge branch 'sql-upsert' of github.com:V0RT3X4/pandas into sql-upsert
cvonsteg f2d3596
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 529e5fb
chore: re-order imports
cvonsteg 6d718dc
remove unnecessary sort
cvonsteg 53e3565
undo sort rmv
cvonsteg 1bff71f
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 26c0b0f
feat: use updates instead of deletes
cvonsteg 59c76ac
chore: update tests to reflect new api
cvonsteg 21a87e1
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 286e8b8
feat: use on_conflict update methodology
cvonsteg 780fcea
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 70e0eb1
testing: add tests for single and composite primary keys
cvonsteg d4fc6d4
chore: add api tests for invalid if_exists, on_conflict args
cvonsteg c783496
chore: missing comma in tests
cvonsteg 4f4e9d9
chore: use backticks
cvonsteg e466ff3
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg f2bc121
chore: use add constraint syntax
cvonsteg b9bfedc
tidy up
cvonsteg 43353f5
chore: merge upstream master
cvonsteg 6196c29
chore: merge master
cvonsteg b27449e
fix: add not null constraints to mysql tables
cvonsteg c8c1826
fix: change mysql type from TEXT to VARCHAR for indexing
cvonsteg e28cd9e
chore: merge master
cvonsteg 197e172
chore: use and_ inplace of tuple_
cvonsteg 6ebe9e8
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg b95f6c9
Merge remote-tracking branch 'upstream/master' into sql-upsert
cvonsteg 8fa8e0e
chore: same change for update stmts
cvonsteg 1dee409
fix: primary keys no longer require names getter
cvonsteg 1f9fca7
chore: merge master
cvonsteg 2b30a9e
chore: merge master
cvonsteg a05937e
chore: remove wrong kwarg
cvonsteg 67143ff
chore: add multi-index lookup
cvonsteg 7e1148e
merge master
cvonsteg 11f201f
tests: fix on conflict with non append tests
cvonsteg 7f0b5dd
fix: pass on_conflict into prep_table
cvonsteg e5d5ce7
merge master
cvonsteg 8123cd7
fix: tests working
cvonsteg 26faabe
chore: check metadata bind before reflect
cvonsteg 9260eca
clean up docstrings
cvonsteg ad9f52f
merge: master
cvonsteg a63a77a
merge: master
cvonsteg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2819,12 +2819,15 @@ def to_sql( | |
| schema : str, optional | ||
| Specify the schema (if database flavor supports this). If None, use | ||
| default schema. | ||
| if_exists : {'fail', 'replace', 'append'}, default 'fail' | ||
| if_exists : {'fail', 'replace', 'append', 'upsert_overwrite', 'upsert_keep'},\ | ||
| default 'fail' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this api is conflating two things. isn't upsert by-definition to overwrite? what is the usecase for 'upsert_keep'? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Over the course of this PR I have come to agree that using the term There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that makes sense. |
||
| How to behave if the table already exists. | ||
|
|
||
| * fail: Raise a ValueError. | ||
| * replace: Drop the table before inserting new values. | ||
| * append: Insert new values to the existing table. | ||
| * upsert_overwrite: Overwrite matches in database with incoming data. | ||
| * upsert_keep: Keep matches in database instead of incoming data. | ||
|
|
||
| index : bool, default True | ||
| Write DataFrame index as a column. Uses `index_label` as the column | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may add ambiguity to the
if_existsargument, as it is referring now to two concepts:Would this be better as a separate
if_row_existsparameter?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the documentation says:
So, the documentation should be updated at the least.
However, I think it would be conceptually cleaner to separate table-existence from row collision-checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're making a good point. I will look to incorporate this shortly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On your original proposal, you mentioned using a
methodparameter. That seems like a reasonable name/approach :-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brylie I think it is a good point, but I am less sure about the action required.
I have actually sent a full reply here:
#14553 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the "upsert" method in SQL checks on the primary key, how about name the parameter
if_key_exists?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be explicit, the parameters should be something like:
if_table_existsif_row_existsSince renaming
if_existstoif_table_existsis backwords incompatible, perhaps that rename could be avoided or theif_existsparameter could be marked as deprecated.What is blocking us from adding a new argument for
if_row_exists?