googleapis / python-bigquery-sqlalchemy

SQLAlchemy dialect for BigQuery
MIT License
426 stars 127 forks source link

Bug: removes a compliance test that fails and replaces with unit test #1110

Closed chalmerlowe closed 4 weeks ago

chalmerlowe commented 1 month ago

This bugfix removes a sqlalchemy compliance test that expects results in sorted order, which BQ does not do using the SQL statement found in that test. We replace that test with a related unit test to confirm that the windowing feature is correctly created in our SQL statements.

Fixes #1109 🦕

Another PR will address these issues related to Enum Errors.

1102 🦕

1103 🦕

1104 🦕

1105 🦕

1106 🦕

1107 🦕

1108 🦕

conventional-commit-lint-gcf[bot] commented 1 month ago

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot https://conventionalcommits.org/

chalmerlowe commented 1 month ago

@Linchin

Can you take a look at this and review/approve?

There used to be two Windowing compliance tests, the second one no longer runs. The test sequence runs the first Windowing test (test_windwo) and not the second:

.../test_dialect_compliance.py::WeCanSetDefaultSchemaWEventsTest::test_wont_work_wo_insert <- .nox/compliance/lib/python3.12/site-packages/sqlalchemy/testing/suite/test_dialect.py SKIPPED [ 99%]
.../test_dialect_compliance.py::WindowFunctionTest_bigquery+bigquery::test_window <- .nox/compliance/lib/python3.12/site-packages/sqlalchemy/testing/suite/test_select.py PASSED [100%]

NOTE: this solves one specific flaky-bot issue related to Windowing. It does not solve the other flakybot issues related to the Enum errors. I want to get this resolution on the books so we can clear out the flakybot issue. I am working in a separate PR on fixes to handle the Enum errors.

Will tag this PR with the PR number for the Enum issue as soon as possible.

Linchin commented 4 weeks ago

Hi @chalmerlowe, thank you for taking the time to fix the flaky issue! Indeed now only the enum tests are failing.

LGTM.