Skip to content
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

supports_statement_cache support #4

Open
clach04 opened this issue May 4, 2022 · 4 comments
Open

supports_statement_cache support #4

clach04 opened this issue May 4, 2022 · 4 comments

Comments

@clach04
Copy link
Member

clach04 commented May 4, 2022

Seeing https://docs.sqlalchemy.org/en/14/errors.html#error-cprf

ingres_sa_demo.py:18: SAWarning: Dialect ingres:pypyodbc will not make use of SQL compilation caching as it does not set the 'supports_statement_cache' attribute to ``True``.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Dialect maintainers should seek to set this attribute to True after appropriate development and testing for SQLAlchemy 1.4 caching support.   Alternatively, this attribute may be set to False which will disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  for row in connection.execute(sqlalchemy.text(query)):
clach04 added a commit that referenced this issue May 19, 2022
Had to set attribute in each sub class as
HasCacheKey._generate_cache_attrs() only checks the sub-class dict for
`supports_statement_cache` attribute.
@clach04
Copy link
Member Author

clach04 commented May 19, 2022

Current code states no caching support, to avoid warning.

@hab6
Copy link
Contributor

hab6 commented Feb 7, 2024

@clach04 Is there still work that needs to be done here? For what it's worth, I ran sa_orm_demo.py using sqlalchemy-ingres 0.0.4.dev1 with various versions of SQLAlchemy (2.0.25, 1.4.51, 1,4,42, 1.4,24) and the code executes successfully with no warnings.

@clach04
Copy link
Member Author

clach04 commented Feb 7, 2024

@hab6 I think this depends on one's perspective, is this issue for:

  1. warnings about caching
  2. actually implementing caching support

I've been mentally treating this ticket as both; 1 is resolved but 2 is outstanding - but that may not be super clear from current write-up/description.

I vote for keeping this ticket open until we tackle number 2. Alternative would be to close and open/link a new ticket for actual caching support and update the title to explicitly mention warnings. What are your thoughts on this?

@hab6
Copy link
Contributor

hab6 commented Feb 7, 2024

@clach04 Thanks for the clarification. I am in agreement to keep the ticket open with the remaining goal of implementing caching support, which is not something I have looked into at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants