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

support SQLAlchemy 1.4.28+ #385

Closed
tswast opened this issue Dec 29, 2021 · 14 comments · Fixed by #499
Closed

support SQLAlchemy 1.4.28+ #385

tswast opened this issue Dec 29, 2021 · 14 comments · Fixed by #499
Assignees
Labels
api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tswast
Copy link
Collaborator

tswast commented Dec 29, 2021

sqlalchemy.exc.SAWarning: Dialect bigquery:bigquery 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)

https://source.cloud.google.com/results/invocations/98c79c9a-dc03-49b5-bba9-db389866d284/targets/cloud-devrel%2Fclient-libraries%2Fpython%2Fgoogleapis%2Fpython-bigquery-sqlalchemy%2Fpresubmit%2Fpresubmit/log

More info here: https://sqlalche.me/e/14/cprf

Also, I recall caching may have been the root cause in #368 ?

@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API. label Dec 29, 2021
@tswast tswast added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Dec 29, 2021
@tswast tswast changed the title warning with SQLAlchemy 1.4 support SQLAlchemy 1.4.26+ Dec 30, 2021
@tswast
Copy link
Collaborator Author

tswast commented Dec 30, 2021

#384 capped the maximum version of SQLAlchemy due to these failing compliance tests. Version 1.4.25 is the last known working version, but we just know that it started failing between 1.4.25 and 1.4.29.

It's not clear if something changed with SQLAlchemy that started this breakage or the compliance tests need additional configuration or a bit of both.

See #386 for more information.

@tswast tswast added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Dec 30, 2021
@tswast
Copy link
Collaborator Author

tswast commented Jan 13, 2022

I know we won't be able to support 1.4.29, but I'll try to expand support as much as we can in #391

@tswast
Copy link
Collaborator Author

tswast commented Jan 14, 2022

1.4.27 has fewer failures, but still fails

___________ ExpandingBoundInTest_bigquery+bigquery.test_typed_str_in ___________
Traceback (most recent call last):
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/testing/suite/test_select.py", line 1147, in test_typed_str_in
    self._assert_result(
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/testing/suite/test_select.py", line 1020, in _assert_result
    eq_(conn.execute(select, params).fetchall(), result)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1289, in execute
    return meth(self, multiparams, params, _EMPTY_EXECUTION_OPTS)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/elements.py", line 325, in _execute_on_connection
    return connection._execute_clauseelement(
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1473, in _execute_clauseelement
    compiled_sql, extracted_params, cache_hit = elem._compile_w_cache(
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/elements.py", line 533, in _compile_w_cache
    compiled_sql = self._compiler(
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/elements.py", line 555, in _compiler
    return dialect.statement_compiler(dialect, self, **kw)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/sqlalchemy_bigquery/base.py", line 192, in __init__
    super(BigQueryCompiler, self).__init__(dialect, statement, *args, **kwargs)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 776, in __init__
    Compiled.__init__(self, dialect, statement, **kwargs)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 451, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 486, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/visitors.py", line 82, in _compiler_dispatch
    return meth(self, **kw)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 1590, in visit_textclause
    BIND_PARAMS.sub(
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 1574, in do_bindparam
    return self.process(textclause._bindparams[name], **kw)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/compiler.py", line 486, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/.nox/compliance-3-10/lib/python3.10/site-packages/sqlalchemy/sql/visitors.py", line 82, in _compiler_dispatch
    return meth(self, **kw)
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/sqlalchemy_bigquery/base.py", line 525, in visit_bindparam
    assert_(self.__expanded_param(param), f"Unexpected param: {param}")
  File "/tmpfs/src/github/python-bigquery-sqlalchemy/sqlalchemy_bigquery/base.py", line 68, in assert_
    raise AssertionError(message)
AssertionError: Unexpected param: (__[POSTCOMPILE_q])
- generated xml file: /tmpfs/src/github/python-bigquery-sqlalchemy/compliance_3.10_sponge_log.xml -

https://source.cloud.google.com/results/invocations/2cc08719-cc62-45a9-a5f0-e0260888937c/targets

@tswast tswast changed the title support SQLAlchemy 1.4.26+ support SQLAlchemy 1.4.27+ Jan 14, 2022
@tswast tswast changed the title support SQLAlchemy 1.4.27+ support SQLAlchemy 1.4.28+ Feb 22, 2022
@Aegistov
Copy link

My team is currently on SQLAlchemy 1.4.31 and we're planning to integrate with sqlalchemy-bigquery — based on the warning in the OP, it doesn't seem like there'll be actual breakage with using 1.4.31 it would just mean poor performance for querying bigquery tables with the ORM while this issue is being worked on. Is that right?

@tswast
Copy link
Collaborator Author

tswast commented Mar 18, 2022

It does seem to at least mostly work. The system tests we had written were passing, but the compliance tests are much more thorough, so I've capped the version until we can figure that out.

@woidda
Copy link

woidda commented Apr 28, 2022

Do you have any news on that by now?

It blocks me (and I guess others) from upgrading SQLAlchemy and pyarrow.

@woidda
Copy link

woidda commented May 25, 2022

@tswast I guess this is not your top priority right now, but do you have any news on when or if this is going to be fixed?

Right now there are two new versions released Would be great to be able to update pyarrow in the projects I am using python-bigquery-sqlalchemy.

@tswast
Copy link
Collaborator Author

tswast commented May 25, 2022

Unfortunately, you're right that it's not a top priority at the moment. If you or someone makes some progress with the compliance test suite on later versions, I'm sure a PR would be much appreciated. See #408

@BrettMoan
Copy link

BrettMoan commented Aug 29, 2022

given that it related to warnings: is there any chance you could release a version that doesn't require your compliance tests to pass? (even a pre-release would work, as i can specifically install the pre-release version.)

I ask as i'm in a environment where we are using poetry and that project has made the opinionated approach to not allow installation of conflicting versions.

I'd like to use bigquery-sqlalchemy just to keep my bigquery tables in sync via alembic and wouldn't be using it for anything else, so performance on the DDL updates isn't as big of issue.

but since i'm on python 3.10 there is no version of bigquery-sqlalchemy that doesn't have the strict version checking, which would require me to pin entirely to sqlalchemy<=1.4.27.

@cofin
Copy link

cofin commented Nov 3, 2022

I'm also looking for a way to relax the sqlalchemy version requirements. I'd be willing to contribute any help needed to allow this to support the sqlalchemy 2.0 pre-release.

@chalmerlowe
Copy link
Collaborator

@cofin @woidda

I have a PR #499 that is pending that should remove the restriction on the sqlalchemy version. Hoping to get this knocked out shortly.

@cofin
Copy link

cofin commented Nov 3, 2022

@chalmerlowe, Thanks for the quick reply. I had just cloned your branch and was going to give it a test.

For the poetry users:

While waiting for this to be merged, it's possible to use this branch now by using the following dependency in pyproject.toml.

sqlalchemy-bigquery = { git = "https://github.com/googleapis/python-bigquery-sqlalchemy.git", branch = "version-fix"}

@chalmerlowe
Copy link
Collaborator

Something about the PR has broken something different in our suite of tests, so YMMV and that is what I am trying to figure out/resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery-sqlalchemy API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants