From 0c0c9be09b90e62df6d33a64452d9647409b23d9 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 13:47:28 +0300 Subject: [PATCH 01/16] feat: support computed columns --- google/cloud/sqlalchemy_spanner/requirements.py | 8 ++++++++ google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/google/cloud/sqlalchemy_spanner/requirements.py b/google/cloud/sqlalchemy_spanner/requirements.py index 1b929847..fffd75b0 100644 --- a/google/cloud/sqlalchemy_spanner/requirements.py +++ b/google/cloud/sqlalchemy_spanner/requirements.py @@ -17,6 +17,14 @@ class Requirements(SuiteRequirements): + @property + def computed_columns(self): + return exclusions.open() + + @property + def computed_columns_stored(self): + return exclusions.open() + @property def foreign_key_constraint_name_reflection(self): return exclusions.open() diff --git a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py index a9abec59..941e08be 100644 --- a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py +++ b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py @@ -254,6 +254,13 @@ def limit_clause(self, select, **kw): class SpannerDDLCompiler(DDLCompiler): """Spanner DDL statements compiler.""" + def visit_computed_column(self, generated, **kw): + """Computed column operator.""" + text = "AS (%s) STORED" % self.sql_compiler.process( + generated.sqltext, include_table=False, literal_binds=True + ) + return text + def visit_drop_table(self, drop_table): """ Cloud Spanner doesn't drop tables which have indexes From 2a2d3e3198e6513d7e3dc70817b8351624dbf2e4 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:14:03 +0300 Subject: [PATCH 02/16] fix tests --- test/test_suite.py | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/test/test_suite.py b/test/test_suite.py index 500114c3..9300502c 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1602,3 +1602,75 @@ def test_read_only(self): with self._engine.connect().execution_options(read_only=True) as connection: connection.execute(select(["*"], from_obj=self._table)).fetchall() assert connection.connection.read_only is True + + +class ComputedReflectionFixtureTest(fixtures.ComputedReflectionFixtureTest): + @classmethod + def define_tables(cls, metadata): + """SPANNER OVERRIDE: + + Avoid using default values for computed columns. + """ + from .. import Integer + from .. import testing + from ..schema import Column + from ..schema import Computed + from ..schema import Table + + Table( + "computed_default_table", + metadata, + Column("id", Integer, primary_key=True), + Column("normal", Integer), + Column("computed_col", Integer, Computed("normal + 42")), + Column("with_default", Integer), + ) + + t = Table( + "computed_column_table", + metadata, + Column("id", Integer, primary_key=True), + Column("normal", Integer), + Column("computed_no_flag", Integer, Computed("normal + 42")), + ) + + if testing.requires.schemas.enabled: + t2 = Table( + "computed_column_table", + metadata, + Column("id", Integer, primary_key=True), + Column("normal", Integer), + Column("computed_no_flag", Integer, Computed("normal / 42")), + schema=config.test_schema, + ) + + if testing.requires.computed_columns_virtual.enabled: + t.append_column( + Column( + "computed_virtual", + Integer, + Computed("normal + 2", persisted=False), + ) + ) + if testing.requires.schemas.enabled: + t2.append_column( + Column( + "computed_virtual", + Integer, + Computed("normal / 2", persisted=False), + ) + ) + if testing.requires.computed_columns_stored.enabled: + t.append_column( + Column( + "computed_stored", Integer, Computed("normal - 42", persisted=True), + ) + ) + if testing.requires.schemas.enabled: + t2.append_column( + Column( + "computed_stored", + Integer, + Computed("normal * 42", persisted=True), + ) + ) From f3ca2b61ec9913d0e470fc2e2a9ccb7772ab2a29 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:18:39 +0300 Subject: [PATCH 03/16] override fixture --- test/test_suite.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_suite.py b/test/test_suite.py index 9300502c..9fcd3016 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -54,6 +54,9 @@ from sqlalchemy.types import Numeric from sqlalchemy.types import Text from sqlalchemy.testing import requires +from sqlalchemy.testing.fixtures import ( + ComputedReflectionFixtureTest as _ComputedReflectionFixtureTest, +) from google.api_core.datetime_helpers import DatetimeWithNanoseconds @@ -1604,7 +1607,7 @@ def test_read_only(self): assert connection.connection.read_only is True -class ComputedReflectionFixtureTest(fixtures.ComputedReflectionFixtureTest): +class ComputedReflectionFixtureTest(_ComputedReflectionFixtureTest): @classmethod def define_tables(cls, metadata): """SPANNER OVERRIDE: From 3d31dfd1cde4c7757725e6b4edada9e839a945ed Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:23:22 +0300 Subject: [PATCH 04/16] override test case --- test/test_suite.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_suite.py b/test/test_suite.py index 9fcd3016..b69f6127 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -92,6 +92,7 @@ QuotedNameArgumentTest as _QuotedNameArgumentTest, ComponentReflectionTest as _ComponentReflectionTest, CompositeKeyReflectionTest as _CompositeKeyReflectionTest, + ComputedReflectionTest as _ComputedReflectionTest, ) from sqlalchemy.testing.suite.test_results import RowFetchTest as _RowFetchTest from sqlalchemy.testing.suite.test_types import ( # noqa: F401, F403 @@ -1677,3 +1678,7 @@ def define_tables(cls, metadata): Computed("normal * 42", persisted=True), ) ) + + +class ComputedReflectionTest(_ComputedReflectionTest, ComputedReflectionFixtureTest): + pass From 9eabb6884a38ca44c5979c6dbe25addcdb51d6b6 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:26:36 +0300 Subject: [PATCH 05/16] fix imports --- test/test_suite.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/test_suite.py b/test/test_suite.py index b69f6127..9f9336bd 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -29,6 +29,7 @@ from sqlalchemy import ForeignKey from sqlalchemy import MetaData from sqlalchemy.schema import DDL +from sqlalchemy.schema import Computed from sqlalchemy.testing import config from sqlalchemy.testing import engines from sqlalchemy.testing import eq_ @@ -1615,12 +1616,6 @@ def define_tables(cls, metadata): Avoid using default values for computed columns. """ - from .. import Integer - from .. import testing - from ..schema import Column - from ..schema import Computed - from ..schema import Table - Table( "computed_default_table", metadata, From 1fc815e2fffb10139e85dfb69721039e6adaac0e Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:31:44 +0300 Subject: [PATCH 06/16] skip unsupported case --- test/test_suite.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_suite.py b/test/test_suite.py index 9f9336bd..139ac266 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1676,4 +1676,6 @@ def define_tables(cls, metadata): class ComputedReflectionTest(_ComputedReflectionTest, ComputedReflectionFixtureTest): - pass + @pytest.mark.skip("Default values are not supported.") + def test_computed_col_default_not_set(self): + pass From 3e086d35d986a58db3b49b269678348d4d5b2eb4 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:40:08 +0300 Subject: [PATCH 07/16] reflect computed --- google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py index 941e08be..17c20b54 100644 --- a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py +++ b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py @@ -486,7 +486,7 @@ def get_columns(self, connection, table_name, schema=None, **kw): list: The table every column dict-like description. """ sql = """ -SELECT column_name, spanner_type, is_nullable +SELECT column_name, spanner_type, is_nullable, is_generated FROM information_schema.columns WHERE table_catalog = '' @@ -512,6 +512,7 @@ def get_columns(self, connection, table_name, schema=None, **kw): "type": self._designate_type(col[1]), "nullable": col[2] == "YES", "default": None, + "computed": col[3] == "GENERATED", } ) return cols_desc From 276a62a3fdd98a86d7af316814ae6a799be7c824 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:43:24 +0300 Subject: [PATCH 08/16] fix --- google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py index 17c20b54..6f2f0ede 100644 --- a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py +++ b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py @@ -486,7 +486,7 @@ def get_columns(self, connection, table_name, schema=None, **kw): list: The table every column dict-like description. """ sql = """ -SELECT column_name, spanner_type, is_nullable, is_generated +SELECT column_name, spanner_type, is_nullable, is_generated, generation_expression FROM information_schema.columns WHERE table_catalog = '' @@ -512,7 +512,10 @@ def get_columns(self, connection, table_name, schema=None, **kw): "type": self._designate_type(col[1]), "nullable": col[2] == "YES", "default": None, - "computed": col[3] == "GENERATED", + "computed": { + "persisted": col[3] == "GENERATED", + "sqltext": col[4], + }, } ) return cols_desc From 4b3695c391365209dd745bc7eef2d43530d8be2d Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:48:33 +0300 Subject: [PATCH 09/16] fix cols desc --- .../sqlalchemy_spanner/sqlalchemy_spanner.py | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py index 6f2f0ede..3acb1c32 100644 --- a/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py +++ b/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py @@ -486,7 +486,7 @@ def get_columns(self, connection, table_name, schema=None, **kw): list: The table every column dict-like description. """ sql = """ -SELECT column_name, spanner_type, is_nullable, is_generated, generation_expression +SELECT column_name, spanner_type, is_nullable, generation_expression FROM information_schema.columns WHERE table_catalog = '' @@ -506,18 +506,20 @@ def get_columns(self, connection, table_name, schema=None, **kw): columns = snap.execute_sql(sql) for col in columns: - cols_desc.append( - { - "name": col[0], - "type": self._designate_type(col[1]), - "nullable": col[2] == "YES", - "default": None, - "computed": { - "persisted": col[3] == "GENERATED", - "sqltext": col[4], - }, + col_desc = { + "name": col[0], + "type": self._designate_type(col[1]), + "nullable": col[2] == "YES", + "default": None, + } + + if col[3] is not None: + col_desc["computed"] = { + "persisted": True, + "sqltext": col[3], } - ) + cols_desc.append(col_desc) + return cols_desc def _designate_type(self, str_repr): From 98874474ac2b8a11e11e6196678ba0e0dc370720 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:53:30 +0300 Subject: [PATCH 10/16] erase unsupported condition --- test/test_suite.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/test_suite.py b/test/test_suite.py index 139ac266..8a230207 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1679,3 +1679,19 @@ class ComputedReflectionTest(_ComputedReflectionTest, ComputedReflectionFixtureT @pytest.mark.skip("Default values are not supported.") def test_computed_col_default_not_set(self): pass + + def test_get_column_returns_computed(self): + insp = inspect(config.db) + + cols = insp.get_columns("computed_default_table") + data = {c["name"]: c for c in cols} + for key in ("id", "normal", "with_default"): + is_true("computed" not in data[key]) + compData = data["computed_col"] + is_true("computed" in compData) + is_true("sqltext" in compData["computed"]) + eq_(self.normalize(compData["computed"]["sqltext"]), "normal+42") + eq_( + "persisted" in compData["computed"], + testing.requires.computed_columns_reflect_persisted.enabled, + ) From 13ec44c5759620494cd74383deeba471bf9c3f1f Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 14:57:47 +0300 Subject: [PATCH 11/16] add import --- test/test_suite.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_suite.py b/test/test_suite.py index 8a230207..2f56a170 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -35,6 +35,7 @@ from sqlalchemy.testing import eq_ from sqlalchemy.testing import provide_metadata, emits_warning from sqlalchemy.testing import fixtures +from sqlalchemy.testing import is_true from sqlalchemy.testing.provision import temp_table_keyword_args from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table From ddbb1f75039fcb0d375a3bd9b2954742a8ccf176 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 15:01:12 +0300 Subject: [PATCH 12/16] erase unsupported statement --- test/test_suite.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/test_suite.py b/test/test_suite.py index 2f56a170..e952de8f 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1692,7 +1692,3 @@ def test_get_column_returns_computed(self): is_true("computed" in compData) is_true("sqltext" in compData["computed"]) eq_(self.normalize(compData["computed"]["sqltext"]), "normal+42") - eq_( - "persisted" in compData["computed"], - testing.requires.computed_columns_reflect_persisted.enabled, - ) From adf7819c1a1581903577b07f2d69c7e41d2f9234 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 15:09:07 +0300 Subject: [PATCH 13/16] try --- google/cloud/sqlalchemy_spanner/requirements.py | 4 ++++ test/test_suite.py | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/google/cloud/sqlalchemy_spanner/requirements.py b/google/cloud/sqlalchemy_spanner/requirements.py index fffd75b0..9db083b6 100644 --- a/google/cloud/sqlalchemy_spanner/requirements.py +++ b/google/cloud/sqlalchemy_spanner/requirements.py @@ -21,6 +21,10 @@ class Requirements(SuiteRequirements): def computed_columns(self): return exclusions.open() + @property + def computed_columns_default_persisted(self): + return exclusions.open() + @property def computed_columns_stored(self): return exclusions.open() diff --git a/test/test_suite.py b/test/test_suite.py index e952de8f..1219c02f 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1692,3 +1692,12 @@ def test_get_column_returns_computed(self): is_true("computed" in compData) is_true("sqltext" in compData["computed"]) eq_(self.normalize(compData["computed"]["sqltext"]), "normal+42") + eq_( + "persisted" in compData["computed"], + testing.requires.computed_columns_reflect_persisted.enabled, + ) + if testing.requires.computed_columns_reflect_persisted.enabled: + eq_( + compData["computed"]["persisted"], + testing.requires.computed_columns_default_persisted.enabled, + ) From 55fceefaa49c798f53645203f7302fd46f335956 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 3 Nov 2021 15:13:02 +0300 Subject: [PATCH 14/16] Revert "try" This reverts commit adf7819c1a1581903577b07f2d69c7e41d2f9234. --- google/cloud/sqlalchemy_spanner/requirements.py | 4 ---- test/test_suite.py | 9 --------- 2 files changed, 13 deletions(-) diff --git a/google/cloud/sqlalchemy_spanner/requirements.py b/google/cloud/sqlalchemy_spanner/requirements.py index 9db083b6..fffd75b0 100644 --- a/google/cloud/sqlalchemy_spanner/requirements.py +++ b/google/cloud/sqlalchemy_spanner/requirements.py @@ -21,10 +21,6 @@ class Requirements(SuiteRequirements): def computed_columns(self): return exclusions.open() - @property - def computed_columns_default_persisted(self): - return exclusions.open() - @property def computed_columns_stored(self): return exclusions.open() diff --git a/test/test_suite.py b/test/test_suite.py index 1219c02f..e952de8f 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1692,12 +1692,3 @@ def test_get_column_returns_computed(self): is_true("computed" in compData) is_true("sqltext" in compData["computed"]) eq_(self.normalize(compData["computed"]["sqltext"]), "normal+42") - eq_( - "persisted" in compData["computed"], - testing.requires.computed_columns_reflect_persisted.enabled, - ) - if testing.requires.computed_columns_reflect_persisted.enabled: - eq_( - compData["computed"]["persisted"], - testing.requires.computed_columns_default_persisted.enabled, - ) From 1a86b75183f2855eef5ced6839c9da23320faf34 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 17 Nov 2021 12:33:13 +0300 Subject: [PATCH 15/16] add docstrings --- test/test_suite.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/test_suite.py b/test/test_suite.py index e952de8f..488d4127 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1682,6 +1682,14 @@ def test_computed_col_default_not_set(self): pass def test_get_column_returns_computed(self): + """ + SPANNER OVERRIDE: + + In Spanner all the generated columns are STORED, + meaning there are no persisted and not persisted + (in the terms of the SQLAlchemy) columns. The + method override omits the persistence reflection checks. + """ insp = inspect(config.db) cols = insp.get_columns("computed_default_table") From 438ccfc94f414a78391c911faa0bbbb590c8389b Mon Sep 17 00:00:00 2001 From: Shanika Kuruppu Date: Fri, 19 Nov 2021 16:13:47 +1100 Subject: [PATCH 16/16] fix: lint --- test/test_suite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_suite.py b/test/test_suite.py index 1b2e0bba..0bc4030c 100644 --- a/test/test_suite.py +++ b/test/test_suite.py @@ -1705,4 +1705,4 @@ def test_get_column_returns_computed(self): compData = data["computed_col"] is_true("computed" in compData) is_true("sqltext" in compData["computed"]) - eq_(self.normalize(compData["computed"]["sqltext"]), "normal+42") \ No newline at end of file + eq_(self.normalize(compData["computed"]["sqltext"]), "normal+42")