Skip to content

Commit 15ffcf0

Browse files
fresioAStobymao
andauthored
fix(fabric): Alter table workaround (#5511)
Co-authored-by: Toby Mao <toby.mao@gmail.com>
1 parent 98998d4 commit 15ffcf0

File tree

2 files changed

+164
-0
lines changed

2 files changed

+164
-0
lines changed

sqlmesh/core/engine_adapter/fabric.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
)
1414
from sqlmesh.utils.errors import SQLMeshError
1515
from sqlmesh.utils.connection_pool import ConnectionPool
16+
from sqlmesh.core.schema_diff import TableAlterOperation
17+
from sqlmesh.utils import random_id
1618

1719

1820
logger = logging.getLogger(__name__)
@@ -153,6 +155,113 @@ def set_current_catalog(self, catalog_name: str) -> None:
153155
f"Unable to switch catalog to {catalog_name}, catalog ended up as {catalog_after_switch}"
154156
)
155157

158+
def alter_table(
159+
self, alter_expressions: t.Union[t.List[exp.Alter], t.List[TableAlterOperation]]
160+
) -> None:
161+
"""
162+
Applies alter expressions to a table. Fabric has limited support for ALTER TABLE,
163+
so this method implements a workaround for column type changes.
164+
This method is self-contained and sets its own catalog context.
165+
"""
166+
if not alter_expressions:
167+
return
168+
169+
# Get the target table from the first expression to determine the correct catalog.
170+
first_op = alter_expressions[0]
171+
expression = first_op.expression if isinstance(first_op, TableAlterOperation) else first_op
172+
if not isinstance(expression, exp.Alter) or not expression.this.catalog:
173+
# Fallback for unexpected scenarios
174+
logger.warning(
175+
"Could not determine catalog from alter expression, executing with current context."
176+
)
177+
super().alter_table(alter_expressions)
178+
return
179+
180+
target_catalog = expression.this.catalog
181+
self.set_current_catalog(target_catalog)
182+
183+
with self.transaction():
184+
for op in alter_expressions:
185+
expression = op.expression if isinstance(op, TableAlterOperation) else op
186+
187+
if not isinstance(expression, exp.Alter):
188+
self.execute(expression)
189+
continue
190+
191+
for action in expression.actions:
192+
table_name = expression.this
193+
194+
table_name_without_catalog = table_name.copy()
195+
table_name_without_catalog.set("catalog", None)
196+
197+
is_type_change = isinstance(action, exp.AlterColumn) and action.args.get(
198+
"dtype"
199+
)
200+
201+
if is_type_change:
202+
column_to_alter = action.this
203+
new_type = action.args["dtype"]
204+
temp_column_name_str = f"{column_to_alter.name}__{random_id(short=True)}"
205+
temp_column_name = exp.to_identifier(temp_column_name_str)
206+
207+
logger.info(
208+
"Applying workaround for column '%s' on table '%s' to change type to '%s'.",
209+
column_to_alter.sql(),
210+
table_name.sql(),
211+
new_type.sql(),
212+
)
213+
214+
# Step 1: Add a temporary column.
215+
add_column_expr = exp.Alter(
216+
this=table_name_without_catalog.copy(),
217+
kind="TABLE",
218+
actions=[
219+
exp.ColumnDef(this=temp_column_name.copy(), kind=new_type.copy())
220+
],
221+
)
222+
add_sql = self._to_sql(add_column_expr)
223+
self.execute(add_sql)
224+
225+
# Step 2: Copy and cast data.
226+
update_sql = self._to_sql(
227+
exp.Update(
228+
this=table_name_without_catalog.copy(),
229+
expressions=[
230+
exp.EQ(
231+
this=temp_column_name.copy(),
232+
expression=exp.Cast(
233+
this=column_to_alter.copy(), to=new_type.copy()
234+
),
235+
)
236+
],
237+
)
238+
)
239+
self.execute(update_sql)
240+
241+
# Step 3: Drop the original column.
242+
drop_sql = self._to_sql(
243+
exp.Alter(
244+
this=table_name_without_catalog.copy(),
245+
kind="TABLE",
246+
actions=[exp.Drop(this=column_to_alter.copy(), kind="COLUMN")],
247+
)
248+
)
249+
self.execute(drop_sql)
250+
251+
# Step 4: Rename the temporary column.
252+
old_name_qualified = f"{table_name_without_catalog.sql(dialect=self.dialect)}.{temp_column_name.sql(dialect=self.dialect)}"
253+
new_name_unquoted = column_to_alter.sql(
254+
dialect=self.dialect, identify=False
255+
)
256+
rename_sql = f"EXEC sp_rename '{old_name_qualified}', '{new_name_unquoted}', 'COLUMN'"
257+
self.execute(rename_sql)
258+
else:
259+
# For other alterations, execute directly.
260+
direct_alter_expr = exp.Alter(
261+
this=table_name_without_catalog.copy(), kind="TABLE", actions=[action]
262+
)
263+
self.execute(direct_alter_expr)
264+
156265

