BUG: Allow mapping as parameters for SQL DBAPI2
State: closed

According to the DBAPI2.0 the parameters of the execute method can be a list or a mapping. The code in the master branch assume that this parameter is a list which can break working code. That's a regression compared to the pandas 0.13.1

Closes #6708


Labels:
name: Bug
name: IO SQL
description: to_sql, read_sql, read_sql_query

Comments:
@gpoulin2 Thanks for your very clear issue report and this quick PR!

Would you also like to add a test for this? This could come in https://github.com/pydata/pandas/blob/master/pandas/io/tests/test_sql.py in `TestSQLApi` or in `_TestSQLAlchemy`. Just test eg a `read_sql` statement with both types of params (list/mapping). If you have questions about how the tests are structured, just ask!

BTW, we always advise to create a new branch when working an a new feature/bug fix (and not working on master), and also to `git rebase upstream/master` and not `git merge upstream/master` because now you have a commit `Merge branch 'master' ...` which shouldn't be there

I will put some test when I will have time (probably this weekend). To fix my git mess (sorry still not that used), you prefer that I reopen a new PR that is cleaner?

@gpoulin2 No, a new PR won't be necessary I think. Just remove that last commit and then rebase properly and force push, something like (if you remotes upstream/origin):

```
git reset --hard HEAD^          # remove last commit
git fetch upstream
git rebase upstream/master
git push -f origin
```

I added tests and it makes me realized that the parameter list was also broken so I corrected appropriately.

I just realized that my tests are wrongs. I correct this soon.

Ah, yes, it is indeed a little bit more complicated .. 
Your tests are also failing (eg for the Postrgresql tests, the column names have to wrapped in `"`s).

But I was thinking, maybe we should only test this in the `TestSQLApi` (with sqlite), as the implementation here we want to test is not really database backend dependent? And then we can leave this out in `TestSQLAlchemy`, and don't have to deal with the backend specific issues (certainly if we would add more database backends in the future in the tests).

I think the docstring of `read_sql` could also use an update -> mention that `params` can also be a dict when using named param style.

I first put the test in `TestSQLApi`. The issue is that the API use SQLAlchemy and not the DBAPI and then doesn't fail without my fixes (which it should). I could remove the test for mysql and postgresql but I thought it would be good to have it (make sure that this simple query work with both). I'm a little surprise about the escaping of the columns name, in my project I use mysql with mysql-connector-python and everything work find without escaping (some columns have cap letter).

SO should I remove the mysql and postgresql test or fix them?

@gpoulin2 Ah, if you fetch latest master and rebase, you will see that there are now two versions of those tests: `TestSQLApi` and `TestSQLLegacyApi` (both deriving from `_TestSQLApi`), so the tests in `_TestSQLApi` now run with both sqlalchemy and DBAPI. 

About the case sensitivity of MySQL, I am not an expert, but I think this is platform dependent (see http://dev.mysql.com/doc/refman/5.6/en/identifier-case-sensitivity.html). In any case, for PostgreSQL this is the reason the tests were failing.

I corrected the tests and change the doctring of read_sql. The build succeed now.

For the MySQL issue, I don't think is a case sensitivity issue (my project uses mysql on linux and is case sensitive). I think is more about the connector. mysql-connector-python probably do more job behind the scene than pymysql.

That's possible. In any case, the tests are passing now!

A last question, can you squash down to one or two commits? (https://github.com/pydata/pandas/wiki/Using-Git#fetch-and-then-rebase-interactively-to-squash-reword-fix-otherwise-change-some-commits)

I think everything is good now.

Thanks a lot!

If you want to tackle some other problems, certainly do!