Skip to content

Commit 1f8a25f

Browse files
waltaskewolavloite
andauthored
fix: Respect existing server default in alter column DDL (googleapis#733)
* fix: Correctly Generate DDL for ALTER COLUMN ... SET DEFAULT Alembic expects `get_column_default_string` to be implemented in order to use it for `ALTER TABLE.. ALTER COLUMN .. SET DEFAULT` DDL. In our case, this means wrapping the default value in parentheses. We implement `get_column_default_string` and have it add parentheses for use in both `CREATE TABLE` and `ALTER TABLE` DDL. Call path for alembic relying on `get_column_default_string` is here: https://github.com/sqlalchemy/alembic/blob/cd4f404358f101b2b930013c609c074baca61468/alembic/ddl/base.py#L252 https://github.com/sqlalchemy/alembic/blob/cd4f404358f101b2b930013c609c074baca61468/alembic/ddl/base.py#L315 * fix: Respect existing server default in alter column DDL Include `DEFAULT (default_value)` stanzas in `ALTER COLUMN` DDL to avoid removing `DEFAULT` values when changing a column's type or nullability. Fixes: googleapis#732 * chore: ignore booleans --------- Co-authored-by: Knut Olav Løite <koloite@gmail.com>
1 parent 1fe9f4b commit 1f8a25f

File tree

5 files changed

+223
-11
lines changed

5 files changed

+223
-11
lines changed

google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
ColumnType,
2121
alter_column,
2222
alter_table,
23+
format_server_default,
2324
format_type,
2425
)
2526
from google.api_core.client_options import ClientOptions
@@ -579,7 +580,7 @@ def get_column_specification(self, column, **kwargs):
579580
elif has_identity:
580581
colspec += " " + self.process(column.identity)
581582
elif default is not None:
582-
colspec += " DEFAULT (" + default + ")"
583+
colspec += f" DEFAULT {default}"
583584
elif hasattr(column, "computed") and column.computed is not None:
584585
colspec += " " + self.process(column.computed)
585586

@@ -590,6 +591,13 @@ def get_column_specification(self, column, **kwargs):
590591

591592
return colspec
592593

