-
Notifications
You must be signed in to change notification settings - Fork 227
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 table upsert support #1660
Add table upsert support #1660
Conversation
…what actually needs to get updated
…inal tweaks to test code soon to paramaterize for pytests
Co-authored-by: Fokko Driesprong <[email protected]>
cc reviewers from the other PR (@Fokko / @corleyma / @tscottcoombes1 / @marcoaanogueira) and the original PR author @mattmartin14 |
Looks great to me. Thanks for getting this over the goal line. I'm excited to get this into other's hands. |
Hey @kevinjqliu, just a note after all the changes are done, it may be best to manually squash the commits and make sure Matt is the author for that...not sure what GitHub will do with Matt's treasure trove of commits move across PRs and your recent ones. |
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 looks great! Thanks again @mattmartin14
Regarding @bitsondatadev's comment, when you do a squash and commit, you'll see multiple authors, example can be found here. When you do a git blame, it will point out Matt himself (except the lines that @kevinjqliu touched) :)
Good enough for me 😂. I'm famous!!! |
@Fokko @kevinjqliu - should I go ahead and close the old PR now? |
@mattmartin14 Yes, please go ahead. Thanks everyone for driving this, @mattmartin14 in particular! |
Great work @mattmartin14 👏🏻 👏🏻 If I'm not mistaken this is your first PR merged in any open source project correct? Not a bad first feature! Mine was adding array types for the Elasticsearch connector in Trino...you know this because in the docs example, the You also know this was acquired later by Presto since they copied it to their docs 😈. |
Correct, this was my first PR to open source; I'm 1/1 😁. And I learned a lot! I was unaware that you added the array type to trino. That is some cool stuff. Thanks again all, |
Thanks everyone for getting this over the finish line! Upsert has been a long awaited feature. I'm excited to include this as part of the upcoming 0.9.0 release. This is truly a team effort and really demonstrates the power of open source and working in the open. Cheers! |
Closes #402
This PR adds the
upsert
function to theTable
class and supports the following upsert operations:This PR is a remake of #1534 due to some infrastructure issues. For additional context, please refer to that PR.