From d937ff5fe6a75b5e7cbe34f3cb4ac7e65bb81ec1 Mon Sep 17 00:00:00 2001 From: butubb <1422726308@qq.com> Date: Wed, 7 Oct 2026 15:35:40 +0800 Subject: [PATCH] =?UTF-8?q?fix(db):=20=5Fensure=5Fcolumns=20=E6=94=B9?= =?UTF-8?q?=E4=B8=BA=E6=8C=89=E6=A8=A1=E5=9E=8B=E5=85=83=E6=95=B0=E6=8D=AE?= =?UTF-8?q?=E6=8E=A8=E5=AF=BC=EF=BC=8C=E5=B9=B6=E8=A1=A5=E4=B8=8A=E6=BC=8F?= =?UTF-8?q?=E5=8A=A0=E7=9A=84=E8=B0=83=E5=BA=A6=E5=AD=97=E6=AE=B5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 上一个提交加了 4 个调度字段，却没在 _ADDED_COLUMNS 里登记，后果是生产环境： - 任务列表接口报 Unknown column，UI 打不开任务列表 - 调度器每 20 秒 tick 一次炸一次，定时任务完全不会触发 最阴险的是启动完全正常——能连库、能起来，只是随后每条查询都失败。 - _ensure_columns 不再遍历手写清单，改为遍历 MonitorBase.metadata.sorted_tables， 从根上消掉「加了字段忘了登记」这类漏 - 新增 _column_ddl：用 CreateColumn 渲染类型与可空性，并给 NOT NULL 列补 DEFAULT。 模型的 default= 是 ORM 侧行为、不会进 DDL，而给已有数据的表加 NOT NULL 列必须有值， 否则能否成功取决于服务端 sql_mode - 主键列跳过：MySQL 不允许 AUTO_INCREMENT 与 DEFAULT 共存 新增 tests/test_monitor_column_migration.py 守住：每个 NOT NULL 列都必须能生成带 DEFAULT 的合法 ALTER。 --- api/monitor/db.py | 77 ++++++++++++++++++++----- tests/test_monitor_column_migration.py | 78 ++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 tests/test_monitor_column_migration.py diff --git a/api/monitor/db.py b/api/monitor/db.py index df0ac81..4c04996 100644 --- a/api/monitor/db.py +++ b/api/monitor/db.py @@ -44,7 +44,9 @@ from contextlib import asynccontextmanager from pathlib import Path from typing import AsyncIterator, Optional -from sqlalchemy import event, text +from sqlalchemy import Column, event, text +from sqlalchemy.dialects import mysql +from sqlalchemy.schema import CreateColumn from sqlalchemy.ext.asyncio import ( AsyncEngine, AsyncSession, @@ -214,14 +216,53 @@ async def init_db() -> None: await _migrate_setting_keys(conn) -# Columns added to a table after it may already exist. ``create_all`` only -# creates missing *tables*, so new columns need an explicit ALTER TABLE. -_ADDED_COLUMNS: dict[str, list[tuple[str, str]]] = { - "monitor_task": [ - ("notify_enabled", "BOOLEAN NOT NULL DEFAULT 0"), - ("last_notified_at", "BIGINT NULL"), - ], -} +# ``create_all`` creates missing *tables* but never adds *columns* to a table that +# already exists, so those need an explicit ALTER TABLE. +# +# Which columns those are is derived from the ORM metadata, not kept by hand. The +# hand-kept version was a trap: forgetting to register a new column there still let +# the app start -- it connects fine, then fails on every query and every scheduler +# tick. Which is exactly what happened when the scheduling columns were added. + + +def _implicit_default(column: Column) -> Optional[str]: + """A literal to seed existing rows with when a NOT NULL column is added.""" + default = column.default + if default is not None and getattr(default, "is_scalar", False): + value = default.arg + if isinstance(value, bool): + return "1" if value else "0" + if isinstance(value, (int, float)): + return str(value) + return "'" + str(value).replace("'", "''") + "'" + + # No scalar default on the model. Fall back to the type's zero value, so that + # adding the column cannot depend on the server's sql_mode. + try: + python_type = column.type.python_type + except NotImplementedError: + return None + if python_type in (bool, int, float): + return "0" + if python_type is str: + return "''" + return None + + +def _column_ddl(column: Column) -> str: + """One column as MySQL DDL for ``ALTER TABLE ... ADD COLUMN``. + + ``CreateColumn`` renders the name, type and nullability. The default is added + separately because a model's ``default=`` is applied by the ORM and never + reaches the DDL -- and a NOT NULL column added to a populated table needs a + value for the rows already sitting there. + """ + ddl = str(CreateColumn(column).compile(dialect=mysql.dialect())) + if not column.nullable and column.server_default is None: + seed = _implicit_default(column) + if seed is not None: + ddl += f" DEFAULT {seed}" + return ddl async def _existing_columns(conn, table: str) -> set[str]: @@ -240,14 +281,22 @@ async def _existing_columns(conn, table: str) -> set[str]: async def _ensure_columns(conn) -> None: - for table, columns in _ADDED_COLUMNS.items(): - existing = await _existing_columns(conn, table) + """Add every model column the live table is missing.""" + for table in MonitorBase.metadata.sorted_tables: + existing = await _existing_columns(conn, table.name) if not existing: # Table did not exist before this run; create_all built it complete. continue - for name, ddl in columns: - if name not in existing: - await conn.execute(text(f"ALTER TABLE {table} ADD COLUMN {name} {ddl}")) + for column in table.columns: + # Primary keys are always present, and MySQL rejects AUTO_INCREMENT + # alongside the DEFAULT this helper appends -- so skip them rather + # than emit DDL that could never run. + if column.name in existing or column.primary_key: + continue + print(f"[monitor.db] 补齐缺失字段 {table.name}.{column.name}", flush=True) + await conn.execute( + text(f"ALTER TABLE {table.name} ADD COLUMN {_column_ddl(column)}") + ) async def _migrate_setting_keys(conn) -> None: diff --git a/tests/test_monitor_column_migration.py b/tests/test_monitor_column_migration.py new file mode 100644 index 0000000..bb1967a --- /dev/null +++ b/tests/test_monitor_column_migration.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +"""Guards for the in-place column migration. + +``create_all`` creates missing tables but never adds columns to a table that +already exists, so new model columns are applied by ``_ensure_columns``. That step +was originally driven by a hand-kept list, and forgetting to update it did not +fail loudly -- the app still started, connected, and then failed on every query +and every scheduler tick. These tests pin down its replacement, which derives the +work from the model metadata. +""" + +import pytest +from sqlalchemy import Boolean, Column, Integer, MetaData, String, Table + +from api.monitor import db +from api.monitor.models import MonitorBase + + +def _migratable_columns(): + for table in MonitorBase.metadata.sorted_tables: + for column in table.columns: + if column.primary_key: + continue + yield pytest.param(column, id=f"{table.name}.{column.name}") + + +def _ddl(column: Column) -> str: + """Render a detached column, so the tests never mutate the real metadata.""" + scratch = Table("scratch", MetaData(), column) + return db._column_ddl(scratch.columns[0]) + + +@pytest.mark.parametrize("column", _migratable_columns()) +def test_column_renders_as_ddl(column): + ddl = db._column_ddl(column) + + assert ddl.startswith(column.name) + # MySQL refuses AUTO_INCREMENT together with the DEFAULT this helper appends + # to NOT NULL columns. + assert "AUTO_INCREMENT" not in ddl + + +@pytest.mark.parametrize("column", _migratable_columns()) +def test_not_null_columns_carry_a_default(column): + """So ADD COLUMN cannot fail on a table that already holds rows. + + Without a DEFAULT, whether the ALTER succeeds depends on the server's + sql_mode -- not something a deployment should hinge on. + """ + if column.nullable: + pytest.skip("nullable column needs no seed value") + + assert "DEFAULT" in db._column_ddl(column) + + +def test_boolean_default_becomes_a_mysql_literal(): + """Python's True is not a SQL keyword; it has to become 1.""" + assert "DEFAULT 1" in _ddl(Column("flag", Boolean, nullable=False, default=True)) + + +def test_string_defaults_are_quoted(): + assert "DEFAULT 'interval'" in _ddl( + Column("mode", String(16), nullable=False, default="interval") + ) + + +def test_integer_defaults_are_not_quoted(): + ddl = _ddl(Column("n", Integer, nullable=False, default=0)) + + assert "DEFAULT 0" in ddl + assert "DEFAULT '0'" not in ddl + + +def test_a_not_null_column_without_a_model_default_falls_back_to_zero(): + """Belt and braces: even a column the model gives no default still migrates.""" + ddl = _ddl(Column("n", Integer, nullable=False)) + + assert "DEFAULT 0" in ddl