Identity and Computed comparison skipped when compare_server_default is not set, as of 1.18.0

オープン
  • 何か問題がありますか？
#1,877 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

  • #1878 @Fire162 による — マージされずにクローズ
  • #1882 @Fire162 による — マージされずにクローズ
  • #1885 @anish-yen による — マージされずにクローズ

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
50/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
postgresql, python, sqlalchemy
領域
database

調査の方向性

Start with _user_compare_server_default() in alembic/autogenerate/compare/server_defaults.py, where the early STOP for compare_server_default is False ends the dispatch chain before the computed and identity comparators run. Compare it with the old gate in MigrationContext._compare_server_default() and the call site in alembic/autogenerate/compare.py. Done when Identity and Computed changes produce diffs under default options, with a test that sets compare_server_default to False, since AutogenFixtureTest always sets it to True. Linked PR #1878 was closed unmerged, so check why before reusing it.

索引モデルが issue の本文から書いたものです。

説明

autogenerate - defaults bug regression

Describe the bug

As of 1.18.0, autogenerate no longer detects Identity changes on an existing column, and no longer emits the "Computed default on ... cannot be modified" warning, unless compare_server_default is set to True or a callable. With the default of compare_server_default=False, adding, removing or altering an Identity produces no diff at all. In 1.17.2 and earlier these were detected regardless of that option.

Expected behavior

Identity and computed comparison should run independently of compare_server_default, as they did through 1.17.2. That option only gated the comparison of plain server defaults.

To Reproduce

import warnings

from sqlalchemy import Column
from sqlalchemy import Computed
from sqlalchemy import create_engine
from sqlalchemy import Identity
from sqlalchemy import Integer
from sqlalchemy import MetaData
from sqlalchemy import Table

import alembic
from alembic.autogenerate import compare_metadata
from alembic.migration import MigrationContext

engine = create_engine("postgresql://scott:tiger@localhost/test")


def table(metadata, *n_args):
    Table(
        "t",
        metadata,
        Column("id", Integer, primary_key=True),
        Column("bar", Integer),
        Column("n", Integer, *n_args, nullable=False),
    )


cases = [
    ("add Identity", (), (Identity(),)),
    ("remove Identity", (Identity(),), ()),
    (
        "change Identity",
        (Identity(increment=1),),
        (Identity(increment=5),),
    ),
    (
        "change Computed",
        (Computed("bar * 5", persisted=True),),
        (Computed("bar * 42", persisted=True),),
    ),
]

print(f"alembic {alembic.__version__}")
for label, in_database, in_model in cases:
    db_metadata = MetaData()
    table(db_metadata, *in_database)
    model_metadata = MetaData()
    table(model_metadata, *in_model)

    with engine.begin() as conn:
        db_metadata.drop_all(conn)
        db_metadata.create_all(conn)

    try:
        with engine.connect() as conn:
            # default options; compare_server_default is not set
            context = MigrationContext.configure(conn)
            with warnings.catch_warnings(record=True) as w:
                warnings.simplefilter("always")
                diffs = compare_metadata(context, model_metadata)
    finally:
        with engine.begin() as conn:
            db_metadata.drop_all(conn)

    diffs = [d[0] for group in diffs for d in group]
    warned = [str(x.message) for x in w if "Computed" in str(x.message)]
    print(f"{label}: diffs={diffs} warnings={warned}")

Output on 1.17.2:

alembic 1.17.2
add Identity: diffs=['modify_default'] warnings=[]
remove Identity: diffs=['modify_default'] warnings=[]
change Identity: diffs=['modify_default'] warnings=[]
change Computed: diffs=[] warnings=['Computed default on t.n cannot be modified']

Output on 1.18.0, and unchanged on current main:

alembic 1.18.0
add Identity: diffs=[] warnings=[]
remove Identity: diffs=[] warnings=[]
change Identity: diffs=[] warnings=[]
change Computed: diffs=[] warnings=[]

Cause

Before the plugin reorganization in e532a7e3, the False check was inside MigrationContext._compare_server_default(), which autogenerate/compare.py called only for plain server defaults. The computed and identity branches ran ahead of it and never consulted the option.

Those branches are now separate comparators on the "column" / "server_default" dispatch chain, and the check moved into _user_compare_server_default() in alembic/autogenerate/compare/server_defaults.py, which is registered with DispatchPriority.FIRST:

if migration_context._user_compare_server_default is False:
    return PriorityDispatchResult.STOP

STOP ends the chain, so _compare_computed_default(), _compare_identity_default() and _dialect_impl_compare_server_default() are all skipped. Only the last of those was gated by the option previously.

The test suite does not catch this because AutogenFixtureTest always configures compare_server_default=True; a test for the fix needs to run with the option at its default.

Versions

  • Alembic: 1.18.0 through 1.20.0, and current main
  • Database: PostgreSQL 18 (the Identity cases need a backend with identity columns; the dispatch logic is not backend specific)
  • SQLAlchemy: 2.1
主要言語
Python
スター
4.5k
フォーク
391
30日以内にマージされた PR はありません

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

sqlalchemy/alembic のほかの issue

sqlalchemy/alembic の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。