Skip to content

Commit 41905e2

Browse files
waltaskewolavloite
andauthored
fix: include schema when creating indices (#637)
* bug: include schema when creating indices Fixes an issue where indices aren't able to be created for tables in schemas. At present, `vist_create_index` emits SQL like ```sql CREATE INDEX index_name (col) ON schema_name.table_name ``` which results in an error when creating the table: ``` Index index_name cannot index a table/index schema_name.table_name which is in a different named schema. ``` because the name needs to be prefixed by schema. `vist_drop_index` does emit the correct SQL with the schema prefix. ```sql DROP INDEX schema_name.index_name ``` This is due to an odd issue where the base class's `visit_create_index` method has in its signature `include_schema=False`: https://github.com/sqlalchemy/sqlalchemy/blob/299284cec65076fd4c76bf1efaae60b60f4d4f7b/lib/sqlalchemy/sql/compiler.py#L6828C23-L6828C43 but `visit_drop_index` hard-codes the value to `True`: https://github.com/sqlalchemy/sqlalchemy/blob/299284cec65076fd4c76bf1efaae60b60f4d4f7b/lib/sqlalchemy/sql/compiler.py#L6870 The dialects handle this by hard-coding `include_schema=True` in `visit_create_index`, e.g. sqlite: https://github.com/sqlalchemy/sqlalchemy/blob/299284cec65076fd4c76bf1efaae60b60f4d4f7b/lib/sqlalchemy/dialects/sqlite/base.py#L1740 The difference in defaults is odd in the base class, but it seems like include_schema=True is the appropriate setting for Spanner. * chore: override default instead of always passing in True --------- Co-authored-by: Knut Olav Løite <[email protected]>
1 parent 028fb28 commit 41905e2

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ def post_create_table(self, table):
529529
return post_cmds
530530

531531
def visit_create_index(
532-
self, create, include_schema=False, include_table_schema=True, **kw
532+
self, create, include_schema=True, include_table_schema=True, **kw
533533
):
534534
text = super().visit_create_index(
535535
create, include_schema, include_table_schema, **kw

test/mockserver_tests/test_basics.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
MetaData,
2222
Table,
2323
Column,
24+
Index,
2425
Integer,
2526
String,
2627
func,
@@ -134,6 +135,47 @@ def test_create_table(self):
134135
requests[0].statements[0],
135136
)
136137

138+
def test_create_table_in_schema(self):
139+
add_result(
140+
"""SELECT true
141+
FROM INFORMATION_SCHEMA.TABLES
142+
WHERE TABLE_SCHEMA="schema" AND TABLE_NAME="users"
143+
LIMIT 1
144+
""",
145+
ResultSet(),
146+
)
147+
engine = create_engine(
148+
"spanner:///projects/p/instances/i/databases/d",
149+
connect_args={"client": self.client, "pool": FixedSizePool(size=10)},
150+
)
151+
metadata = MetaData()
152+
Table(
153+
"users",
154+
metadata,
155+
Column("user_id", Integer, primary_key=True),
156+
Column("user_name", String(16), nullable=False),
157+
Index("ix_users_user_id", "user_id"),
158+
schema="schema",
159+
)
160+
metadata.create_all(engine)
161+
requests = self.database_admin_service.requests
162+
eq_(1, len(requests))
163+
is_instance_of(requests[0], UpdateDatabaseDdlRequest)
164+
eq_(2, len(requests[0].statements))
165+
166+
eq_(
167+
"CREATE TABLE schema.users (\n"
168+
"\tuser_id INT64 NOT NULL "
169+
"GENERATED BY DEFAULT AS IDENTITY (BIT_REVERSED_POSITIVE), \n"
170+
"\tuser_name STRING(16) NOT NULL\n"
171+
") PRIMARY KEY (user_id)",
172+
requests[0].statements[0],
173+
)
174+
eq_(
175+
"CREATE INDEX schema.ix_users_user_id ON schema.users (user_id)",
176+
requests[0].statements[1],
177+
)
178+
137179
def test_create_multiple_tables(self):
138180
for i in range(2):
139181
add_result(

0 commit comments

Comments
 (0)