codeant-ai-for-open-source[bot] commented on code in PR #41176: URL: https://github.com/apache/superset/pull/41176#discussion_r3500183906
########## superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py: ########## @@ -0,0 +1,562 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""add_versioning_tables + +Creates the full schema backing entity versioning in a single +migration: + +1. ``version_transaction`` β audit log keyed by Continuum's per-flush + transaction id (plus a Postgres-specific id sequence). +2. **Parent shadow tables** mirroring each versioned entity's columns: + ``dashboards_version`` / ``slices_version`` / ``tables_version``. +3. ``version_changes`` β field-level diff log keyed to a + ``(transaction, entity)`` pair; each row describes one atomic change + (one field or one child-collection element) that occurred during a + save. +4. **Child shadow tables** for the collections Continuum auto-registers + when ``__versioned__`` is applied to ``TableColumn`` / ``SqlMetric`` + and the ``slices`` exclude is removed from + ``Dashboard.__versioned__``: ``table_columns_version`` / + ``sql_metrics_version`` / ``dashboard_slices_version``. + +All shadow tables follow the validity-strategy shape (mirrored columns ++ ``transaction_id`` / ``end_transaction_id`` / ``operation_type`` +bookkeeping with FKs to ``version_transaction.id``). The current +version row has ``end_transaction_id = NULL``. + +This migration replaces three iterative migrations from the spike phase +(``56cd24c07170``, ``e1f3c5a7b9d0``, ``f7a2b3c4d5e6``) that captured the +same schema in three steps as the feature was developed. Compacting +gives downstream operators one migration to apply / reverse and one +review surface. The ``revision`` hash is reused from the original first +migration so anyone still tracking the chain by that hash lands on the +same logical change set. + +Generated by hand because the current Continuum + Alembic-autogenerate +interaction trips on the renamed ``transaction`` -> ``version_transaction`` +table key (``KeyError`` lookups in ``table_key_to_table``). Column +inventories were sourced from the live model ``__table__`` definitions +and ``version_class(...).__table__`` / Continuum association metadata. + +Primary key choice. Both ``version_transaction.id`` and +``version_changes.id`` are ``BigInteger`` autoincrement β a deliberate +carveout from the project's UUID-PK convention for new models (see +``CLAUDE.md`` Β§"UUID Migration"). ``version_transaction`` is keyed +externally by SQLAlchemy-Continuum via +``nextval('version_transaction_id_seq')`` on every INSERT; matching +that contract is required for ``versioning_manager`` to function. +``version_changes`` follows the same shape because the user-facing +identity is the ``(transaction_id, entity_kind, entity_id, sequence)`` +composite unique key, not the row id; the API surfaces a deterministic +UUIDv5 ``version_uuid`` derived from ``entity.uuid`` and +``transaction_id`` for stable external references. + +Revision ID: 56cd24c07170 +Revises: 2bee73611e32 +Create Date: 2026-05-28 19:50:00.000000 + +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op +from sqlalchemy_utils import UUIDType + +from superset.utils.core import MediumText + +revision = "56cd24c07170" +# Stacked on the composite-PK association-tables change (2bee73611e32) so the +# Continuum shadow tables this migration creates can mirror the +# composite-PK shape of the live association tables. If that change +# is removed from the stack, this should be reverted to "ce6bd21901ab". +down_revision = "2bee73611e32" + + +def upgrade() -> None: + bind = op.get_bind() + + # ------------------------------------------------------------------ + # version_transaction + # + # Audit log for each versioning event. Continuum emits + # ``nextval('version_transaction_id_seq')`` on every INSERT, so the + # sequence must exist before the table on Postgres. SQLite/MySQL + # ignore the explicit CREATE SEQUENCE (they auto-increment natively). + # ------------------------------------------------------------------ + if bind.dialect.name == "postgresql": + op.execute("CREATE SEQUENCE IF NOT EXISTS version_transaction_id_seq") + + op.create_table( + "version_transaction", + sa.Column( + "id", + sa.BigInteger(), + sa.Sequence("version_transaction_id_seq"), + primary_key=True, + autoincrement=True, + nullable=False, + ), Review Comment: **Suggestion:** The `version_transaction.id` primary key is declared as `BigInteger` with autoincrement, but on SQLite autoincrement semantics require an `INTEGER PRIMARY KEY`; this can break transaction-row creation when version capture is enabled (missing/invalid generated ids). Use a SQLite-specific integer variant for this PK so inserts are reliably auto-generated across dialects. [type error] <details> <summary><b>Severity Level:</b> Critical π¨</summary> ```mdx - β SQLite version_transaction inserts may fail under capture. - β οΈ Dataset update endpoint errors when capture enabled. ``` </details> <details> <summary><b>Steps of Reproduction β </b></summary> ```mdx 1. Apply the versioning schema migration `superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py` to a Superset instance using SQLite for its metadata DB; `upgrade()` at lines 90-131 creates `version_transaction` with `id` defined at lines 106-113 as `sa.Column("id", sa.BigInteger(), sa.Sequence("version_transaction_id_seq"), primary_key=True, autoincrement=True, nullable=False)`. 2. Enable version capture in application initialization by setting `ENABLE_VERSIONING_CAPTURE = True` in `superset_config.py`, which causes `SupersetAppInitializer.init_versioning()` in `superset/initialization/__init__.py:690-729` to register the baseline and change-record listeners instead of detaching them when the flag is False. 3. Observe that SQLAlchemy-Continuumβs `VersionTransactionFactory` in `superset/versioning/factory.py:13-39` renames its transaction table to `version_transaction` and wires the default sequence name `version_transaction_id_seq` for the `id` column; with capture enabled, every flush that modifies a versioned model (e.g. dashboards, charts, datasets) attempts to insert a row into `version_transaction`. 4. On a SQLite-backed deployment, the DDL from lines 106-113 creates a `BigInteger` primary key rather than an `INTEGER PRIMARY KEY` rowid alias, and SQLite ignores the explicit `CREATE SEQUENCE` from lines 12-13; when an API endpoint like `PUT /api/v1/dataset/<id>` (implemented in `superset/datasets/api.py:500-538`, which performs `UpdateDatasetCommand(pk, item, override_columns).run()`) triggers a versioning flush, inserts into `version_transaction` rely on autoincremented ids and can fail or produce invalid ids because SQLite does not auto-generate values for this non-`INTEGER PRIMARY KEY` configuration, breaking version capture and potentially causing request failures. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b0c691886a424a2d92a7286e40133e02&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b0c691886a424a2d92a7286e40133e02&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent π€ </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py **Line:** 106:113 **Comment:** *Type Error: The `version_transaction.id` primary key is declared as `BigInteger` with autoincrement, but on SQLite autoincrement semantics require an `INTEGER PRIMARY KEY`; this can break transaction-row creation when version capture is enabled (missing/invalid generated ids). Use a SQLite-specific integer variant for this PK so inserts are reliably auto-generated across dialects. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41176&comment_hash=6f41961de3ec9f711c5a9bbb54b967b93aea6b5223c9df07d969c5ac047a7761&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41176&comment_hash=6f41961de3ec9f711c5a9bbb54b967b93aea6b5223c9df07d969c5ac047a7761&reaction=dislike'>π</a> ########## superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py: ########## @@ -0,0 +1,580 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""composite_pk_association_tables + +Replace the unused synthetic ``id INTEGER PRIMARY KEY`` on eight many-to-many +association tables with a composite primary key on the two FK columns. Drops +the now-redundant ``UniqueConstraint(fk1, fk2)`` on the two tables that +already carry one. Pre-flight: deletes rows with NULL FK values (six tables +allow them today) and any duplicate ``(fk1, fk2)`` rows. + +Motivated by SQLAlchemy-Continuum issue #129 (M2M restore against junction +tables with surrogate PKs); also closes the data-integrity hole where six +of the eight tables lacked DB-level uniqueness. + +Revision ID: 2bee73611e32 +Revises: 78a40c08b4be +Create Date: 2026-05-01 23:36:34.050058 + +""" + +import logging +from typing import NamedTuple + +import sqlalchemy as sa +from alembic import op +from alembic.operations.base import BatchOperations +from sqlalchemy import inspect +from sqlalchemy.engine import Connection + +# revision identifiers, used by Alembic. +revision = "2bee73611e32" +down_revision = "78a40c08b4be" + +logger = logging.getLogger("alembic.env") + + +class AssociationTable(NamedTuple): + """A junction table being converted from surrogate-id PK to composite-FK PK.""" + + name: str + fk1: str + fk2: str + + +# Order is alphabetical by table name; deterministic for review and bisection. +AFFECTED_TABLES: list[AssociationTable] = [ + AssociationTable("dashboard_roles", "dashboard_id", "role_id"), + AssociationTable("dashboard_slices", "dashboard_id", "slice_id"), + AssociationTable("dashboard_user", "user_id", "dashboard_id"), + AssociationTable("report_schedule_user", "user_id", "report_schedule_id"), + AssociationTable("rls_filter_roles", "role_id", "rls_filter_id"), + AssociationTable("rls_filter_tables", "table_id", "rls_filter_id"), + AssociationTable("slice_user", "user_id", "slice_id"), + AssociationTable("sqlatable_user", "user_id", "table_id"), +] + +# These two tables already declare ``UniqueConstraint(fk1, fk2)`` in the model; +# the composite PK subsumes it, so the migration drops the redundant constraint. +TABLES_WITH_PRE_EXISTING_UNIQUE: set[str] = { + "dashboard_slices", + "report_schedule_user", +} + +# Documentation set: tables whose FK columns are nullable in their original +# create_table migrations (``dashboard_roles.dashboard_id`` from revision +# e11ccdd12658 is the most recent addition). ``report_schedule_user`` is the +# only affected table created with both FK columns ``NOT NULL`` and is +# intentionally absent here. This set is no longer consulted at runtime β the +# upgrade now runs the NULL-FK cleanup on every affected table because the +# DELETE is a cheap no-op when the columns are already NOT NULL, and that +# eliminates the risk of bugs from this set going stale (the +# ``dashboard_roles`` omission caught in PR review was exactly that bug). +TABLES_WITH_NULLABLE_FKS: set[str] = { + "dashboard_roles", + "dashboard_slices", + "dashboard_user", + "rls_filter_roles", + "rls_filter_tables", + "slice_user", + "sqlatable_user", +} + + +def _check_no_external_fks_to_id(conn: Connection) -> None: + """Raise ``RuntimeError`` if any foreign key in the database references one + of the eight junction-table ``id`` columns. Uses SQLAlchemy's ``Inspector`` + for dialect-agnostic introspection across PostgreSQL, MySQL, and SQLite. + + Scope limitation: ``Inspector.get_table_names()`` returns tables in the + connection's default schema only. On PostgreSQL deployments where Superset + metadata lives in a non-default schema, or on multi-schema deployments + that allow cross-schema FKs, an external FK in another schema would not + be detected. This is acceptable for the standard single-schema + deployment that Superset documents; operators with multi-schema + metadata should run the equivalent inventory query against + ``information_schema.referential_constraints`` themselves before + applying. + """ + affected = {t.name for t in AFFECTED_TABLES} + insp = inspect(conn) + for table_name in insp.get_table_names(): + if table_name in affected: + continue + for fk in insp.get_foreign_keys(table_name): + if fk["referred_table"] in affected and "id" in fk["referred_columns"]: + raise RuntimeError( + f"Cannot drop synthetic id from {fk['referred_table']}: " + f"external FK {fk.get('name', '<unnamed>')} on {table_name} " + f"references {fk['referred_table']}({fk['referred_columns']}). " + "Drop or migrate the referencing FK before applying this " + "migration." + ) + + +def _table_clause(t: AssociationTable) -> sa.sql.expression.TableClause: + """Build a lightweight SQLAlchemy ``TableClause`` for ``t`` exposing the + columns the helper queries reference (``id``, ``fk1``, ``fk2``). Used so + that the dedupe / cleanup / assert SQL can be expressed via SQLAlchemy + core constructs rather than via string interpolation.""" + return sa.table(t.name, sa.column("id"), sa.column(t.fk1), sa.column(t.fk2)) + + +def _delete_null_fk_rows(conn: Connection, t: AssociationTable) -> int: + """Delete rows where ``t.fk1`` or ``t.fk2`` is NULL on ``t.name``. + + Returns the deletion count. Required because primary-key columns must be + NOT NULL; the PK-add downstream would fail with a cryptic constraint + violation if any NULL-FK rows survived. Run unconditionally on every + affected table β see ``TABLES_WITH_NULLABLE_FKS`` above for the rationale. + """ + tbl = _table_clause(t) + stmt = sa.delete(tbl).where(sa.or_(tbl.c[t.fk1].is_(None), tbl.c[t.fk2].is_(None))) + result = conn.execute(stmt) + n = result.rowcount or 0 + if n: + logger.warning( + "Deleted %d row(s) with NULL FK from %s before composite-PK promotion", + n, + t.name, + ) + return n + + +def _dedupe_by_min_id(conn: Connection, t: AssociationTable) -> int: + """Delete duplicate ``(t.fk1, t.fk2)`` rows from ``t.name`` keeping ``MIN(id)``. + + Returns the deletion count. The ``NOT IN`` argument is wrapped in an + extra ``SELECT keep_id FROM (...) AS s`` derived table because MySQL + rejects ``DELETE FROM t WHERE id NOT IN (SELECT MIN(id) FROM t GROUP BY + ...)`` with ERROR 1093 unless the inner SELECT is materialized through + a derived table. SQLAlchemy's ``.subquery()`` produces that wrap. + + Logs a sample (up to 10) of the discarded ``(fk1, fk2, id)`` tuples at + WARN before deletion, so operators can audit which rows are dropped β + the "keep ``MIN(id)``" policy preserves the original row, which is + correct in practice but discards any later, semantically-identical + re-grants. + """ + tbl = _table_clause(t) + + keep_min = ( + sa.select(sa.func.min(tbl.c.id).label("keep_id")) + .group_by(tbl.c[t.fk1], tbl.c[t.fk2]) + .subquery("keep_min") + ) + keep_ids = sa.select(keep_min.c.keep_id) + discarded = tbl.c.id.notin_(keep_ids) + + sample_stmt = ( + sa.select(tbl.c[t.fk1], tbl.c[t.fk2], tbl.c.id).where(discarded).limit(10) + ) + sample = list(conn.execute(sample_stmt)) + + delete_stmt = sa.delete(tbl).where(discarded) + result = conn.execute(delete_stmt) + n = result.rowcount or 0 + if n: + logger.warning( + "Deduped %d duplicate row(s) from %s; sample of discarded " + "(%s, %s, id) tuples (up to 10): %s", + n, + t.name, + t.fk1, + t.fk2, + sample, + ) + return n + + +def _assert_no_duplicates(conn: Connection, t: AssociationTable) -> None: + """Raise ``RuntimeError`` if any ``(t.fk1, t.fk2)`` duplicate group remains. + + Called after ``_dedupe_by_min_id`` to surface silent dialect-dependent + dedupe failures (e.g., a MySQL syntax issue) as an actionable error + before the PK-add fires with a less-helpful constraint-violation message. + """ + tbl = _table_clause(t) + duplicate_groups = ( + sa.select(sa.literal(1)) + .select_from(tbl) + .group_by(tbl.c[t.fk1], tbl.c[t.fk2]) + .having(sa.func.count() > 1) + .subquery("duplicate_groups") + ) + count_stmt = sa.select(sa.func.count()).select_from(duplicate_groups) + if remaining := conn.scalar(count_stmt) or 0: + raise RuntimeError( + f"Dedupe failed for {t.name}: {remaining} duplicate " + f"({t.fk1}, {t.fk2}) groups remain after _dedupe_by_min_id. " + f"Check the dedupe SQL for dialect {conn.dialect.name}." + ) + + +def _build_pre_upgrade_table( + insp: sa.engine.reflection.Inspector, + t: AssociationTable, + fks: list[dict] | None = None, +) -> sa.Table: + """Build a ``Table`` object representing the pre-upgrade schema of ``t``, + explicitly *without* any redundant ``UniqueConstraint(t.fk1, t.fk2)``. + Used as ``copy_from`` to ``batch_alter_table`` so the rebuilt table + omits the unnamed UNIQUE constraint deterministically across dialects + (SQLite reflects unnamed UNIQUEs with ``name=None``, defeating the + standard ``batch_op.drop_constraint(name)`` path). + + Reflects column types and FK targets (with original FK constraint names + preserved) from the live database; only the redundant UNIQUE is omitted. + + *fks* lets a caller pass a pre-captured ``get_foreign_keys`` result. + The MySQL upgrade path drops the live FK constraints before building + this table, so re-reflecting here would only see them via the + Inspector's per-instance ``info_cache`` β an implementation detail, + not a contract. Passing the pre-drop list makes the dependency + explicit instead of relying on reflection caching. + """ + md = sa.MetaData() + if fks is None: + fks = insp.get_foreign_keys(t.name) + fks_for_col: dict[str, list[dict]] = {} + for fk in fks: + for col_name in fk["constrained_columns"]: + fks_for_col.setdefault(col_name, []).append(fk) + + cols: list[sa.Column] = [] + for c in insp.get_columns(t.name): + col_kwargs = {"nullable": c.get("nullable", True)} + if c["name"] == "id": + col_kwargs["primary_key"] = True + col_kwargs["autoincrement"] = True + fk_args = [] + for fk in fks_for_col.get(c["name"], []): + idx = fk["constrained_columns"].index(c["name"]) + target = f"{fk['referred_table']}.{fk['referred_columns'][idx]}" + options = {} + if fk.get("options", {}).get("ondelete"): + options["ondelete"] = fk["options"]["ondelete"] + if fk.get("name"): + options["name"] = fk["name"] + fk_args.append(sa.ForeignKey(target, **options)) + cols.append(sa.Column(c["name"], c["type"], *fk_args, **col_kwargs)) + return sa.Table(t.name, md, *cols) + + +def _drop_redundant_unique_by_name( + conn: Connection, insp: sa.engine.reflection.Inspector, t: AssociationTable +) -> None: + """Drop the redundant ``UNIQUE(fk1, fk2)`` constraint by its reflected + name on PostgreSQL / MySQL. + + The two tables in ``TABLES_WITH_PRE_EXISTING_UNIQUE`` carry a UNIQUE + constraint that the composite primary key subsumes. PostgreSQL and + MySQL both auto-name UNIQUE constraints (``<table>_<cols>_key`` on + Postgres, ``<table>_<col>_<n>`` or the explicit ``uq_*`` we may have + given it on MySQL), so they're reflectable by name. SQLite is + handled separately via ``recreate="always"`` + ``copy_from`` because + it reflects unnamed UNIQUEs with ``name=None``. + + No-op if no matching UNIQUE is found (defensive β re-runs after a + partial application should not error). + """ + for uc in insp.get_unique_constraints(t.name): + if set(uc.get("column_names", [])) == {t.fk1, t.fk2} and uc.get("name"): + op.drop_constraint(uc["name"], t.name, type_="unique") + return + + +# MySQL ON DELETE actions that the downgrade re-create loop is allowed +# to interpolate into raw SQL. The reflected value comes from MySQL's +# information_schema (so not user input), but a whitelist eliminates +# the "what if an unexpected value appears" question entirely. The +# four entries are the SQL-standard set; SET DEFAULT is intentionally +# excluded because InnoDB silently downgrades it to NO ACTION. +_VALID_ONDELETE_ACTIONS: frozenset[str] = frozenset( + {"CASCADE", "SET NULL", "RESTRICT", "NO ACTION"} +) + + +def _enforce_not_null_for_sqlite( + batch_op: BatchOperations, t: AssociationTable, conn: Connection +) -> None: + """Force ``NOT NULL`` on the FK columns post-PK-promotion on SQLite only. + + SQLite has a long-standing quirk: composite ``PRIMARY KEY`` does not + promote constituent columns to ``NOT NULL`` (only ``INTEGER PRIMARY KEY`` + does). PostgreSQL and MySQL implicitly promote the PK columns to + ``NOT NULL`` when the constraint is added, making the explicit + ``alter_column`` redundant there. + + Skipping the ``alter_column`` on MySQL is also functionally required: + MySQL 8 rejects ``ALTER COLUMN`` on a column that participates in a + foreign key constraint with ``ERROR 1832 (HY000): Cannot change column + 'X': used in a foreign key constraint 'Y'`` whenever the table has + data β even when the only change is ``NULL`` β ``NOT NULL`` and the + column is already part of a freshly-added composite primary key (which + InnoDB has just made implicitly ``NOT NULL`` anyway). The error fires + on populated tables but not on empty ones, which is why CI's + ``test-mysql`` shard (fresh schema) didn't catch this and a real + production-shaped install does. + + Only SQLite still needs the explicit step, and SQLite has no FK + enforcement objection. + """ + if conn.dialect.name == "sqlite": + batch_op.alter_column(t.fk1, existing_type=sa.Integer, nullable=False) + batch_op.alter_column(t.fk2, existing_type=sa.Integer, nullable=False) + + +def upgrade() -> None: + conn = op.get_bind() + _check_no_external_fks_to_id(conn) + insp = inspect(conn) + + for t in AFFECTED_TABLES: + # Resumability guard: on MySQL every DDL statement auto-commits, so + # a failure at table N of 8 leaves tables 1..N-1 already converted + # while ``alembic_version`` is still un-stamped. Without this guard + # a re-run would fail at table 1 (``drop_column("id")`` on a table + # that no longer has ``id``), and ``downgrade`` can't run either + # (the revision was never stamped) β recovery would need manual + # surgery. A converted table is identified by the absent ``id`` + # column; skipping it makes re-running the upgrade safe on every + # dialect (Postgres/SQLite wrap the migration in a transaction, so + # the guard is simply never hit there). + if "id" not in {c["name"] for c in insp.get_columns(t.name)}: + logger.info( + "%s: already converted (no surrogate id column); skipping", + t.name, + ) + continue + + # Run NULL-FK cleanup unconditionally: it is a no-op DELETE on tables + # whose FK columns are already NOT NULL (cheap), and skipping it on a + # table whose FK was nullable would leave the PK-add to fail with a + # cryptic constraint violation. Cf. ``TABLES_WITH_NULLABLE_FKS`` above + # for documentation of which tables are known to have nullable FKs. + _delete_null_fk_rows(conn, t) + _dedupe_by_min_id(conn, t) + _assert_no_duplicates(conn, t) + + # Two tables (``dashboard_slices``, ``report_schedule_user``) + # carry a redundant ``UNIQUE(fk1, fk2)`` that the composite PK + # subsumes. Three dialect-specific paths: + # + # * **PostgreSQL** β the UNIQUE constraint has a stable + # reflected name (Postgres default convention), so we + # ``DROP CONSTRAINT`` by name and then run the structural + # change as direct ALTER. This avoids the full-table copy + # that ``recreate="always"`` would trigger + # (``CREATE TABLE AS SELECT β DROP β RENAME``), holding + # ``ACCESS EXCLUSIVE`` only for the (much shorter) PK + # index build instead of the full copy duration. + # + # * **MySQL** β InnoDB binds the FK constraints to the + # redundant UNIQUE's underlying index for back-reference, + # so a direct ``DROP CONSTRAINT`` of the UNIQUE raises + # ``ERROR 1553``. Use ``recreate="always"`` to rebuild the + # table without the UNIQUE; drop the FKs first to dodge + # the ``ERROR 1826`` (duplicate FK constraint name) that + # the temp-table phase would otherwise provoke. The FKs + # are re-created automatically as part of ``copy_from``. + # + # * **SQLite** β unnamed UNIQUE constraints reflect with + # ``name=None`` and can't be dropped by name. Use + # ``recreate="always"`` + ``copy_from`` (omits UNIQUE). + # SQLite always rebuilds for PK changes anyway, so the + # recreate isn't extra cost there. + if t.name in TABLES_WITH_PRE_EXISTING_UNIQUE: + if conn.dialect.name == "postgresql": + _drop_redundant_unique_by_name(conn, insp, t) + with op.batch_alter_table(t.name) as batch_op: + batch_op.drop_column("id") + batch_op.create_primary_key(f"pk_{t.name}", [t.fk1, t.fk2]) + _enforce_not_null_for_sqlite(batch_op, t, conn) + else: + # Capture the FK list BEFORE dropping: the copy_from table + # below must embed these constraints, and re-reflecting + # after the drop only works via the Inspector's + # per-instance info_cache (see _build_pre_upgrade_table). + pre_drop_fks = insp.get_foreign_keys(t.name) + if conn.dialect.name == "mysql": + for fk in pre_drop_fks: + if fk_name := fk.get("name"): + op.drop_constraint(fk_name, t.name, type_="foreignkey") + with op.batch_alter_table( + t.name, + recreate="always", + copy_from=_build_pre_upgrade_table(insp, t, fks=pre_drop_fks), + ) as batch_op: + batch_op.drop_column("id") + batch_op.create_primary_key(f"pk_{t.name}", [t.fk1, t.fk2]) + _enforce_not_null_for_sqlite(batch_op, t, conn) + else: + with op.batch_alter_table(t.name) as batch_op: + batch_op.drop_column("id") + batch_op.create_primary_key(f"pk_{t.name}", [t.fk1, t.fk2]) + _enforce_not_null_for_sqlite(batch_op, t, conn) + + +def downgrade() -> None: + # Inverse order: undo upgrade transformations from last-applied to + # first-applied. Within each table, drop the composite PK, restore the + # surrogate ``id`` column, and re-add the original ``UNIQUE`` constraint + # on the two tables that previously carried one. + # + # Note: FK columns remain NOT NULL after downgrade (intentional asymmetry + # β see UPDATING.md). Restoring the original nullable state would require + # an explicit ``alter_column`` per FK per table for no operator value; + # junction-table NULL FKs were always meaningless under ``secondary=`` + # semantics. + # The downgrade names the restored PK ``<table>_pkey`` (matching Postgres' + # default constraint-naming convention, which was the original constraint + # name before this migration ran) so a downgrade-then-upgrade round-trip + # doesn't collide on the upgrade's ``pk_<table>`` name. + # + # Adding a NOT NULL ``id`` column to a table with existing rows requires + # a default that fires on the existing rows. ``sa.Identity()`` (Postgres + # 10+ / MySQL 8+) and ``sa.Sequence`` (with explicit nextval) both + # backfill existing rows during ALTER TABLE; bare ``autoincrement=True`` + # does not. ``Identity`` is the modern portable choice. + conn = op.get_bind() + insp = inspect(conn) + is_mysql = conn.dialect.name == "mysql" + for t in reversed(AFFECTED_TABLES): + if is_mysql: + _downgrade_mysql_table(insp, t) + else: + with op.batch_alter_table(t.name) as batch_op: + batch_op.drop_constraint(f"pk_{t.name}", type_="primary") + batch_op.add_column( + sa.Column( + "id", + sa.Integer, + sa.Identity(always=False), + nullable=False, + ) + ) Review Comment: **Suggestion:** The non-MySQL downgrade path unconditionally uses `sa.Identity(...)` when re-adding `id`; SQLite does not support identity columns in this form, so downgrade can fail or produce non-generated ids on SQLite installations. Add a SQLite-specific downgrade branch that restores `id` with SQLite-compatible PK generation. [api mismatch] <details> <summary><b>Severity Level:</b> Major β οΈ</summary> ```mdx - β SQLite downgrade may leave junction rows without ids. - β οΈ Rolling back composite-PK migration unsafe on SQLite. ``` </details> <details> <summary><b>Steps of Reproduction β </b></summary> ```mdx 1. Use the SQLite-focused migration test helper in `superset/tests/integration_tests/migrations/composite_pk_round_trip__tests.py:87-102`, which imports the migration module `2026-05-01_23-36_2bee73611e32_composite_pk_association_tables` and exposes `_migration.upgrade` / `_migration.downgrade` for an in-memory SQLite engine via `_run_with_alembic_context()`. 2. Create the pre-migration schema with existing data: call `_build_pre_migration_schema(engine)` from `composite_pk_round_trip__tests.py:50-72` to create the eight association tables, then insert rows into one or more of them (similar to the data setup in `test_migration_module_constants_are_consistent()` and `test_upgrade_scrubs_null_fks_and_duplicates()` at lines 153-180) so there are populated junction tables prior to downgrade. 3. Apply the composite-PK migration and then downgrade on SQLite by calling `_run_with_alembic_context(engine, _migration.upgrade)` followed by `_run_with_alembic_context(engine, _migration.downgrade)` as in `test_round_trip_against_in_memory_sqlite()` at lines 118-132; downgrade executes the non-MySQL branch in `superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py:461-470`, adding an `id` column defined as `sa.Column("id", sa.Integer, sa.Identity(always=False), nullable=False)`. 4. On SQLite, `sa.Identity()` is explicitly documented as not supported in MySQL or SQLite in `superset/migrations/versions/2025-11-04_11-26_33d7e0e21daa_add_semantic_layers_and_views.py:4-7`, so relying on it for backfilling existing NOT NULL `id` values during downgrade risks either leaving existing rows with NULL or non-generated ids or raising DDL errors; subsequent inserts into downgraded junction tables will then hit NOT NULL or PK violations, breaking rollback behavior on SQLite. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=261b0660a03d4bc68c80f70ea72d2999&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=261b0660a03d4bc68c80f70ea72d2999&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent π€ </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py **Line:** 461:470 **Comment:** *Api Mismatch: The non-MySQL downgrade path unconditionally uses `sa.Identity(...)` when re-adding `id`; SQLite does not support identity columns in this form, so downgrade can fail or produce non-generated ids on SQLite installations. Add a SQLite-specific downgrade branch that restores `id` with SQLite-compatible PK generation. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41176&comment_hash=3be5de6cc82f4e9798c9dae63a65baea3507f440c2d1938626f7c512fdd1f58c&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41176&comment_hash=3be5de6cc82f4e9798c9dae63a65baea3507f440c2d1938626f7c512fdd1f58c&reaction=dislike'>π</a> ########## superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py: ########## @@ -0,0 +1,562 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""add_versioning_tables + +Creates the full schema backing entity versioning in a single +migration: + +1. ``version_transaction`` β audit log keyed by Continuum's per-flush + transaction id (plus a Postgres-specific id sequence). +2. **Parent shadow tables** mirroring each versioned entity's columns: + ``dashboards_version`` / ``slices_version`` / ``tables_version``. +3. ``version_changes`` β field-level diff log keyed to a + ``(transaction, entity)`` pair; each row describes one atomic change + (one field or one child-collection element) that occurred during a + save. +4. **Child shadow tables** for the collections Continuum auto-registers + when ``__versioned__`` is applied to ``TableColumn`` / ``SqlMetric`` + and the ``slices`` exclude is removed from + ``Dashboard.__versioned__``: ``table_columns_version`` / + ``sql_metrics_version`` / ``dashboard_slices_version``. + +All shadow tables follow the validity-strategy shape (mirrored columns ++ ``transaction_id`` / ``end_transaction_id`` / ``operation_type`` +bookkeeping with FKs to ``version_transaction.id``). The current +version row has ``end_transaction_id = NULL``. + +This migration replaces three iterative migrations from the spike phase +(``56cd24c07170``, ``e1f3c5a7b9d0``, ``f7a2b3c4d5e6``) that captured the +same schema in three steps as the feature was developed. Compacting +gives downstream operators one migration to apply / reverse and one +review surface. The ``revision`` hash is reused from the original first +migration so anyone still tracking the chain by that hash lands on the +same logical change set. + +Generated by hand because the current Continuum + Alembic-autogenerate +interaction trips on the renamed ``transaction`` -> ``version_transaction`` +table key (``KeyError`` lookups in ``table_key_to_table``). Column +inventories were sourced from the live model ``__table__`` definitions +and ``version_class(...).__table__`` / Continuum association metadata. + +Primary key choice. Both ``version_transaction.id`` and +``version_changes.id`` are ``BigInteger`` autoincrement β a deliberate +carveout from the project's UUID-PK convention for new models (see +``CLAUDE.md`` Β§"UUID Migration"). ``version_transaction`` is keyed +externally by SQLAlchemy-Continuum via +``nextval('version_transaction_id_seq')`` on every INSERT; matching +that contract is required for ``versioning_manager`` to function. +``version_changes`` follows the same shape because the user-facing +identity is the ``(transaction_id, entity_kind, entity_id, sequence)`` +composite unique key, not the row id; the API surfaces a deterministic +UUIDv5 ``version_uuid`` derived from ``entity.uuid`` and +``transaction_id`` for stable external references. + +Revision ID: 56cd24c07170 +Revises: 2bee73611e32 +Create Date: 2026-05-28 19:50:00.000000 + +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op +from sqlalchemy_utils import UUIDType + +from superset.utils.core import MediumText + +revision = "56cd24c07170" +# Stacked on the composite-PK association-tables change (2bee73611e32) so the +# Continuum shadow tables this migration creates can mirror the +# composite-PK shape of the live association tables. If that change +# is removed from the stack, this should be reverted to "ce6bd21901ab". +down_revision = "2bee73611e32" + + +def upgrade() -> None: + bind = op.get_bind() + + # ------------------------------------------------------------------ + # version_transaction + # + # Audit log for each versioning event. Continuum emits + # ``nextval('version_transaction_id_seq')`` on every INSERT, so the + # sequence must exist before the table on Postgres. SQLite/MySQL + # ignore the explicit CREATE SEQUENCE (they auto-increment natively). + # ------------------------------------------------------------------ + if bind.dialect.name == "postgresql": + op.execute("CREATE SEQUENCE IF NOT EXISTS version_transaction_id_seq") + + op.create_table( + "version_transaction", + sa.Column( + "id", + sa.BigInteger(), + sa.Sequence("version_transaction_id_seq"), + primary_key=True, + autoincrement=True, + nullable=False, + ), + sa.Column("issued_at", sa.DateTime(), nullable=True), + sa.Column("remote_addr", sa.String(50), nullable=True), + sa.Column("user_id", sa.Integer(), nullable=True), + # ``action_kind`` carries the high-level avenue that produced + # this transaction (``restore`` / ``import`` / ``clone``). + # ``NULL`` is the default "ordinary save" β most rows leave + # this empty. Commands set + # ``session.info["_versioning_action_kind"]`` before commit; + # the change-record listener stamps the value here. Parallel + # to ``version_changes.entity_kind`` and ``version_changes.kind`` + # β the schema's third ``*_kind`` column, at transaction scope. + sa.Column("action_kind", sa.String(32), nullable=True), + ) + + if bind.dialect.name == "postgresql": + op.execute( + "ALTER SEQUENCE version_transaction_id_seq OWNED BY version_transaction.id" + ) + + # ------------------------------------------------------------------ + # dashboards_version + # ------------------------------------------------------------------ + op.create_table( + "dashboards_version", + sa.Column("uuid", UUIDType(binary=True), nullable=True), + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("dashboard_title", sa.String(500), nullable=True), + # ``MediumText()`` mirrors the live column type β on MySQL plain + # ``TEXT`` caps at 64 KB, which large dashboards exceed; an + # oversized live write would then fail the shadow INSERT under + # ``STRICT_TRANS_TABLES`` (or silently truncate without it) and + # corrupt the history. Postgres ``TEXT`` is unbounded and SQLite + # ignores the length annotation so this is MySQL-driven. + sa.Column("position_json", MediumText(), nullable=True), + sa.Column("description", sa.Text(), nullable=True), + sa.Column("css", MediumText(), nullable=True), + sa.Column("theme_id", sa.Integer(), nullable=True), + sa.Column("certified_by", sa.Text(), nullable=True), + sa.Column("certification_details", sa.Text(), nullable=True), + sa.Column("json_metadata", MediumText(), nullable=True), + sa.Column("slug", sa.String(255), nullable=True), + sa.Column("published", sa.Boolean(), nullable=True), + sa.Column("is_managed_externally", sa.Boolean(), nullable=True), + sa.Column("external_url", sa.Text(), nullable=True), + sa.Column("transaction_id", sa.BigInteger(), nullable=False), + sa.Column("end_transaction_id", sa.BigInteger(), nullable=True), + sa.Column("operation_type", sa.SmallInteger(), nullable=False), + sa.PrimaryKeyConstraint("id", "transaction_id"), + sa.ForeignKeyConstraint( + ["transaction_id"], + ["version_transaction.id"], + name="fk_dashboards_version_transaction_id", + ), + sa.ForeignKeyConstraint( + ["end_transaction_id"], + ["version_transaction.id"], + name="fk_dashboards_version_end_transaction_id", + ), + ) + op.create_index( + "ix_dashboards_version_end_transaction_id", + "dashboards_version", + ["end_transaction_id"], + ) + op.create_index( + "ix_dashboards_version_operation_type", + "dashboards_version", + ["operation_type"], + ) + op.create_index( + "ix_dashboards_version_transaction_id", + "dashboards_version", + ["transaction_id"], + ) + + # ------------------------------------------------------------------ + # slices_version (Charts) + # ------------------------------------------------------------------ + op.create_table( + "slices_version", + sa.Column("uuid", UUIDType(binary=True), nullable=True), + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("slice_name", sa.String(250), nullable=True), + sa.Column("datasource_id", sa.Integer(), nullable=True), + sa.Column("datasource_type", sa.String(200), nullable=True), + sa.Column("datasource_name", sa.String(2000), nullable=True), + sa.Column("viz_type", sa.String(250), nullable=True), + sa.Column("params", MediumText(), nullable=True), + sa.Column("description", sa.Text(), nullable=True), + sa.Column("cache_timeout", sa.Integer(), nullable=True), + sa.Column("certified_by", sa.Text(), nullable=True), + sa.Column("certification_details", sa.Text(), nullable=True), + sa.Column("is_managed_externally", sa.Boolean(), nullable=True), + sa.Column("external_url", sa.Text(), nullable=True), + sa.Column("transaction_id", sa.BigInteger(), nullable=False), + sa.Column("end_transaction_id", sa.BigInteger(), nullable=True), + sa.Column("operation_type", sa.SmallInteger(), nullable=False), + sa.PrimaryKeyConstraint("id", "transaction_id"), + sa.ForeignKeyConstraint( + ["transaction_id"], + ["version_transaction.id"], + name="fk_slices_version_transaction_id", + ), + sa.ForeignKeyConstraint( + ["end_transaction_id"], + ["version_transaction.id"], + name="fk_slices_version_end_transaction_id", + ), + ) + op.create_index( + "ix_slices_version_end_transaction_id", + "slices_version", + ["end_transaction_id"], + ) + op.create_index( + "ix_slices_version_operation_type", + "slices_version", + ["operation_type"], + ) + op.create_index( + "ix_slices_version_transaction_id", + "slices_version", + ["transaction_id"], + ) + + # ------------------------------------------------------------------ + # tables_version (SqlaTable / Datasets) + # ------------------------------------------------------------------ + op.create_table( + "tables_version", + sa.Column("uuid", UUIDType(binary=True), nullable=True), + sa.Column("id", sa.Integer(), nullable=False), + sa.Column("description", sa.Text(), nullable=True), + sa.Column("default_endpoint", sa.Text(), nullable=True), + sa.Column("is_featured", sa.Boolean(), nullable=True), + sa.Column("filter_select_enabled", sa.Boolean(), nullable=True), + sa.Column("offset", sa.Integer(), nullable=True), + sa.Column("cache_timeout", sa.Integer(), nullable=True), + sa.Column("params", sa.String(1000), nullable=True), + sa.Column("is_managed_externally", sa.Boolean(), nullable=True), + sa.Column("external_url", sa.Text(), nullable=True), + sa.Column("table_name", sa.String(250), nullable=True), + sa.Column("main_dttm_col", sa.String(250), nullable=True), + sa.Column("currency_code_column", sa.String(250), nullable=True), + sa.Column("database_id", sa.Integer(), nullable=True), + sa.Column("fetch_values_predicate", sa.Text(), nullable=True), + sa.Column("schema", sa.String(255), nullable=True), + sa.Column("catalog", sa.String(256), nullable=True), + sa.Column("sql", MediumText(), nullable=True), + sa.Column("is_sqllab_view", sa.Boolean(), nullable=True), + sa.Column("template_params", sa.Text(), nullable=True), + sa.Column("extra", sa.Text(), nullable=True), + sa.Column("normalize_columns", sa.Boolean(), nullable=True), + sa.Column("always_filter_main_dttm", sa.Boolean(), nullable=True), + sa.Column("folders", sa.JSON(), nullable=True), + sa.Column("transaction_id", sa.BigInteger(), nullable=False), + sa.Column("end_transaction_id", sa.BigInteger(), nullable=True), + sa.Column("operation_type", sa.SmallInteger(), nullable=False), + sa.PrimaryKeyConstraint("id", "transaction_id"), + sa.ForeignKeyConstraint( + ["transaction_id"], + ["version_transaction.id"], + name="fk_tables_version_transaction_id", + ), + sa.ForeignKeyConstraint( + ["end_transaction_id"], + ["version_transaction.id"], + name="fk_tables_version_end_transaction_id", + ), + ) + op.create_index( + "ix_tables_version_end_transaction_id", + "tables_version", + ["end_transaction_id"], + ) + op.create_index( + "ix_tables_version_operation_type", + "tables_version", + ["operation_type"], + ) + op.create_index( + "ix_tables_version_transaction_id", + "tables_version", + ["transaction_id"], + ) + + # ------------------------------------------------------------------ + # version_changes + # + # Field-level diff log keyed to a (transaction, entity) pair. Each + # row describes one atomic change (one field or one child-collection + # element) that occurred to one entity during a save. + # + # ``(entity_kind, entity_id)`` is a polymorphic reference: depending + # on ``entity_kind`` (``"chart"`` / ``"dashboard"`` / ``"dataset"``) + # the ``entity_id`` is the integer PK on ``slices`` / ``dashboards`` / + # ``tables`` respectively. SQL has no native polymorphic FK, so the + # constraint is intentionally omitted β cleanup relies on the + # ``CASCADE`` from ``version_transaction.id`` plus command-layer + # ordering for entity deletes (the command that hard-deletes the + # entity runs inside the same transaction that prunes its history). + # A bare ``DELETE FROM <entity_table> WHERE id = X`` outside that + # transactional boundary leaves orphan ``version_changes`` rows + # whose ``entity_id`` references a vanished row β the read-side + # tombstone-state lookup handles this gracefully. + # ------------------------------------------------------------------ + op.create_table( + "version_changes", + sa.Column( + "id", + sa.BigInteger(), + primary_key=True, + autoincrement=True, + nullable=False, Review Comment: **Suggestion:** The `version_changes.id` column repeats the same cross-dialect issue: SQLite does not reliably auto-generate ids for `BigInteger` PKs, so change-record inserts can fail once capture writes to this table. Use the same dialect-safe PK type strategy here as for the transaction table. [type error] <details> <summary><b>Severity Level:</b> Critical π¨</summary> ```mdx - β SQLite change-log inserts into version_changes may fail. - β οΈ Version-history and activity views lose audit records. ``` </details> <details> <summary><b>Steps of Reproduction β </b></summary> ```mdx 1. Run the `upgrade()` function in `superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py` on a SQLite metadata database; at lines 320-379 it creates the `version_changes` table, with the primary key column defined at lines 322-327 as `sa.Column("id", sa.BigInteger(), primary_key=True, autoincrement=True, nullable=False)`. 2. Enable version capture by setting `ENABLE_VERSIONING_CAPTURE = True` so `SupersetAppInitializer.init_versioning()` in `superset/initialization/__init__.py:690-729` registers the change-record listener; this listener emits `ChangeRecord` instances defined in `superset/versioning/diff.py:13-37` and persists them via `session.bulk_insert_mappings` into `version_changes`. 3. Trigger a change that produces diff records, such as updating a dataset via the `PUT /api/v1/dataset/<id>` endpoint in `superset/datasets/api.py:500-538` or cloning a dashboard via `CopyDashboardCommand` in `superset/commands/dashboard/copy.py:35-55`; these commands run inside SQLAlchemy sessions, and with capture enabled they cause the change-record listener to insert multiple rows into `version_changes` for each edited entity. 4. Because the `version_changes.id` column on SQLite is a `BigInteger` primary key rather than an `INTEGER PRIMARY KEY`, and SQLiteβs autoincrement semantics are tied to rowid-style integer PKs, inserts issued by the change-record listener may not receive valid auto-generated ids or can hit NOT NULL / PK constraint errors; this breaks change-log persistence, causing version-history and activity-stream consumers (which rely on `version_changes` as described in `superset/versioning/diff.py:15-21`) to fail or return incomplete histories. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5bf884659d8641c8bc6fb514a3733275&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5bf884659d8641c8bc6fb514a3733275&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent π€ </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py **Line:** 322:327 **Comment:** *Type Error: The `version_changes.id` column repeats the same cross-dialect issue: SQLite does not reliably auto-generate ids for `BigInteger` PKs, so change-record inserts can fail once capture writes to this table. Use the same dialect-safe PK type strategy here as for the transaction table. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41176&comment_hash=70ad868092239c60956e0a22c18ae3e3ac31ef3d3c731064ea1c2499cd438f65&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41176&comment_hash=70ad868092239c60956e0a22c18ae3e3ac31ef3d3c731064ea1c2499cd438f65&reaction=dislike'>π</a> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
