Skip to content

fix(sqlalchemy-bigquery): wrap string in WKT in geography system tests#17780

Merged
ohmayr merged 2 commits into
mainfrom
fix-sqlalchemy-bigquery-geography-system-tests
Jul 20, 2026
Merged

fix(sqlalchemy-bigquery): wrap string in WKT in geography system tests#17780
ohmayr merged 2 commits into
mainfrom
fix-sqlalchemy-bigquery-geography-system-tests

Conversation

@ohmayr

@ohmayr ohmayr commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Wraps spatial string parameters with WKT(...) in ST_Contains and ST_Intersects system test queries. This ensures GeoAlchemy2 compiles ST_GeogFromText(...) so BigQuery receives GEOGRAPHY parameters instead of throwing argument type mismatch errors (GEOGRAPHY, STRING). Unskips geography system tests.

Towards #17287 🦕

@ohmayr
ohmayr requested review from a team as code owners July 20, 2026 18:35
@ohmayr
ohmayr requested review from tswast and removed request for a team July 20, 2026 18:35

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request unskips several GeoAlchemy2 system tests (test_geoalchemy2_core, test_geoalchemy2_orm, and test_geoalchemy2_orm_w_relationship) in packages/sqlalchemy-bigquery/tests/system/test_geography.py. To resolve the previous failures, the spatial string literals in the queries are now wrapped with WKT imported from sqlalchemy_bigquery. There are no review comments, and I have no additional feedback to provide.

@parthea parthea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but holding off on formal approval. Please also update the unit test here

# TODO(http://github.com/googleapis/google-cloud-python/issues/17287): Unskip once bug is resolved.
@pytest.mark.skip(reason="Failing in CI with AssertionError.")
def test_geoalchemy2_core(faux_conn, last_query):

Comment thread packages/sqlalchemy-bigquery/tests/system/test_geography.py

@parthea parthea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the system test fix. The unit test fix review is in a separate PR : #17769

@ohmayr
ohmayr merged commit 26d43c1 into main Jul 20, 2026
38 of 39 checks passed
@ohmayr
ohmayr deleted the fix-sqlalchemy-bigquery-geography-system-tests branch July 20, 2026 23:54
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

Successfully merging this pull request may close these issues.

2 participants