From e9d15a6ae29554f50045a1d98bcd8a4417b2a1f6 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Fri, 9 Aug 2024 16:40:46 +0000 Subject: [PATCH 1/4] removes a compliance test that fails and replaces with unit test --- .../test_dialect_compliance.py | 4 ++ tests/unit/test_select.py | 50 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/tests/sqlalchemy_dialect_compliance/test_dialect_compliance.py b/tests/sqlalchemy_dialect_compliance/test_dialect_compliance.py index 4af38a84..ff14db9a 100644 --- a/tests/sqlalchemy_dialect_compliance/test_dialect_compliance.py +++ b/tests/sqlalchemy_dialect_compliance/test_dialect_compliance.py @@ -47,6 +47,7 @@ QuotedNameArgumentTest, SimpleUpdateDeleteTest as _SimpleUpdateDeleteTest, TimestampMicrosecondsTest as _TimestampMicrosecondsTest, + WindowFunctionTest, ) from sqlalchemy.testing.suite.test_types import ( @@ -636,3 +637,6 @@ def test_no_results_for_non_returning_insert(cls): del LongNameBlowoutTest # Requires features (indexes, primary keys, etc., that BigQuery doesn't have. del PostCompileParamsTest # BQ adds backticks to bind parameters, causing failure of tests TODO: fix this? del QuotedNameArgumentTest # Quotes aren't allowed in BigQuery table names. +del ( + WindowFunctionTest.test_window_rows_between +) # test expects BQ to return sorted results diff --git a/tests/unit/test_select.py b/tests/unit/test_select.py index 9d2ba21e..27c5df11 100644 --- a/tests/unit/test_select.py +++ b/tests/unit/test_select.py @@ -507,3 +507,53 @@ def test_visit_mod_binary(faux_conn): expected = "MOD(`table`.`foo`, %(foo_1:INT64)s)" assert result == expected + + +def test_window_rows_between(faux_conn): + """This is a replacement for the + 'test_window_rows_between' + test in sqlalchemy's suite of compliance tests. + + Their test is expecting things in sorted order and BQ + doesn't return sorted results they way they expect so that + test fails. + + Note: that test only appears in: + sqlalchemy/lib/sqlalchemy/testing/suite/test_select.py + in version 2.0.32. It appears as those that test will be + replaced with the a similar but new test called: + 'test_window_rows_between_w_caching' + due to the fact the rows are part of the cache key right now and + not handled as binds. This is related to sqlalchemy Issue #11515 + + It is expected the new test will also have the same sorting failure. + """ + + table = setup_table( + faux_conn, + "table", + sqlalchemy.Column("id", sqlalchemy.String), + sqlalchemy.Column("col1", sqlalchemy.Integer), + sqlalchemy.Column("col2", sqlalchemy.Integer), + ) + + stmt = sqlalchemy.select( + sqlalchemy.func.max(table.c.col2).over( + order_by=[table.c.col1], + rows=(-5, 0), + ) + ) + + sql = stmt.compile( + dialect=faux_conn.dialect, + compile_kwargs={"literal_binds": True}, + ) + + result = str(sql) + expected = ( + "SELECT max(`table`.`col2`) " + "OVER (ORDER BY `table`.`col1` " + "ROWS BETWEEN 5 PRECEDING AND CURRENT ROW) AS `anon_1` \n" # newline character required here to match + "FROM `table`" + ) + assert result == expected From ca5f444a7a37e86f67fc99a63a172a7763b1a9e3 Mon Sep 17 00:00:00 2001 From: Chalmer Lowe Date: Fri, 9 Aug 2024 12:45:32 -0400 Subject: [PATCH 2/4] Update tests/unit/test_select.py --- tests/unit/test_select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_select.py b/tests/unit/test_select.py index 27c5df11..fc82d1a2 100644 --- a/tests/unit/test_select.py +++ b/tests/unit/test_select.py @@ -515,7 +515,7 @@ def test_window_rows_between(faux_conn): test in sqlalchemy's suite of compliance tests. Their test is expecting things in sorted order and BQ - doesn't return sorted results they way they expect so that + doesn't return sorted results the way they expect so that test fails. Note: that test only appears in: From 9805923d07193f7b0d73e5697ebf6787e7c60723 Mon Sep 17 00:00:00 2001 From: Chalmer Lowe Date: Fri, 9 Aug 2024 12:46:58 -0400 Subject: [PATCH 3/4] Update tests/unit/test_select.py --- tests/unit/test_select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_select.py b/tests/unit/test_select.py index fc82d1a2..8119de74 100644 --- a/tests/unit/test_select.py +++ b/tests/unit/test_select.py @@ -520,7 +520,7 @@ def test_window_rows_between(faux_conn): Note: that test only appears in: sqlalchemy/lib/sqlalchemy/testing/suite/test_select.py - in version 2.0.32. It appears as those that test will be + in version 2.0.32. It appears as though that test will be replaced with the a similar but new test called: 'test_window_rows_between_w_caching' due to the fact the rows are part of the cache key right now and From 4fa73b4fd3cefcedbc2975a406a2b33571b3666e Mon Sep 17 00:00:00 2001 From: Chalmer Lowe Date: Fri, 9 Aug 2024 12:47:03 -0400 Subject: [PATCH 4/4] Update tests/unit/test_select.py --- tests/unit/test_select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_select.py b/tests/unit/test_select.py index 8119de74..a600bdf9 100644 --- a/tests/unit/test_select.py +++ b/tests/unit/test_select.py @@ -521,7 +521,7 @@ def test_window_rows_between(faux_conn): Note: that test only appears in: sqlalchemy/lib/sqlalchemy/testing/suite/test_select.py in version 2.0.32. It appears as though that test will be - replaced with the a similar but new test called: + replaced with a similar but new test called: 'test_window_rows_between_w_caching' due to the fact the rows are part of the cache key right now and not handled as binds. This is related to sqlalchemy Issue #11515