BUG: Allow mapping as parameters for SQL DBAPI2

[ghost]

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

[jorisvandenbossche]

@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

[ghost]

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?

[jorisvandenbossche]

@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

[ghost]

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

[ghost]

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

[jorisvandenbossche]

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).

[jorisvandenbossche]

The column names are case-sensitive, but eg Postgresql converts everything to lower case, unless you specify it as eg "Name" (or Name for mysql)

[jorisvandenbossche]

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.

[ghost]

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?

[jorisvandenbossche]

@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.

[ghost]

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.

[jorisvandenbossche]

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)

[ghost]

I think everything is good now.

[jorisvandenbossche]

Thanks a lot!

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