From d6e6c3990a4db518e14908d5e8a395f10007793a Mon Sep 17 00:00:00 2001 From: Luke Mino-Altherr Date: Sat, 14 Mar 2026 22:29:45 -0400 Subject: [PATCH] Disallow all-null meta rows: add CHECK constraint, skip null values on write - convert_metadata_to_rows returns [] for None values instead of an all-null row - Remove dead None branch from _scalar_to_row - Simplify null filter in common.py to just check for row absence - Add CHECK constraint ck_asset_reference_meta_has_value to model and migration 0003 Amp-Thread-ID: https://ampcode.com/threads/T-019cef4e-5240-7749-bb25-1f17fcf9c09c Co-authored-by: Amp --- .../versions/0003_add_metadata_job_id.py | 20 +++++++++++++++++++ app/assets/database/models.py | 4 ++++ .../database/queries/asset_reference.py | 11 +--------- app/assets/database/queries/common.py | 10 +--------- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/alembic_db/versions/0003_add_metadata_job_id.py b/alembic_db/versions/0003_add_metadata_job_id.py index 3a0e0d0cf..1d75e836e 100644 --- a/alembic_db/versions/0003_add_metadata_job_id.py +++ b/alembic_db/versions/0003_add_metadata_job_id.py @@ -48,8 +48,28 @@ def upgrade() -> None: "ix_asset_references_preview_id", ["preview_id"] ) + # Purge any all-null meta rows before adding the constraint + op.execute( + "DELETE FROM asset_reference_meta" + " WHERE val_str IS NULL AND val_num IS NULL AND val_bool IS NULL AND val_json IS NULL" + ) + with op.batch_alter_table( + "asset_reference_meta", naming_convention=NAMING_CONVENTION + ) as batch_op: + batch_op.create_check_constraint( + "ck_asset_reference_meta_has_value", + "val_str IS NOT NULL OR val_num IS NOT NULL OR val_bool IS NOT NULL OR val_json IS NOT NULL", + ) + def downgrade() -> None: + with op.batch_alter_table( + "asset_reference_meta", naming_convention=NAMING_CONVENTION + ) as batch_op: + batch_op.drop_constraint( + "ck_asset_reference_meta_has_value", type_="check" + ) + with op.batch_alter_table( "asset_references", naming_convention=NAMING_CONVENTION ) as batch_op: diff --git a/app/assets/database/models.py b/app/assets/database/models.py index 733e807d0..5c7ff8154 100644 --- a/app/assets/database/models.py +++ b/app/assets/database/models.py @@ -191,6 +191,10 @@ class AssetReferenceMeta(Base): Index("ix_asset_reference_meta_key_val_str", "key", "val_str"), Index("ix_asset_reference_meta_key_val_num", "key", "val_num"), Index("ix_asset_reference_meta_key_val_bool", "key", "val_bool"), + CheckConstraint( + "val_str IS NOT NULL OR val_num IS NOT NULL OR val_bool IS NOT NULL OR val_json IS NOT NULL", + name="ck_asset_reference_meta_has_value", + ), ) diff --git a/app/assets/database/queries/asset_reference.py b/app/assets/database/queries/asset_reference.py index 04019b374..7af552483 100644 --- a/app/assets/database/queries/asset_reference.py +++ b/app/assets/database/queries/asset_reference.py @@ -46,15 +46,6 @@ def _check_is_scalar(v): def _scalar_to_row(key: str, ordinal: int, value) -> dict: """Convert a scalar value to a typed projection row.""" - if value is None: - return { - "key": key, - "ordinal": ordinal, - "val_str": None, - "val_num": None, - "val_bool": None, - "val_json": None, - } if isinstance(value, bool): return {"key": key, "ordinal": ordinal, "val_bool": bool(value)} if isinstance(value, (int, float, Decimal)): @@ -68,7 +59,7 @@ def _scalar_to_row(key: str, ordinal: int, value) -> dict: def convert_metadata_to_rows(key: str, value) -> list[dict]: """Turn a metadata key/value into typed projection rows.""" if value is None: - return [_scalar_to_row(key, 0, None)] + return [] if _check_is_scalar(value): return [_scalar_to_row(key, 0, value)] diff --git a/app/assets/database/queries/common.py b/app/assets/database/queries/common.py index 94ec5a526..89bb49327 100644 --- a/app/assets/database/queries/common.py +++ b/app/assets/database/queries/common.py @@ -101,20 +101,12 @@ def apply_metadata_filter( def _exists_clause_for_value(key: str, value) -> sa.sql.ClauseElement: if value is None: - no_row_for_key = sa.not_( + return sa.not_( sa.exists().where( AssetReferenceMeta.asset_reference_id == AssetReference.id, AssetReferenceMeta.key == key, ) ) - null_row = _exists_for_pred( - key, - AssetReferenceMeta.val_json.is_(None), - AssetReferenceMeta.val_str.is_(None), - AssetReferenceMeta.val_num.is_(None), - AssetReferenceMeta.val_bool.is_(None), - ) - return sa.or_(no_row_for_key, null_row) if isinstance(value, bool): return _exists_for_pred(key, AssetReferenceMeta.val_bool == bool(value))