Implementation for insert method for SQLAlchemy backends

[harsharaj96]

Closes #2613

The implementation is similar to what is currently implemented for the Impala database. Please have a look and merge it if you think this helps your requirement. I have also added commentary to what will work and what won't in the code so that enhancements can be made to it later on. Thanks.

[pep8speaks]

Hello @harsharaj96! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-06-03 00:39:31 UTC

[datapythonista]

Thanks for the contribution @harsharaj96. I'll have a closer look later, but to give a bit more context. SQLite, PosgreSQL, and MySQL are the backends that use SQLAlchemy internally. Didn't check if PostgreSQL and MySEL have the insert method, but I think it should be possible to have a single insert for the 3 of them.

Do you mind doing some more researh on it?

Also, for this PR we'll need:

• Add a test for the new insert method
• A a line in the release notes for this change
• Optionally, in the tutorials we've got a part where we comment that inserting is possible in Ibis, but we don't have an example because it doesn't work in Ibis. Adding an example there would be nice too (we can surely do it in a follow up PR, don't worry if you don't feel like doing it here).

[datapythonista]

Also, before we can merge, you'll have to run black . and isort ., which will make some automatic changes to the files, and commit those changes, so the CI is green.

[harsharaj96]

Hi @datapythonista, Firstly, thanks for your response. Based on lookup, PostgresSQL and MySQL clients in this repo don't have insert methods implemented. Having a common insert method implemented in the SQLAlchemy sounds good but I am confused about how it is different from the load_data method that is already implemented which uses a dataframe to load data into a table.

[harsharaj96]

I have run the black . and isort . commands and made changes to the file. Seems like the CI checks also passed now. Thanks for the inputs.

[harsharaj96]

Regarding the test case for the insert method: I haven't totally understood the other test cases for SQLite, like table creation and loading data. I'll try to add a test case once I understand them. If you can point me to any reference example usages or starting points, it would be great. Thanks in advance @datapythonista.

[datapythonista]

You can check the clickhouse backend as reference. In the client.py and test_client.py you have an implementation and a test.

load_data translates to SQL LOAD DATA while insert translate to SQL INSERT. Ibis just wraps that, and let users choose which SQL statement is more convenient.

Besides the test, you are also missing here an entry in docs/source/release/index.rst.

Thanks!

[datapythonista]

@harsharaj96 do you have time to add the test and the entry in the release notes?

[harsharaj96]

@harsharaj96 do you have time to add the test and the entry in the release notes?

Hi @datapythonista , I'm so sorry, for some reason I missed your previous msg/mail. I'll try to work on it today and let you know. Thanks.

[harsharaj96]

Hi @datapythonista,

I have now added test cases for insert (two functions with allowed functionalities, insert using a dataframe and insert using a table). I have also added the entry in the release note (in the sqlite_insert PR branch itself). Please take a look and if you think everything is fine then please merge this PR.

Thanks!

[jreback]

pls also rebase on master & remove the extra file.

[harsharaj96]

Hi @datapythonista, @jreback,

Updated the code with your suggestions and requests. Please check and revert if I'm missing something.

Thanks!

[datapythonista]

Checked quickly and changes look great now. Let me know when this is ready for a last review.

[harsharaj96]

Hi @datapythonista, All the suggested changes are worked on. You can do the review and inform me if I'm missing anything or if something more is required. Thanks!

[harsharaj96]

Hi @datapythonista, All changes are fixed/updated. Please confirm and let me know. Thanks a lot!

[harsharaj96]

Hi @datapythonista, Did you get a chance to look at the changes?

[harsharaj96]

Hi @datapythonista, Added new changes as well. Have provided individual comments to a few of these changes please check them for more clarity. Thanks!

[datapythonista]

@harsharaj96 can you merge master into your branch please? Just realised the release notes are conflicting.

[datapythonista]

Never mind, forgot I could quickly do it myself from the GitHub interface.

[jreback]

will look

[harsharaj96]

Hi @jreback, Added new changes as well. Have provided individual comments to a few of these changes please check them for more clarity. Thanks!

[harsharaj96]

Hi @datapythonista / @jreback , Can you please check and approve the changes so that checks can be performed? I've added a couple of changes based on the last failing checks. Just wanted to see if they are working fine. Thanks!

[datapythonista]

@harsharaj96 in case you haven't seen it, tests run and failed.

[harsharaj96]

@datapythonista , So sorry, might have missed it. Added a main bug fix. Please approve for a re-run of tests.

[harsharaj96]

Hi @datapythonista, Added a couple of more bug fixes, please check and make the workflow run the checks and tests. Thanks!

[harsharaj96]

Hi @datapythonista & @jreback, All the checks and tests have passed. Please take a look and approve the pull request if you think everything is fine. Thanks a lot for your inputs and suggestions. Cheers!

[harsharaj96]

Hi @datapythonista & @jreback,
Please go through the changes and let me know if I can help you with anything else.
Thanks!

[harsharaj96]

Hi @datapythonista, I've added new changes based on your suggestions. Added a few replies to some of the comments and implemented a few. Please take a look and let me know. Thanks!

[jreback]

i will look soon

[harsharaj96]

Hi @jreback & @datapythonista, I've added new changes based on your suggestions. Added a few replies to some of the comments and implemented a few. Please take a look and let me know. Thanks!

[harsharaj96]

Hi @jreback, The tests which are failing are not related to the PR we are working on. These failing tests are due to a few commits on master branch. Thanks!

[jreback]

thanks @harsharaj96

[datapythonista]

Thanks a lot for all your work on this @harsharaj96, great job!