157266
class FabricHttpClient:
158267
def __init__(self, tenant_id: str, workspace_id: str, client_id: str, client_secret: str):

tests/core/engine_adapter/test_fabric.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,61 @@ def test_replace_query(adapter: FabricEngineAdapter, mocker: MockerFixture):
9191
]
9292

9393

94+
def test_alter_table_column_type_workaround(adapter: FabricEngineAdapter, mocker: MockerFixture):
95+
"""
96+
Tests the alter_table method's workaround for changing a column's data type.
97+
"""
98+
# Mock set_current_catalog to avoid connection pool side effects
99+
set_catalog_mock = mocker.patch.object(adapter, "set_current_catalog")
100+
# Mock random_id to have a predictable temporary column name
101+
mocker.patch("sqlmesh.core.engine_adapter.fabric.random_id", return_value="abcdef")
102+
103+
alter_expression = exp.Alter(
104+
this=exp.to_table("my_db.my_schema.my_table"),
105+
actions=[
106+
exp.AlterColumn(
107+
this=exp.to_column("col_a"),
108+
dtype=exp.DataType.build("BIGINT"),
109+
)
110+
],
111+
)
112+
113+
adapter.alter_table([alter_expression])
114+
115+
set_catalog_mock.assert_called_once_with("my_db")
116+
117+
expected_calls = [
118+
"ALTER TABLE [my_schema].[my_table] ADD [col_a__abcdef] BIGINT;",
119+
"UPDATE [my_schema].[my_table] SET [col_a__abcdef] = CAST([col_a] AS BIGINT);",
120+
"ALTER TABLE [my_schema].[my_table] DROP COLUMN [col_a];",
121+
"EXEC sp_rename 'my_schema.my_table.col_a__abcdef', 'col_a', 'COLUMN'",
122+
]
123+
124+
assert to_sql_calls(adapter) == expected_calls
125+
126+
127+
def test_alter_table_direct_alteration(adapter: FabricEngineAdapter, mocker: MockerFixture):
128+
"""
129+
Tests the alter_table method for direct alterations like adding a column.
130+
"""
131+
set_catalog_mock = mocker.patch.object(adapter, "set_current_catalog")
132+
133+
alter_expression = exp.Alter(
134+
this=exp.to_table("my_db.my_schema.my_table"),
135+
actions=[exp.ColumnDef(this=exp.to_column("new_col"), kind=exp.DataType.build("INT"))],
136+
)
137+
138+
adapter.alter_table([alter_expression])
139+
140+
set_catalog_mock.assert_called_once_with("my_db")
141+
142+
expected_calls = [
143+
"ALTER TABLE [my_schema].[my_table] ADD [new_col] INT;",
144+
]
145+
146+
assert to_sql_calls(adapter) == expected_calls
147+
148+
94149
def test_merge_pandas(
95150
make_mocked_engine_adapter: t.Callable, mocker: MockerFixture, make_temp_table_name: t.Callable
96151
):

0 commit comments

Comments
 (0)