Fix #35484 - re-allow sqlalchemy declarative system to use % in read_sql.

[machow]

• closes BUG: read_sql no longer works simply with SqlAlchemy selectables and a quick fix #35484
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

• Adds tests to cover sql cases combining parameters and %.
• Notably, these tests highlight two regressions in v1.1
  • pd.read_sql("SELECT 1 %% 2", engine) fails. See BUG: Regression in pd.read_sql_query between 1.0.5 and 1.1.0 #35871.
  • pd.read_sql(sqlalchemy.sql.text("SELECT 1 % 2"), engine) fails. See BUG: read_sql no longer works simply with SqlAlchemy selectables and a quick fix #35484.
• Fixes second issue by modifying SqlDatabase to not use no_parameter, and wrapping string queries in sql.text() when no parameters are passed.

I was going nuts trying to keep track of all the cases, so tossed them into a table! I didn't have time to explicitly check v1.05, so would be helpful if someone could verify that column (or I may be able to loop back to it)!

query	v1.05	v.1.1	PR	issue
no params, %	❌	✅	✅	#34211 (merged)
no params, %%	✅	❌	❌	#35871
params, %	❌	❌	❌	psycopg/psycopg2#827
params, %%	✅	✅	✅
sqla declarative, %	✅	❌	✅	#35484

[jbrockmendel]

@TomAugspurger any idea who to ping about sqlalchemy review?

[TomAugspurger]

I'm not sure. Perhaps contributors on the issue that caused the regression.

…

On Wed, Sep 23, 2020 at 3:03 PM jbrockmendel ***@***.***> wrote: @TomAugspurger <https://github.com/TomAugspurger> any idea who to ping about sqlalchemy review? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36275 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKAOIV77NXROY6LJSU4PL3SHJICTANCNFSM4RF2DVNQ> .

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

[jbrockmendel]

@john-bodley can you take a look at this

[john-bodley]

I guess this is a known issue with SQLAlchemy (using 1.2.8 per the Pandas specs) using the default connection for certain dialects, i.e.,

from sqlalchemy import create_engine

engine = create_engine('postgresql://postgres:@localhost:5433/postgres', echo=False) 
engine.execute("SELECT 1 % 2")  # TypeError: 'dict' object does not support indexing

versus the raw-connection:

from sqlalchemy import create_engine

engine = create_engine('postgresql://postgres:@localhost:5433/postgres', echo=False) 
cursor = engine.raw_connection().cursor() 
cursor.execute("SELECT 1 % 2")
cursor.fetchone()  # (1,)

Googling the issue it seems taking the sql.text(...) approach as proposed by @machow seems to be a commonly recommended solution.

[jreback]

can you write a whatsnew entry (1.2). i think this would make evaluating this easier.

[jreback]

can you add an explict xfail / message or reason

[jreback]

can you also merge master

[machow]

@john-bodley @jreback thanks for your feedback. I'll have some time after next week--will add a whatsnew entry and bring things up to date.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

[jreback]

closing this as stale, though maybe fixed by #37534 (in 1.2) in any event.

if this is not the case @machow you can create a new PR.