Skip to content

Commit f532224

Browse files
committed
dont compare server defaults if user FN returns non-None
fixed regression caused by e532a7e where we started continuing default server default comparison even if user defined function returned False. Fixes: #1777 Change-Id: I99fe8a10bc58a83aae1f1fae48b3b151a4db64c6
1 parent ff71a4f commit f532224

2 files changed

Lines changed: 62 additions & 0 deletions

File tree

alembic/autogenerate/compare/server_defaults.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,11 @@ def _user_compare_server_default(
227227
cname,
228228
)
229229
return PriorityDispatchResult.STOP
230+
elif is_diff is False:
231+
# if user compare server_default returns False and not None,
232+
# it means "dont do any more server_default comparison"
233+
return PriorityDispatchResult.STOP
234+
230235
return PriorityDispatchResult.CONTINUE
231236

232237

tests/test_autogen_diffs.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -884,6 +884,63 @@ def test_server_default_no_false_positives(
884884

885885
assert not diff
886886

887+
@testing.combinations(
888+
(False, "different", False),
889+
(True, "original", True),
890+
(None, "different", True),
891+
id_="naa",
892+
argnames="return_value,new_default,expect_diff",
893+
)
894+
def test_user_compare_server_default_return_values(
895+
self, return_value, new_default, expect_diff, connection, metadata
896+
):
897+
"""Test user compare_server_default callable return values.
898+
899+
This is a regression test for #1777 where the plugin refactoring
900+
broke the handling of False return values from user-defined
901+
compare_server_default callables.
902+
903+
- False: stop comparison, no diff detected
904+
- True: diff detected regardless of actual difference
905+
- None: continue to default comparison logic
906+
"""
907+
908+
def my_compare_server_default(
909+
context,
910+
inspected_col,
911+
metadata_col,
912+
inspected_default,
913+
metadata_default,
914+
rendered_metadata_default,
915+
):
916+
return return_value
917+
918+
t1 = Table(
919+
"t1",
920+
metadata,
921+
Column("x", VARCHAR(30), server_default=text("'original'")),
922+
)
923+
t1.create(connection)
924+
925+
new_metadata = MetaData()
926+
Table(
927+
"t1",
928+
new_metadata,
929+
Column("x", VARCHAR(30), server_default=text(f"'{new_default}'")),
930+
)
931+
932+
mc = MigrationContext.configure(
933+
connection,
934+
opts={"compare_server_default": my_compare_server_default},
935+
)
936+
937+
diff = api.compare_metadata(mc, new_metadata)
938+
if expect_diff:
939+
eq_(len(diff), 1)
940+
eq_(diff[0][0][0], "modify_default")
941+
else:
942+
assert not diff
943+
887944

888945
class CompareMetadataToInspectorTest(TestBase):
889946
__backend__ = True

0 commit comments

Comments
 (0)