594+
def get_column_default_string(self, column):
595+
default = super().get_column_default_string(column)
596+
if default is not None:
597+
return f"({default})"
598+
599+
return default
600+
593601
def visit_computed_column(self, generated, **kw):
594602
"""Computed column operator."""
595603
text = "AS (%s) STORED" % self.sql_compiler.process(
@@ -1860,11 +1868,14 @@ def do_execute_no_params(self, cursor, statement, context=None):
18601868
def visit_column_nullable(
18611869
element: "ColumnNullable", compiler: "SpannerDDLCompiler", **kw
18621870
) -> str:
1863-
return "%s %s %s %s" % (
1864-
alter_table(compiler, element.table_name, element.schema),
1865-
alter_column(compiler, element.column_name),
1866-
format_type(compiler, element.existing_type),
1867-
"" if element.nullable else "NOT NULL",
1871+
return _format_alter_column(
1872+
compiler,
1873+
element.table_name,
1874+
element.schema,
1875+
element.column_name,
1876+
element.existing_type,
1877+
element.nullable,
1878+
element.existing_server_default,
18681879
)
18691880

18701881

@@ -1873,9 +1884,34 @@ def visit_column_nullable(
18731884
def visit_column_type(
18741885
element: "ColumnType", compiler: "SpannerDDLCompiler", **kw
18751886
) -> str:
1876-
return "%s %s %s %s" % (
1877-
alter_table(compiler, element.table_name, element.schema),
1878-
alter_column(compiler, element.column_name),
1879-
"%s" % format_type(compiler, element.type_),
1880-
"" if element.existing_nullable else "NOT NULL",
1887+
return _format_alter_column(
1888+
compiler,
1889+
element.table_name,
1890+
element.schema,
1891+
element.column_name,
1892+
element.type_,
1893+
element.existing_nullable,
1894+
element.existing_server_default,
1895+
)
1896+
1897+
1898+
def _format_alter_column(
1899+
compiler, table_name, schema, column_name, type_, nullable, server_default
1900+
):
1901+
# Older versions of SQLAlchemy pass in a boolean to indicate whether there
1902+
# is an existing DEFAULT constraint, instead of the actual DEFAULT constraint
1903+
# expression. In those cases, we do not want to explicitly include the DEFAULT
1904+
# constraint in the expression that is generated here.
1905+
if isinstance(server_default, bool):
1906+
server_default = None
1907+
return "%s %s %s%s%s" % (
1908+
alter_table(compiler, table_name, schema),
1909+
alter_column(compiler, column_name),
1910+
format_type(compiler, type_),
1911+
"" if nullable else " NOT NULL",
1912+
(
1913+
""
1914+
if server_default is None
1915+
else f" DEFAULT {format_server_default(compiler, server_default)}"
1916+
),
18811917
)
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Copyright 2025 Google LLC All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from sqlalchemy import func
16+
from sqlalchemy.orm import DeclarativeBase
17+
from sqlalchemy.orm import Mapped
18+
from sqlalchemy.orm import mapped_column
19+
20+
21+
class Base(DeclarativeBase):
22+
pass
23+
24+
25+
class Singer(Base):
26+
__tablename__ = "singers"
27+
id: Mapped[str] = mapped_column(
28+
server_default=func.GENERATE_UUID(), primary_key=True
29+
)
30+
name: Mapped[str]
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Copyright 2025 Google LLC All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from sqlalchemy import create_engine
16+
from sqlalchemy.testing import eq_, is_instance_of
17+
from google.cloud.spanner_v1 import FixedSizePool, ResultSet
18+
from test.mockserver_tests.mock_server_test_base import MockServerTestBase, add_result
19+
from google.cloud.spanner_admin_database_v1 import UpdateDatabaseDdlRequest
20+
21+
22+
class TestCreateTableDefault(MockServerTestBase):
23+
def test_create_table_with_default(self):
24+
from test.mockserver_tests.default_model import Base
25+
26+
add_result(
27+
"""SELECT true
28+
FROM INFORMATION_SCHEMA.TABLES
29+
WHERE TABLE_SCHEMA="" AND TABLE_NAME="singers"
30+
LIMIT 1
31+
""",
32+
ResultSet(),
33+
)
34+
engine = create_engine(
35+
"spanner:///projects/p/instances/i/databases/d",
36+
connect_args={"client": self.client, "pool": FixedSizePool(size=10)},
37+
)
38+
Base.metadata.create_all(engine)
39+
requests = self.database_admin_service.requests
40+
eq_(1, len(requests))
41+
is_instance_of(requests[0], UpdateDatabaseDdlRequest)
42+
eq_(1, len(requests[0].statements))
43+
eq_(
44+
"CREATE TABLE singers (\n"
45+
"\tid STRING(MAX) NOT NULL DEFAULT (GENERATE_UUID()), \n"
46+
"\tname STRING(MAX) NOT NULL\n"
47+
") PRIMARY KEY (id)",
48+
requests[0].statements[0],
49+
)

test/unit/__init__.py

Whitespace-only changes.

test/unit/test_alembic.py

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
from alembic.ddl import base as ddl_base
16+
from google.cloud.sqlalchemy_spanner import sqlalchemy_spanner
17+
from sqlalchemy import String, TextClause
18+
from sqlalchemy.testing import eq_
19+
from sqlalchemy.testing.plugin.plugin_base import fixtures
20+
21+
22+
class TestAlembicTest(fixtures.TestBase):
23+
def test_visit_column_nullable_with_not_null_column(self):
24+
ddl = sqlalchemy_spanner.visit_column_nullable(
25+
ddl_base.ColumnNullable(
26+
name="tbl", column_name="col", nullable=False, existing_type=String(256)
27+
),
28+
sqlalchemy_spanner.SpannerDDLCompiler(
29+
sqlalchemy_spanner.SpannerDialect(), None
30+
),
31+
)
32+
eq_(ddl, "ALTER TABLE tbl ALTER COLUMN col STRING(256) NOT NULL")
33+
34+
def test_visit_column_nullable_with_nullable_column(self):
35+
ddl = sqlalchemy_spanner.visit_column_nullable(
36+
ddl_base.ColumnNullable(
37+
name="tbl", column_name="col", nullable=True, existing_type=String(256)
38+
),
39+
sqlalchemy_spanner.SpannerDDLCompiler(
40+
sqlalchemy_spanner.SpannerDialect(), None
41+
),
42+
)
43+
eq_(ddl, "ALTER TABLE tbl ALTER COLUMN col STRING(256)")
44+
45+
def test_visit_column_nullable_with_default(self):
46+
ddl = sqlalchemy_spanner.visit_column_nullable(
47+
ddl_base.ColumnNullable(
48+
name="tbl",
49+
column_name="col",
50+
nullable=False,
51+
existing_type=String(256),
52+
existing_server_default=TextClause("GENERATE_UUID()"),
53+
),
54+
sqlalchemy_spanner.SpannerDDLCompiler(
55+
sqlalchemy_spanner.SpannerDialect(), None
56+
),
57+
)
58+
eq_(
59+
ddl,
60+
"ALTER TABLE tbl "
61+
"ALTER COLUMN col "
62+
"STRING(256) NOT NULL DEFAULT (GENERATE_UUID())",
63+
)
64+
65+
def test_visit_column_type(self):
66+
ddl = sqlalchemy_spanner.visit_column_type(
67+
ddl_base.ColumnType(
68+
name="tbl",
69+
column_name="col",
70+
type_=String(256),
71+
existing_nullable=True,
72+
),
73+
sqlalchemy_spanner.SpannerDDLCompiler(
74+
sqlalchemy_spanner.SpannerDialect(), None
75+
),
76+
)
77+
eq_(ddl, "ALTER TABLE tbl ALTER COLUMN col STRING(256)")
78+
79+
def test_visit_column_type_with_default(self):
80+
ddl = sqlalchemy_spanner.visit_column_type(
81+
ddl_base.ColumnType(
82+
name="tbl",
83+
column_name="col",
84+
type_=String(256),
85+
existing_nullable=False,
86+
existing_server_default=TextClause("GENERATE_UUID()"),
87+
),
88+
sqlalchemy_spanner.SpannerDDLCompiler(
89+
sqlalchemy_spanner.SpannerDialect(), None
90+
),
91+
)
92+
eq_(
93+
ddl,
94+
"ALTER TABLE tbl "
95+
"ALTER COLUMN col "
96+
"STRING(256) NOT NULL DEFAULT (GENERATE_UUID())",
97+
)

0 commit comments

Comments
 (0)