From f4588a51000d03f624a7ddb637b5fb783571444e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 18:20:35 +0300 Subject: [PATCH 01/27] Remove legacy compatibility catalogs --- CHANGELOG.md | 10 + README.md | 8 +- src/openstatspec/sql/inplace_transform.py | 70 +- src/openstatspec/sql/wide.py | 1040 +++++++++------------ tests/conformance.py | 2 +- tests/test_atomic_import.py | 119 +-- tests/test_attribute_catalog.py | 73 +- tests/test_document_round_trip.py | 7 +- tests/test_inplace_transform.py | 4 +- tests/test_loss_reports.py | 52 +- tests/test_pyspssio_catalog_authority.py | 34 +- tests/test_sav_sqlite.py | 55 +- tests/test_spss_catalog_preflight.py | 31 +- tests/test_sql_services.py | 36 +- tests/test_vls_compatible_names.py | 45 +- 15 files changed, 609 insertions(+), 977 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index edb3058..9066451 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog + +## Unreleased + +### Fixed + +- Removed the temporary `*_catalog` compatibility schema and made SAV/ZSAV + import, validation, export, fidelity reporting, and in-place metadata edits + use the normative UUID-keyed OpenStatSpec catalog exclusively. +- Existing databases that contain only the former compatibility catalog must be + remediated manually before export. All notable changes to this reference implementation are documented here. ## 0.4.0 — 2026-07-31 diff --git a/README.md b/README.md index dead014..7d090c3 100644 --- a/README.md +++ b/README.md @@ -11,10 +11,10 @@ For each supported import, one source dataset becomes one dedicated wide SQL table. Cases are rows and source variables are physical SQL columns. The singular UUID-keyed tables from the specification (`dataset`, `variable`, `operation`, `fidelity_event`, and related metadata tables) are the public -catalog contract. Historical `*_catalog` tables are an internal compatibility -layer for the current exporter and are not the standard database interface. -The adapter does not reshape data, create EAV -or long-form tables, or harmonize studies or waves. +catalog contract and the only catalog representation created or read by the +adapter. Databases created with the former `*_catalog` compatibility schema +must be remediated to the normative schema before use. The adapter does not +reshape data, create EAV or long-form tables, or harmonize studies or waves. Unsupported source features, SQL targets, or export paths fail explicitly. There is no silent truncation, type conversion, metadata loss, or partial diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index e763a02..85dfeed 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -5,7 +5,6 @@ from collections.abc import Callable, Mapping from dataclasses import dataclass from datetime import UTC, datetime -import json from typing import Any from uuid import uuid4 @@ -22,8 +21,7 @@ ) from .capabilities import effective_profile from .normative import catalog as core_catalog -from .wide import catalog as legacy_catalog -from .wide import normalized_metadata_tables, physical_name +from .wide import physical_name from .workflow import TransformationError @@ -229,32 +227,23 @@ def _target_identity_state( return str(row.dataset_id), schema, table_name, relation_count -def _legacy_identifiers(dataset: dict[str, Any]) -> tuple[str, str]: - dataset_name = dataset.get("dataset_name") +def _physical_table_name(dataset: dict[str, Any]) -> str: table_name = dataset.get("physical_table_name") - if not isinstance(dataset_name, str) or not dataset_name: - raise TransformationError( - "dataset_invalid", "The target lacks its legacy catalog identity." - ) if not isinstance(table_name, str) or not table_name: raise TransformationError( "dataset_invalid", "The target lacks its physical wide-table name." ) - return dataset_name, table_name + return table_name def _replace_value_labels( connection: Any, *, core: Any, - legacy_variable: Table, - legacy_labels: Table, - legacy_dataset_id: str, variable: dict[str, Any], labels: tuple[ValueLabel, ...], ) -> None: variable_id = str(variable["variable_id"]) - ordinal = int(variable["source_ordinal"]) old_set = connection.execute( select(core.variable_value_label_set.c.value_label_set_id).where( core.variable_value_label_set.c.variable_id == variable_id @@ -286,31 +275,14 @@ def _replace_value_labels( value_label_set_id=label_set_id, ordinal=label_ordinal, code_kind="numeric" if item.value.type == "binary64" else "string", - numeric_code=(item.value.number() if item.value.type == "binary64" else None), - string_code=(str(item.value.value) if item.value.type == "string" else None), - label=item.label, - )) - connection.execute(delete(legacy_labels).where( - legacy_labels.c.dataset_id == legacy_dataset_id, - legacy_labels.c.variable_ordinal == ordinal, - )) - for label_ordinal, item in enumerate(labels, start=1): - connection.execute(insert(legacy_labels).values( - dataset_id=legacy_dataset_id, - variable_ordinal=ordinal, - ordinal=label_ordinal, - value_type="numeric" if item.value.type == "binary64" else "text", - numeric_value=(item.value.number() if item.value.type == "binary64" else None), - text_value=(str(item.value.value) if item.value.type == "string" else None), + numeric_code=( + item.value.number() if item.value.type == "binary64" else None + ), + string_code=( + str(item.value.value) if item.value.type == "string" else None + ), label=item.label, )) - legacy_json = { - str(_typed_value(item.value)): item.label for item in labels - } - connection.execute(update(legacy_variable).where( - legacy_variable.c.dataset_id == legacy_dataset_id, - legacy_variable.c.ordinal == ordinal, - ).values(value_labels=json.dumps(legacy_json, ensure_ascii=False))) def _apply_plan_on_connection( @@ -335,7 +307,7 @@ def _apply_plan_on_connection( "The target dataset's physical wide table does not exist.", ) dataset, variables, schema = _input_schema(connection, dataset_id) - legacy_dataset_id, table_name = _legacy_identifiers(dataset) + table_name = _physical_table_name(dataset) plan = submission.plan bound = bind_transformation_plan(plan, schema) output_by_name = { @@ -386,9 +358,6 @@ def _apply_plan_on_connection( "storage-width operation.", ) core = core_catalog(MetaData()) - legacy_metadata = MetaData() - _, legacy_variable, _, _ = legacy_catalog(legacy_metadata) - _, legacy_labels, _, _ = normalized_metadata_tables(legacy_metadata) relation = Table( table_name, MetaData(), @@ -427,18 +396,6 @@ def _apply_plan_on_connection( "variable_label": None, } connection.execute(insert(core.variable).values(**target_variable)) - connection.execute(insert(legacy_variable).values( - dataset_id=legacy_dataset_id, - ordinal=new_ordinal, - source_name=operation.target, - physical_name=target_physical, - storage_kind="numeric", - string_width=None, - label="", - attributes="{}", - value_labels="{}", - missing_ranges="[]", - )) variables.append(target_variable) by_name[operation.target.casefold()] = target_variable relation = Table( @@ -466,17 +423,10 @@ def _apply_plan_on_connection( connection.execute(update(core.variable).where( core.variable.c.variable_id == variable["variable_id"] ).values(variable_label=operation.label)) - connection.execute(update(legacy_variable).where( - legacy_variable.c.dataset_id == legacy_dataset_id, - legacy_variable.c.ordinal == variable["source_ordinal"], - ).values(label=operation.label)) elif isinstance(operation, ReplaceValueLabelsOperation): _replace_value_labels( connection, core=core, - legacy_variable=legacy_variable, - legacy_labels=legacy_labels, - legacy_dataset_id=legacy_dataset_id, variable=by_name[operation.variable.casefold()], labels=operation.labels, ) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index a9e3bad..e38ed84 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -9,16 +9,20 @@ from collections.abc import Iterable, Mapping from typing import Any -from sqlalchemy import delete, BigInteger, Boolean, Column, Float, Integer, MetaData, String, Table, Text, create_engine, insert, inspect, select, text, update +from sqlalchemy import ( + BigInteger, Column, Float, MetaData, Table, Text, create_engine, insert, + inspect, select, +) from sqlalchemy.dialects import mysql, postgresql, sqlite from ..core import UnsupportedOperationError from .capabilities import effective_profile from .profiles import preflight, validate_connection_url from .normative import ( + CATALOG_CONTRACT_ID, + CATALOG_SCHEMA_VERSION, catalog as normative_catalog, create as create_normative_catalog, delete_dataset_representation as delete_normative_dataset, - dataset_id_for_name as normative_dataset_id_for_name, finish_operation as finish_normative_operation, record_fidelity_events as record_normative_fidelity_events, record_operation as record_normative_operation, @@ -62,283 +66,38 @@ def binary64_type() -> Float: ) -def catalog(metadata: MetaData) -> tuple[Table, Table, Table, Table]: - datasets = Table( - "dataset_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("data_table", String(255), nullable=False, unique=True), - Column("source_format", String(16), nullable=False), - Column("source_name", Text, nullable=False), - Column("source_table_name", Text), - Column("source_sha256", String(64), nullable=False), - Column("source_created_at", String(40)), - Column("source_modified_at", String(40)), - Column("imported_at", String(40), nullable=False), - Column("source_encoding", String(128)), - Column("case_count", BigInteger, nullable=False), - Column("file_label", Text, nullable=False, default=""), - Column("documents", Text, nullable=False, default="[]"), - Column("file_attributes", Text, nullable=False, default="{}"), - Column("case_weight_variable", String(255)), - Column("multiple_response_sets", Text, nullable=False, default="{}"), - ) - variables = Table( - "variable_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("ordinal", Integer, primary_key=True), - Column("source_name", String(255), nullable=False), - Column("physical_name", String(255), nullable=False), - Column("storage_kind", String(16), nullable=False), - Column("readstat_storage_type", String(32)), - Column("string_width", Integer), - Column("label", Text, nullable=False, default=""), - Column("format", String(64)), - # format remains the legacy print-format mirror; SPSS has a distinct write format. - Column("print_format", String(64)), - Column("write_format", String(64)), - Column("measure", String(32)), - Column("role", String(32)), - Column("alignment", String(32)), - Column("display_width", Integer), - Column("attributes", Text, nullable=False, default="{}"), - Column("compat_name", String(255)), - Column("value_labels", Text, nullable=False, default="{}"), - Column("missing_ranges", Text, nullable=False, default="[]"), - ) - fidelity_events = Table( - "fidelity_event_catalog", metadata, - Column("operation_id", String(36), primary_key=True), - Column("ordinal", Integer, primary_key=True), - Column("dataset_id", String(255)), - Column("direction", String(16), nullable=False), - Column("severity", String(16), nullable=False), - Column("detail", Text, nullable=False), - Column("details", Text, nullable=False, default="{}"), - Column("code", String(128), nullable=False), - ) - operations = Table( - "operation_catalog", metadata, - Column("operation_id", String(36), primary_key=True), - Column("direction", String(16), nullable=False), - Column("status", String(16), nullable=False), - Column("dataset_id", String(255)), - Column("source", Text), - Column("destination", Text), - Column("created_at", String(40), nullable=False), - Column("completed_at", String(40)), - Column("details", Text, nullable=False, default="{}"), - ) - return datasets, variables, fidelity_events, operations - - -def _now() -> str: - return datetime.now(UTC).isoformat() - -def _migrate_catalog_columns( - connection: Any, datasets: Table, variables: Table, multiple_response: Table, -) -> None: - """Add pyspssio metadata columns to catalogs created by earlier adapters. - - This additive migration is intentionally small and portable: no existing - source data or dictionary row is rewritten, while a later import can store - the newly observable metadata alongside it. - """ - inspector = inspect(connection) - additions = { - datasets.name: { - "file_attributes": "TEXT NOT NULL DEFAULT '{}'", - "case_weight_variable": "VARCHAR(255)", - }, - variables.name: { - "role": "VARCHAR(32)", - "attributes": "TEXT NOT NULL DEFAULT '{}'", - "compat_name": "VARCHAR(255)", - "print_format": "VARCHAR(64)", - "write_format": "VARCHAR(64)", - }, - multiple_response.name: { - "is_dichotomy": "BOOLEAN", - "use_category_labels": "BOOLEAN", - "use_first_var_label": "BOOLEAN", - "counted_value_type": "VARCHAR(16)", - "counted_numeric": "DOUBLE", - "counted_text": "TEXT", - }, - } - preparer = connection.dialect.identifier_preparer - for table_name, columns in additions.items(): - if not inspector.has_table(table_name): - continue - existing = {column["name"] for column in inspector.get_columns(table_name)} - for name, declaration in columns.items(): - if name not in existing: - connection.execute(text( - f"ALTER TABLE {preparer.quote(table_name)} ADD COLUMN " - f"{preparer.quote(name)} {declaration}" - )) - - - -def _event_rows( - *, operation_id: str, dataset_id: str | None, direction: str, - fidelity_events: Iterable[Mapping[str, Any]], -) -> list[dict[str, Any]]: - """Normalize the public compact diagnostic shape into durable catalog rows.""" - rows: list[dict[str, Any]] = [] - for ordinal, event in enumerate(fidelity_events, start=1): - detail = str(event["detail"]) - details = event.get("details", {}) - rows.append({ - "operation_id": operation_id, "ordinal": ordinal, "dataset_id": dataset_id, - "direction": str(event.get("direction", direction)), - "severity": str(event.get("severity", "warning")), - "code": str(event["code"]), "detail": detail, - "details": json.dumps(details, default=str, sort_keys=True), - }) - - return rows def _record_failed_preflight( - *, engine: Any, metadata: MetaData, datasets: Table, variable_catalog: Table, - multiple_response_catalog: Table, fidelity_event_catalog: Table, - operation_catalog: Table, normative: Any, operation_id: str, source_name: str, - source_format: str, variable_count: int, profile_name: str, error: Exception, + *, engine: Any, normative: Any, operation_id: str, source_name: str, + source_format: str, variable_count: int, profile_name: str, + error: Exception, ) -> None: - """Persist a failed preflight without creating any source dataset state.""" + """Persist a failed preflight in the normative audit catalog.""" with engine.begin() as connection: create_normative_catalog(connection, normative) - metadata.create_all(connection, tables=[ - datasets, variable_catalog, multiple_response_catalog, - fidelity_event_catalog, operation_catalog, - ]) failed_at = datetime.now(UTC).replace(tzinfo=None) record_normative_operation( connection, normative, operation_id=operation_id, operation_kind="import", status="failed", source_format=source_format, started_at=failed_at, completed_at=failed_at, ) - connection.execute(insert(operation_catalog).values( - operation_id=operation_id, direction="import", status="failed", dataset_id=None, - source=source_name, created_at=_now(), completed_at=_now(), - details=json.dumps({"reason": "preflight", "variable_count": variable_count, - "capability": getattr(error, "details", {})}, sort_keys=True), - )) - failed_events = ({ - "code": getattr(error, "code", "target_capability_exceeded"), - "detail": str(error), "severity": "error", "source_item": source_name, - "details": {"variable_count": variable_count, "profile": profile_name, - **getattr(error, "details", {})}, - },) - connection.execute(insert(fidelity_event_catalog), _event_rows( - operation_id=operation_id, dataset_id=None, direction="import", - fidelity_events=failed_events, - )) record_normative_fidelity_events( connection, normative, operation_id=operation_id, dataset_id=None, - direction="import", events=failed_events, + direction="import", events=({ + "code": str(getattr( + error, "code", "target_capability_exceeded")).replace("-", "_"), + "detail": str(error), "severity": "error", + "source_item": source_name, + "details": { + "variable_count": variable_count, + "profile": profile_name, + **getattr(error, "details", {}), + }, + },), ) -def multiple_response_set_catalog(metadata: MetaData) -> Table: - return Table( - "multiple_response_set_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("set_name", String(255), primary_key=True), - Column("member_ordinal", Integer, primary_key=True), - Column("kind", String(16)), Column("label", Text), - Column("is_dichotomy", Boolean), - Column("use_category_labels", Boolean), - Column("use_first_var_label", Boolean), - Column("counted_value", Text), - Column("counted_value_type", String(16)), - Column("counted_numeric", binary64_type()), - Column("counted_text", Text), - Column("variable_name", String(255)), - Column("definition", Text, nullable=False), - ) - - -def source_extension_catalog(metadata: MetaData) -> Table: - """Namespaced raw source semantics retained even when export is fail-closed.""" - return Table( - "source_extension_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("extension_key", String(255), primary_key=True), - Column("payload", Text, nullable=False), - ) - - -def source_extension_rows(dataset_id: str, extensions: Mapping[str, Any]) -> list[dict[str, Any]]: - return [ - {"dataset_id": dataset_id, "extension_key": str(key), - "payload": json.dumps(payload, default=str, ensure_ascii=False, sort_keys=True)} - for key, payload in sorted(extensions.items()) - ] - - -def document_catalog(metadata: MetaData) -> Table: - """Ordered file documents, normalized independently from the legacy JSON column.""" - return Table( - "document_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("ordinal", Integer, primary_key=True), - Column("text", Text, nullable=False), - ) - - -def value_label_catalog(metadata: MetaData) -> Table: - """Typed, ordered value labels; JSON on variable_catalog remains a read fallback.""" - return Table( - "value_label_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("variable_ordinal", Integer, primary_key=True), - Column("ordinal", Integer, primary_key=True), - Column("value_type", String(16), nullable=False), - Column("numeric_value", binary64_type()), - Column("text_value", Text), - Column("label", Text, nullable=False), - ) - - -def missing_rule_catalog(metadata: MetaData) -> Table: - """Typed inclusive SPSS user-missing intervals, including discrete values as lo == hi.""" - return Table( - "missing_rule_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("variable_ordinal", Integer, primary_key=True), - Column("ordinal", Integer, primary_key=True), - Column("kind", String(16), nullable=False), - Column("lower_type", String(16), nullable=False), - Column("lower_numeric", binary64_type()), - Column("lower_text", Text), - Column("upper_type", String(16), nullable=False), - Column("upper_numeric", binary64_type()), - Column("upper_text", Text), - Column("lower_inclusive", Boolean, nullable=False, default=True), - Column("upper_inclusive", Boolean, nullable=False, default=True), - ) - - -def attribute_catalog(metadata: MetaData) -> Table: - """Ordered SPSS custom-attribute values for files and variables. - - SPSS custom attributes are text-valued, but one attribute name can carry - an ordered array of values. ``scope`` is ``file`` for a file attribute - (with ``variable_ordinal == 0``) and ``variable`` for an attribute of one - source variable. This table is authoritative whenever it contains rows; - the JSON columns on older catalogs remain a migration fallback. - """ - return Table( - "attribute_catalog", metadata, - Column("dataset_id", String(255), primary_key=True), - Column("scope", String(16), primary_key=True), - Column("variable_ordinal", Integer, primary_key=True), - Column("attribute_ordinal", Integer, primary_key=True), - Column("value_ordinal", Integer, primary_key=True), - Column("attribute_name", String(255), nullable=False), - Column("attribute_value", Text, nullable=False), - ) def _attribute_values(value: Any) -> list[str]: @@ -377,26 +136,6 @@ def append(scope: str, variable_ordinal: int, attributes: Mapping[str, Any] | No return rows -def attributes_from_rows( - rows: Iterable[Mapping[str, Any]], *, variables: Iterable[Mapping[str, Any]], -) -> tuple[dict[str, Any], dict[str, dict[str, Any]]]: - """Rebuild ordered attributes; rows, not legacy JSON, are authoritative.""" - by_ordinal = {int(item["ordinal"]): str(item["source_name"]) for item in variables} - grouped: dict[tuple[str, int], dict[str, list[str]]] = {} - for row in rows: - target = grouped.setdefault((str(row["scope"]), int(row["variable_ordinal"])), {}) - target.setdefault(str(row["attribute_name"]), []).append(str(row["attribute_value"])) - - def collapse(values: Mapping[str, list[str]]) -> dict[str, Any]: - return {name: value[0] if len(value) == 1 else value for name, value in values.items()} - - file_attributes = collapse(grouped.get(("file", 0), {})) - variable_attributes = { - by_ordinal[ordinal]: collapse(attributes) - for (scope, ordinal), attributes in grouped.items() - if scope == "variable" and ordinal in by_ordinal - } - return file_attributes, variable_attributes def _typed_endpoint(value: Any) -> tuple[str, float | None, str | None]: @@ -464,11 +203,6 @@ def missing_rule_rows(dataset_id: str, variables: list[dict[str, Any]]) -> list[ return rows -def normalized_metadata_tables(metadata: MetaData) -> tuple[Table, Table, Table, Table]: - return ( - document_catalog(metadata), value_label_catalog(metadata), missing_rule_catalog(metadata), - attribute_catalog(metadata), - ) def _mr_counted_value(definition: Mapping[str, Any]) -> tuple[str | None, float | None, str | None]: value = definition.get("counted_value", definition.get("countedvalue")) @@ -690,44 +424,6 @@ def multiple_response_set_rows(dataset_id: str, definitions: str) -> list[dict[s return rows -def multiple_response_sets_from_rows(rows: Iterable[Mapping[str, Any]]) -> dict[str, dict[str, Any]]: - """Reconstruct pyspssio MR metadata from normalized catalog rows. - - The new typed fields are authoritative. ``definition`` only supplies - backwards compatibility for catalogs written before those fields existed. - """ - result: dict[str, dict[str, Any]] = {} - for row in rows: - name = str(row["set_name"]) - if name not in result: - try: - legacy = json.loads(row.get("definition") or "{}") - except (TypeError, json.JSONDecodeError): - legacy = {} - definition = dict(legacy) if isinstance(legacy, Mapping) else {} - definition["variable_list"] = [] - result[name] = definition - definition = result[name] - if row.get("label") is not None: - definition["label"] = row["label"] - if row.get("is_dichotomy") is not None: - definition["is_dichotomy"] = bool(row["is_dichotomy"]) - elif row.get("kind") is not None: - definition["is_dichotomy"] = str(row["kind"]).upper() == "MD" - if row.get("use_category_labels") is not None: - definition["use_category_labels"] = bool(row["use_category_labels"]) - if row.get("use_first_var_label") is not None: - definition["use_first_var_label"] = bool(row["use_first_var_label"]) - value_type = row.get("counted_value_type") - if value_type == "numeric": - definition["counted_value"] = row.get("counted_numeric") - elif value_type == "text": - definition["counted_value"] = row.get("counted_text") - elif row.get("counted_value") not in (None, ""): - definition["counted_value"] = row["counted_value"] - if row.get("variable_name") is not None: - definition["variable_list"].append(row["variable_name"]) - return result def physical_name(source_name: str, used: set[str]) -> str: stem = _IDENTIFIER.sub("_", source_name).strip("_").lower() or "variable" @@ -747,30 +443,26 @@ def data_table_name(dataset_id: str) -> str: def create_wide_dataset( *, database_url: str, dataset_id: str, source_name: str, source_format: str, - rows: Iterable[Mapping[str, Any]], variables: list[dict[str, Any]], file_label: str = "", - source_encoding: str | None = None, documents: str = "[]", - file_attributes: str = "{}", case_weight_variable: str | None = None, + rows: Iterable[Mapping[str, Any]], variables: list[dict[str, Any]], + file_label: str = "", documents: str = "[]", file_attributes: str = "{}", file_attribute_values: Mapping[str, Any] | None = None, variable_attribute_values: Mapping[str, Mapping[str, Any]] | None = None, - source_table_name: str | None = None, - source_sha256: str = "", + case_weight_variable: str | None = None, multiple_response_sets: str = "{}", + source_encoding: str | None = None, + source_table_name: str | None = None, source_sha256: str = "", source_created_at: str | None = None, source_modified_at: str | None = None, - imported_at: str = "", - multiple_response_sets: str = "{}", + imported_at: str | None = None, source_extensions: Mapping[str, Any] | None = None, fidelity_events: Iterable[Mapping[str, Any]] = (), operation_details: Mapping[str, Any] | None = None, ) -> dict[str, Any]: + del source_table_name, source_created_at, source_modified_at, operation_details validate_connection_url(database_url) profile, _active_connection = effective_profile(database_url) engine = create_engine(database_url) - metadata = MetaData() - datasets, variable_catalog, fidelity_event_catalog, operation_catalog = catalog(metadata) - normative = normative_catalog(metadata) - multiple_response_catalog = multiple_response_set_catalog(metadata) - source_extensions_catalog = source_extension_catalog(metadata) - documents_catalog, value_labels_catalog, missing_rules_catalog, attributes_catalog = normalized_metadata_tables(metadata) + normative = normative_catalog(MetaData()) operation_id = str(uuid4()) + normative_dataset_id = str(uuid4()) fidelity_events = tuple(fidelity_events) source_rows = list(rows) try: @@ -782,51 +474,59 @@ def create_wide_dataset( ) except Exception as error: _record_failed_preflight( - engine=engine, metadata=metadata, datasets=datasets, - variable_catalog=variable_catalog, multiple_response_catalog=multiple_response_catalog, - fidelity_event_catalog=fidelity_event_catalog, operation_catalog=operation_catalog, - operation_id=operation_id, source_name=source_name, - source_format=source_format, variable_count=len(variables), - profile_name=profile.name, error=error, normative=normative, + engine=engine, normative=normative, operation_id=operation_id, + source_name=source_name, source_format=source_format, + variable_count=len(variables), profile_name=profile.name, error=error, ) raise + data_table = Table( - data_table_name(dataset_id), metadata, + data_table_name(dataset_id), MetaData(), Column("__case_ordinal", BigInteger, primary_key=True, nullable=False), - *(Column(item["physical_name"], binary64_type() if item["storage_kind"] == "numeric" else string_type(profile), - nullable=item["storage_kind"] == "numeric") for item in variables), + *( + Column( + item["physical_name"], + binary64_type() if item["storage_kind"] == "numeric" + else string_type(profile), + nullable=item["storage_kind"] == "numeric", + ) + for item in variables + ), ) materialized = [ {"__case_ordinal": ordinal, **row} for ordinal, row in enumerate(source_rows, start=1) ] - normative_dataset_id = str(uuid4()) + docs_rows = document_rows(normative_dataset_id, documents) + labels_rows = value_label_rows(normative_dataset_id, variables) + missing_rows = missing_rule_rows(normative_dataset_id, variables) + attributes_rows = attribute_rows( + normative_dataset_id, variables, + file_attributes=file_attribute_values, + variable_attributes=variable_attribute_values, + ) + mrset_rows = multiple_response_set_rows( + normative_dataset_id, multiple_response_sets, + ) namespace_owned = False data_table_was_absent = False try: + with engine.begin() as setup: + create_normative_catalog(setup, normative) + namespace_owned = True with engine.begin() as connection: - create_normative_catalog(connection, normative) - namespace_owned = True - metadata.create_all(connection, tables=[ - datasets, variable_catalog, multiple_response_catalog, - source_extensions_catalog, documents_catalog, value_labels_catalog, - missing_rules_catalog, attributes_catalog, fidelity_event_catalog, - operation_catalog, - ]) - _migrate_catalog_columns( - connection, datasets, variable_catalog, multiple_response_catalog, - ) if connection.execute( - select(datasets.c.dataset_id).where( - datasets.c.dataset_id == dataset_id + select(normative.dataset.c.dataset_id).where( + normative.dataset.c.dataset_name == dataset_id ) ).first(): raise ValueError( f"Dataset {dataset_id!r} already exists; imports never overwrite a dataset." ) if connection.execute( - select(datasets.c.dataset_id).where( - datasets.c.data_table == data_table.name + select(normative.dataset.c.dataset_id).where( + normative.dataset.c.physical_table_name == data_table.name, + normative.dataset.c.physical_table_schema.is_(None), ) ).first(): raise ValueError( @@ -837,69 +537,13 @@ def create_wide_dataset( raise ValueError( f"Physical data-table name {data_table.name!r} is already occupied." ) - data_table_was_absent = True record_normative_operation( connection, normative, operation_id=operation_id, operation_kind="import", status="started", source_format=source_format, ) - connection.execute(insert(operation_catalog).values( - operation_id=operation_id, direction="import", status="running", - dataset_id=dataset_id, source=source_name, created_at=_now(), - details=json.dumps({ - "variable_count": len(variables), - **dict(operation_details or {}), - }, sort_keys=True), - )) + data_table_was_absent = True data_table.create(connection) - connection.execute(insert(datasets).values( - dataset_id=dataset_id, data_table=data_table.name, - source_format=source_format, source_name=source_name, - source_encoding=source_encoding, case_count=len(materialized), - source_table_name=source_table_name, source_sha256=source_sha256, - source_created_at=source_created_at, - source_modified_at=source_modified_at, imported_at=imported_at, - file_label=file_label, documents=documents, - file_attributes=file_attributes, - case_weight_variable=case_weight_variable, - multiple_response_sets=multiple_response_sets, - )) - connection.execute( - insert(variable_catalog), - [dict(dataset_id=dataset_id, **item) for item in variables], - ) - docs_rows = document_rows(dataset_id, documents) - if docs_rows: - connection.execute(insert(documents_catalog), docs_rows) - labels_rows = value_label_rows(dataset_id, variables) - if labels_rows: - connection.execute(insert(value_labels_catalog), labels_rows) - missing_rows = missing_rule_rows(dataset_id, variables) - if missing_rows: - connection.execute(insert(missing_rules_catalog), missing_rows) - attributes_rows = attribute_rows( - dataset_id, variables, - file_attributes=file_attribute_values, - variable_attributes=variable_attribute_values, - ) - if attributes_rows: - connection.execute(insert(attributes_catalog), attributes_rows) - mrset_rows = multiple_response_set_rows( - dataset_id, multiple_response_sets, - ) - if mrset_rows: - connection.execute(insert(multiple_response_catalog), mrset_rows) - extension_rows = source_extension_rows( - dataset_id, source_extensions or {}, - ) - if extension_rows: - connection.execute(insert(source_extensions_catalog), extension_rows) - event_rows = _event_rows( - operation_id=operation_id, dataset_id=dataset_id, - direction="import", fidelity_events=fidelity_events, - ) - if event_rows: - connection.execute(insert(fidelity_event_catalog), event_rows) store_normative_dataset( connection, normative, dataset_name=dataset_id, source_format=source_format, physical_table_name=data_table.name, @@ -920,65 +564,34 @@ def create_wide_dataset( ) if materialized: connection.execute(insert(data_table), materialized) - connection.execute(update(operation_catalog).where( - operation_catalog.c.operation_id == operation_id - ).values(status="succeeded", completed_at=_now())) finish_normative_operation( connection, normative, operation_id=operation_id, status="succeeded", ) except Exception as error: - if ( - profile.name in {"mysql", "mariadb", "dolt"} - and namespace_owned - and data_table_was_absent - ): + if namespace_owned: try: with engine.begin() as cleanup: - delete_normative_dataset( - cleanup, normative, normative_dataset_id, - ) - cleanup_inspector = inspect(cleanup) - for table in ( - multiple_response_catalog, source_extensions_catalog, - documents_catalog, value_labels_catalog, - missing_rules_catalog, attributes_catalog, - fidelity_event_catalog, variable_catalog, + create_normative_catalog(cleanup, normative) + if cleanup.execute( + select(normative.dataset.c.dataset_id).where( + normative.dataset.c.dataset_id == normative_dataset_id + ) + ).first(): + delete_normative_dataset( + cleanup, normative, normative_dataset_id, + ) + if ( + data_table_was_absent + and inspect(cleanup).has_table(data_table.name) ): - if cleanup_inspector.has_table(table.name): - cleanup.execute( - delete(table).where(table.c.dataset_id == dataset_id) - ) - if cleanup_inspector.has_table(datasets.name): - cleanup.execute( - delete(datasets).where(datasets.c.dataset_id == dataset_id) + data_table.drop(cleanup, checkfirst=True) + operation_exists = cleanup.execute( + select(normative.operation.c.operation_id).where( + normative.operation.c.operation_id == operation_id ) - data_table.drop(cleanup, checkfirst=True) - metadata.create_all(cleanup, tables=[ - datasets, variable_catalog, multiple_response_catalog, - source_extensions_catalog, documents_catalog, - value_labels_catalog, missing_rules_catalog, - attributes_catalog, fidelity_event_catalog, - operation_catalog, - ]) - failed_event = ({ - "code": "import_failed", - "detail": str(error), - "severity": "error", - "source_item": source_name, - "details": { - "profile": profile.name, - "phase": "post_ddl", - "cleanup": "complete", - "error_type": type(error).__name__, - }, - },) - normative_operation_exists = cleanup.execute(select( - normative.operation.c.operation_id - ).where( - normative.operation.c.operation_id == operation_id - )).first() - if normative_operation_exists: + ).first() + if operation_exists: finish_normative_operation( cleanup, normative, operation_id=operation_id, status="failed", @@ -988,41 +601,17 @@ def create_wide_dataset( record_normative_operation( cleanup, normative, operation_id=operation_id, operation_kind="import", status="failed", - source_format=source_format, started_at=failed_at, - completed_at=failed_at, + source_format=source_format, + started_at=failed_at, completed_at=failed_at, ) - mirror_operation_exists = cleanup.execute(select( - operation_catalog.c.operation_id - ).where( - operation_catalog.c.operation_id == operation_id - )).first() - if mirror_operation_exists: - cleanup.execute(update(operation_catalog).where( - operation_catalog.c.operation_id == operation_id - ).values( - status="failed", dataset_id=None, completed_at=_now(), - )) - else: - cleanup.execute(insert(operation_catalog).values( - operation_id=operation_id, direction="import", - status="failed", dataset_id=None, source=source_name, - created_at=_now(), completed_at=_now(), - details=json.dumps({ - "reason": "post_ddl", - "variable_count": len(variables), - }, sort_keys=True), - )) - cleanup.execute( - insert(fidelity_event_catalog), - _event_rows( - operation_id=operation_id, dataset_id=None, - direction="import", fidelity_events=failed_event, - ), - ) record_normative_fidelity_events( cleanup, normative, operation_id=operation_id, - dataset_id=None, direction="import", - events=failed_event, + dataset_id=None, direction="import", events=({ + "code": "import_failed", + "detail": str(error), "severity": "error", + "source_item": source_name, + "details": {"reason": "post_preflight"}, + },), ) except Exception as cleanup_error: raise RuntimeError( @@ -1030,126 +619,351 @@ def create_wide_dataset( ) from cleanup_error raise return { - "dataset_id": dataset_id, "data_table": data_table.name, - "case_count": len(materialized), "operation_id": operation_id, + "dataset_id": normative_dataset_id, + "dataset_name": dataset_id, + "data_table": data_table.name, + "case_count": len(materialized), + "operation_id": operation_id, } -def _endpoint_from_row(row: Mapping[str, Any], *, prefix: str) -> Any: - endpoint_type = row[f"{prefix}_type"] - if endpoint_type == "lowest": - return -sys.float_info.max - if endpoint_type == "highest": - return sys.float_info.max - return row[f"{prefix}_numeric"] if endpoint_type == "numeric" else row[f"{prefix}_text"] def read_wide_dataset( *, database_url: str, dataset_id: str, profile: Any | None = None, ) -> tuple[dict[str, Any], list[dict[str, Any]], list[dict[str, Any]]]: - """Read a strict dataset only after resolving the active server profile.""" + """Read an export descriptor from the normative catalog without mutation.""" if profile is None: profile, _active = effective_profile(database_url) engine = create_engine(database_url) - metadata = MetaData() - datasets, variable_catalog, _, _ = catalog(metadata) - multiple_response_catalog = multiple_response_set_catalog(metadata) - source_extensions_catalog = source_extension_catalog(metadata) - documents_catalog, value_labels_catalog, missing_rules_catalog, attributes_catalog = normalized_metadata_tables(metadata) - with engine.begin() as connection: - metadata.create_all(connection, tables=[datasets, variable_catalog, multiple_response_catalog, source_extensions_catalog, documents_catalog, value_labels_catalog, missing_rules_catalog, attributes_catalog]) - _migrate_catalog_columns(connection, datasets, variable_catalog, multiple_response_catalog) - dataset = dict(connection.execute(select(datasets).where(datasets.c.dataset_id == dataset_id)).mappings().one()) - data_table = Table(dataset["data_table"], MetaData(), autoload_with=connection) - variables = [dict(item) for item in connection.execute( - select(variable_catalog).where(variable_catalog.c.dataset_id == dataset_id).order_by(variable_catalog.c.ordinal) - ).mappings().all()] - rows = [dict(item) for item in connection.execute( + normative = normative_catalog(MetaData()) + with engine.connect() as connection: + _verify_normative_catalog(connection, normative) + dataset_row = _resolve_normative_dataset(connection, normative, dataset_id) + core_id = str(dataset_row["dataset_id"]) + data_table = Table( + str(dataset_row["physical_table_name"]), MetaData(), + schema=dataset_row["physical_table_schema"], + autoload_with=connection, + ) + source_variables = connection.execute( + select(normative.variable) + .where(normative.variable.c.dataset_id == core_id) + .order_by(normative.variable.c.source_ordinal) + ).mappings().all() + variables = [_export_variable(row) for row in source_variables] + variables_by_id = { + str(row["variable_id"]): variable + for row, variable in zip(source_variables, variables, strict=True) + } + variable_ids = tuple(variables_by_id) + rows = [dict(row) for row in connection.execute( select(data_table).order_by(data_table.c.__case_ordinal) - ).mappings().all()] - document_rows_result = connection.execute( - select(documents_catalog).where(documents_catalog.c.dataset_id == dataset_id) - .order_by(documents_catalog.c.ordinal) + ).mappings()] + documents = connection.execute( + select(normative.document) + .where(normative.document.c.dataset_id == core_id) + .order_by(normative.document.c.source_ordinal) + ).mappings().all() + dataset_attributes = connection.execute( + select(normative.dataset_attribute) + .where(normative.dataset_attribute.c.dataset_id == core_id) + .order_by( + normative.dataset_attribute.c.attribute_name, + normative.dataset_attribute.c.array_ordinal, + ) ).mappings().all() - label_rows_result = connection.execute( - select(value_labels_catalog).where(value_labels_catalog.c.dataset_id == dataset_id) - .order_by(value_labels_catalog.c.variable_ordinal, value_labels_catalog.c.ordinal) + variable_attributes = connection.execute( + select(normative.variable_attribute) + .where(normative.variable_attribute.c.variable_id.in_(variable_ids)) + .order_by( + normative.variable_attribute.c.variable_id, + normative.variable_attribute.c.attribute_name, + normative.variable_attribute.c.array_ordinal, + ) ).mappings().all() - missing_rows_result = connection.execute( - select(missing_rules_catalog).where(missing_rules_catalog.c.dataset_id == dataset_id) - .order_by(missing_rules_catalog.c.variable_ordinal, missing_rules_catalog.c.ordinal) + labels = connection.execute( + select( + normative.variable_value_label_set.c.variable_id, + normative.value_label, + ) + .join( + normative.value_label, + normative.value_label.c.value_label_set_id + == normative.variable_value_label_set.c.value_label_set_id, + ) + .where( + normative.variable_value_label_set.c.variable_id.in_(variable_ids) + ) + .order_by( + normative.variable_value_label_set.c.variable_id, + normative.value_label.c.ordinal, + ) ).mappings().all() - attribute_rows_result = connection.execute( - select(attributes_catalog).where(attributes_catalog.c.dataset_id == dataset_id) + missing_rules = connection.execute( + select(normative.missing_rule) + .where(normative.missing_rule.c.variable_id.in_(variable_ids)) .order_by( - attributes_catalog.c.scope, attributes_catalog.c.variable_ordinal, - attributes_catalog.c.attribute_ordinal, attributes_catalog.c.value_ordinal, + normative.missing_rule.c.variable_id, + normative.missing_rule.c.ordinal, ) ).mappings().all() - mrset_rows_result = connection.execute( - select(multiple_response_catalog).where(multiple_response_catalog.c.dataset_id == dataset_id) - .order_by(multiple_response_catalog.c.set_name, multiple_response_catalog.c.member_ordinal) + variable_sets = connection.execute( + select(normative.variable_set) + .where(normative.variable_set.c.dataset_id == core_id) + .order_by(normative.variable_set.c.source_ordinal) ).mappings().all() - extension_rows_result = connection.execute( - select(source_extensions_catalog).where(source_extensions_catalog.c.dataset_id == dataset_id) - .order_by(source_extensions_catalog.c.extension_key) + variable_set_ids = tuple( + str(row["variable_set_id"]) for row in variable_sets + ) + variable_set_members = connection.execute( + select(normative.variable_set_member) + .where(normative.variable_set_member.c.variable_set_id.in_(variable_set_ids)) + .order_by( + normative.variable_set_member.c.variable_set_id, + normative.variable_set_member.c.source_ordinal, + ) ).mappings().all() - - if document_rows_result: - dataset["documents"] = json.dumps([item["text"] for item in document_rows_result], ensure_ascii=False) - if attribute_rows_result: - file_attributes, variable_attributes = attributes_from_rows( - attribute_rows_result, variables=variables, + response_sets = connection.execute( + select(normative.multiple_response_set) + .where(normative.multiple_response_set.c.dataset_id == core_id) + .order_by(normative.multiple_response_set.c.source_ordinal) + ).mappings().all() + response_set_ids = tuple( + str(row["multiple_response_set_id"]) for row in response_sets ) - dataset["file_attributes"] = json.dumps(file_attributes, ensure_ascii=False) - for variable in variables: - variable["attributes"] = json.dumps( - variable_attributes.get(variable["source_name"], {}), ensure_ascii=False, + response_members = connection.execute( + select(normative.multiple_response_member) + .where( + normative.multiple_response_member.c.multiple_response_set_id.in_( + response_set_ids + ) ) - if mrset_rows_result: - dataset["multiple_response_sets"] = json.dumps( - multiple_response_sets_from_rows(mrset_rows_result), ensure_ascii=False, default=str, + .order_by( + normative.multiple_response_member.c.multiple_response_set_id, + normative.multiple_response_member.c.source_ordinal, + ) + ).mappings().all() + weight_id = connection.execute( + select(normative.dataset_weight_variable.c.variable_id).where( + normative.dataset_weight_variable.c.dataset_id == core_id + ) + ).scalar_one_or_none() + + dataset = { + "dataset_id": core_id, + "data_table": str(dataset_row["physical_table_name"]), + "physical_table_schema": dataset_row["physical_table_schema"], + "source_format": dataset_row["source_format"], + "source_encoding": dataset_row["source_encoding"], + "case_count": int(dataset_row["source_case_count"]), + "file_label": dataset_row["dataset_label"] or "", + "documents": json.dumps( + [row["document_text"] for row in documents], ensure_ascii=False, + ), + "file_attributes": json.dumps( + _collapse_attributes(dataset_attributes), ensure_ascii=False, + ), + "case_weight_variable": ( + variables_by_id[str(weight_id)]["source_name"] + if weight_id is not None else None + ), + } + for row in labels: + variable = variables_by_id[str(row["variable_id"])] + values = json.loads(variable["value_labels"]) + code = ( + row["numeric_code"] + if row["code_kind"] == "numeric" else row["string_code"] + ) + values[str(code)] = row["label"] + variable["value_labels"] = json.dumps(values, ensure_ascii=False) + for row in missing_rules: + variable = variables_by_id[str(row["variable_id"])] + rules = json.loads(variable["missing_ranges"]) + rules.append(_export_missing_rule(row)) + variable["missing_ranges"] = json.dumps(rules, ensure_ascii=False) + grouped_attributes: dict[str, list[Mapping[str, Any]]] = {} + for row in variable_attributes: + grouped_attributes.setdefault(str(row["variable_id"]), []).append(row) + for variable_id, attribute_rows in grouped_attributes.items(): + variables_by_id[variable_id]["attributes"] = json.dumps( + _collapse_attributes(attribute_rows), ensure_ascii=False, + ) + names_by_id = { + variable_id: variable["source_name"] + for variable_id, variable in variables_by_id.items() + } + members_by_variable_set: dict[str, list[str]] = {} + for row in variable_set_members: + members_by_variable_set.setdefault(str(row["variable_set_id"]), []).append( + names_by_id[str(row["variable_id"])] ) - if extension_rows_result: - dataset["source_extensions"] = { - item["extension_key"]: json.loads(item["payload"]) - for item in extension_rows_result + dataset["source_extensions"] = { + "spss.variable_sets": { + str(row["set_name"]): members_by_variable_set.get( + str(row["variable_set_id"]), [] + ) + for row in variable_sets } - variables_by_ordinal = {item["ordinal"]: item for item in variables} - labels_by_variable: dict[int, dict[Any, str]] = {} - for item in label_rows_result: - labels_by_variable.setdefault(item["variable_ordinal"], {})[ - item["numeric_value"] if item["value_type"] == "numeric" else item["text_value"] - ] = item["label"] - for ordinal, labels in labels_by_variable.items(): - variables_by_ordinal[ordinal]["value_labels"] = json.dumps(labels, ensure_ascii=False) - rules_by_variable: dict[int, list[dict[str, Any]]] = {} - for item in missing_rows_result: - lower = _endpoint_from_row(item, prefix="lower") - upper = _endpoint_from_row(item, prefix="upper") - rules_by_variable.setdefault(item["variable_ordinal"], []).append({"lo": lower, "hi": upper}) - for ordinal, rules in rules_by_variable.items(): - variables_by_ordinal[ordinal]["missing_ranges"] = json.dumps(rules, ensure_ascii=False) + } if variable_sets else {} + members_by_response_set: dict[str, list[str]] = {} + for row in response_members: + set_id = str(row["multiple_response_set_id"]) + variable_id = str(row["variable_id"]) + if variable_id not in names_by_id: + raise _catalog_error( + "multiple-response-member-not-found", + "A multiple-response set references an unknown variable.", + multiple_response_set_id=set_id, + variable_id=variable_id, + ) + members_by_response_set.setdefault(set_id, []).append( + names_by_id[variable_id] + ) + dataset["multiple_response_sets"] = json.dumps({ + str(row["set_name"]): _export_response_set( + row, + members_by_response_set.get( + str(row["multiple_response_set_id"]), [] + ), + ) + for row in response_sets + }, ensure_ascii=False) return dataset, variables, rows +def _verify_normative_catalog(connection: Any, tables: Any) -> None: + if not inspect(connection).has_table(tables.catalog_identity.name): + raise RuntimeError("The core OpenStatSpec catalog is absent.") + identities = connection.execute(select(tables.catalog_identity)).mappings().all() + if len(identities) != 1 or ( + identities[0]["catalog_identity_key"] != 1 + or identities[0]["contract_id"] != CATALOG_CONTRACT_ID + or identities[0]["schema_version"] != CATALOG_SCHEMA_VERSION + ): + raise RuntimeError("The core OpenStatSpec catalog identity is incompatible.") + + +def _resolve_normative_dataset( + connection: Any, tables: Any, identifier: str, +) -> Mapping[str, Any]: + row = connection.execute( + select(tables.dataset).where(tables.dataset.c.dataset_id == identifier) + ).mappings().one_or_none() + if row is not None: + return row + return connection.execute( + select(tables.dataset).where(tables.dataset.c.dataset_name == identifier) + ).mappings().one() + + +def _format_json(family: Any, width: Any, decimals: Any) -> str | None: + if family is None: + return None + try: + numeric_family = int(family) + except (TypeError, ValueError): + suffix = f".{int(decimals or 0)}" if decimals else "" + return f"{family}{int(width)}{suffix}" + return json.dumps([numeric_family, width, decimals]) + + +def _export_variable(row: Mapping[str, Any]) -> dict[str, Any]: + return { + "ordinal": int(row["source_ordinal"]), + "source_name": row["source_name"], + "physical_name": row["physical_name"], + "storage_kind": row["storage_kind"], + "string_width": row["declared_string_width"], + "label": row["variable_label"] or "", + "print_format": _format_json( + row["print_format_family"], row["print_format_width"], + row["print_format_decimals"], + ), + "write_format": _format_json( + row["write_format_family"], row["write_format_width"], + row["write_format_decimals"], + ), + "measure": row["measurement_level"], + "role": row["variable_role"], + "alignment": row["display_alignment"], + "display_width": row["display_width"], + "attributes": "{}", + "compat_name": None, + "value_labels": "{}", + "missing_ranges": "[]", + } + + +def _collapse_attributes(rows: Iterable[Mapping[str, Any]]) -> dict[str, Any]: + grouped: dict[str, list[str]] = {} + for row in rows: + grouped.setdefault(str(row["attribute_name"]), []).append( + str(row["attribute_value"]) + ) + return { + name: values[0] if len(values) == 1 else values + for name, values in grouped.items() + } + + +def _export_missing_rule(row: Mapping[str, Any]) -> Any: + if row["rule_kind"] == "discrete": + return ( + row["numeric_value"] + if row["code_kind"] == "numeric" else row["string_value"] + ) + lower = ( + -sys.float_info.max + if row["lower_special"] == "LOWEST" else row["numeric_lower"] + ) + upper = ( + sys.float_info.max + if row["upper_special"] == "HIGHEST" else row["numeric_upper"] + ) + return {"lo": lower, "hi": upper} + + +def _export_response_set( + row: Mapping[str, Any], members: list[str], +) -> dict[str, Any]: + definition: dict[str, Any] = { + "variable_list": members, + "is_dichotomy": str(row["set_kind"]).upper() == "MD", + "use_category_labels": row["category_label_behavior"] == "counted_values", + "use_first_var_label": row["label_source"] == "variable_label", + } + if row["set_label"] is not None: + definition["label"] = row["set_label"] + if row["counted_value_kind"] == "numeric": + definition["counted_value"] = row["counted_numeric_value"] + elif row["counted_value_kind"] == "string": + definition["counted_value"] = row["counted_string_value"] + return definition + + def read_fidelity_events(*, database_url: str, dataset_id: str) -> tuple[dict[str, Any], ...]: - """Read import-time fidelity diagnostics for a catalogued dataset.""" + """Read fidelity diagnostics from the normative catalog.""" engine = create_engine(database_url) - metadata = MetaData() - _, _, fidelity_event_catalog, _ = catalog(metadata) + normative = normative_catalog(MetaData()) with engine.connect() as connection: - fidelity_event_catalog.create(connection, checkfirst=True) + _verify_normative_catalog(connection, normative) + dataset = _resolve_normative_dataset(connection, normative, dataset_id) events = connection.execute( - select(fidelity_event_catalog) - .where(fidelity_event_catalog.c.dataset_id == dataset_id) - .order_by(fidelity_event_catalog.c.code) + select(normative.fidelity_event) + .where(normative.fidelity_event.c.dataset_id == dataset["dataset_id"]) + .order_by(normative.fidelity_event.c.event_code) ).mappings().all() - return tuple({ - "code": item["code"], "detail": item["detail"], - "details": json.loads(item["details"] or "{}"), - } for item in events) - + result = [] + for item in events: + details = json.loads(item["detail_json"] or "{}") + result.append({ + "code": item["event_code"], + "detail": details.pop("message", ""), + "details": details, + }) + return tuple(result) def record_export_operation( @@ -1157,43 +971,32 @@ def record_export_operation( allowed_fidelity_events: Iterable[Mapping[str, Any]], operation_details: Mapping[str, Any] | None = None, ) -> str: - """Persist a completed export and the fidelity loss explicitly accepted by its caller.""" + """Persist a completed export only in the normative audit catalog.""" + del destination, operation_details engine = create_engine(database_url) - metadata = MetaData() - datasets, variables, fidelity_events, operations = catalog(metadata) - normative = normative_catalog(metadata) - multiple_response = multiple_response_set_catalog(metadata) + normative = normative_catalog(MetaData()) operation_id = str(uuid4()) events = tuple(allowed_fidelity_events) with engine.begin() as connection: - metadata.create_all(connection, tables=[datasets, variables, multiple_response, fidelity_events, operations]) - create_normative_catalog(connection, normative) - normative_dataset_id = normative_dataset_id_for_name(connection, normative, dataset_id) + _verify_normative_catalog(connection, normative) + dataset = _resolve_normative_dataset(connection, normative, dataset_id) completed_at = datetime.now(UTC).replace(tzinfo=None) record_normative_operation( connection, normative, operation_id=operation_id, operation_kind="export", status="succeeded", source_format=None, started_at=completed_at, completed_at=completed_at, ) - connection.execute(insert(operations).values( - operation_id=operation_id, direction="export", status="succeeded", dataset_id=dataset_id, - destination=destination, created_at=_now(), completed_at=_now(), - details=json.dumps({"allow_loss": [event["code"] for event in events], **dict(operation_details or {})}, sort_keys=True), - )) - rows = _event_rows( - operation_id=operation_id, dataset_id=dataset_id, direction="export", - fidelity_events=({**event, "severity": event.get("severity", "warning"), - "details": {**event.get("details", {}), "accepted_by_user": True}} - for event in events), - ) - if rows: - connection.execute(insert(fidelity_events), rows) record_normative_fidelity_events( connection, normative, operation_id=operation_id, - dataset_id=normative_dataset_id, direction="export", - events=({**event, "severity": event.get("severity", "warning"), - "details": {**event.get("details", {}), "accepted_by_user": True}} - for event in events), + dataset_id=str(dataset["dataset_id"]), direction="export", + events=({ + **event, + "severity": event.get("severity", "warning"), + "details": { + **event.get("details", {}), + "accepted_by_user": True, + }, + } for event in events), ) return operation_id @@ -1211,7 +1014,12 @@ def validate_wide_dataset(*, database_url: str, dataset_id: str) -> dict[str, An if not variables: raise ValueError("A conforming dataset needs at least one source variable.") expected_columns = {"__case_ordinal", *(item["physical_name"] for item in variables)} - reflected_table = Table(dataset["data_table"], MetaData(), autoload_with=create_engine(database_url)) + engine = create_engine(database_url) + reflected_table = Table( + dataset["data_table"], MetaData(), + schema=dataset["physical_table_schema"], + autoload_with=engine, + ) reflected_columns = {column.name: column for column in reflected_table.columns} actual_columns = set(reflected_columns) if actual_columns != expected_columns: diff --git a/tests/conformance.py b/tests/conformance.py index 677f865..dbe1ae3 100755 --- a/tests/conformance.py +++ b/tests/conformance.py @@ -143,7 +143,7 @@ def compare_sav_semantics(source: str | Path, exported: str | Path) -> dict[str, for attribute in ( "encoding", "file_label", "case_weight_var", "file_attributes", "mrsets", "var_types", "var_formats", "var_labels", "var_alignments", "var_column_widths", "var_measure_levels", "var_roles", - "var_value_labels", "var_attributes", "var_compat_names", + "var_value_labels", "var_attributes", ): if source_metadata.get(attribute) != exported_metadata.get(attribute): failures.append(attribute) diff --git a/tests/test_atomic_import.py b/tests/test_atomic_import.py index 88a98a3..7eadfff 100755 --- a/tests/test_atomic_import.py +++ b/tests/test_atomic_import.py @@ -44,17 +44,19 @@ def test_failed_preflight_persists_operation_without_creating_dataset(tmp_path) ) connection = sqlite3.connect(database_path) - assert connection.execute("select count(*) from dataset_catalog").fetchone() == (0,) + assert connection.execute("select count(*) from dataset").fetchone() == (0,) assert "data_too_wide" not in { row[0] for row in connection.execute("select name from sqlite_master where type = 'table'") } assert connection.execute( - "select direction, status, dataset_id from operation_catalog" - ).fetchall() == [("import", "failed", None)] + "select operation_kind, status from operation" + ).fetchall() == [("import", "failed")] direction, severity, code, details = connection.execute( - "select direction, severity, code, details from fidelity_event_catalog" + "select direction, severity, event_code, detail_json from fidelity_event" ).fetchone() - assert (direction, severity, code) == ("import", "error", "target_capability_exceeded") + assert (direction, severity, code) == ( + "import", "error", "target_capability_exceeded", + ) assert '"variable_count": 2001' in details @@ -75,9 +77,13 @@ def test_identifier_mapping_preflight_records_failure_before_dataset_creation(tm ) connection = sqlite3.connect(database_path) - assert connection.execute("select count(*) from dataset_catalog").fetchone() == (0,) - assert connection.execute("select dataset_id, status from operation_catalog").fetchall() == [(None, "failed")] - details = connection.execute("select details from fidelity_event_catalog").fetchone()[0] + assert connection.execute("select count(*) from dataset").fetchone() == (0,) + assert connection.execute( + "select status from operation" + ).fetchall() == [("failed",)] + details = connection.execute( + "select detail_json from fidelity_event" + ).fetchone()[0] assert '"reason": "physical_identifier_mapping_invalid"' in details @@ -102,14 +108,16 @@ def test_declared_string_width_preflight_is_atomic_and_diagnostic(tmp_path, monk ) connection = sqlite3.connect(database_path) - assert connection.execute("select count(*) from dataset_catalog").fetchone() == (0,) + assert connection.execute("select count(*) from dataset").fetchone() == (0,) assert "data_too_wide_string" not in { row[0] for row in connection.execute("select name from sqlite_master where type = 'table'") } assert connection.execute( - "select dataset_id, status from operation_catalog" - ).fetchall() == [(None, "failed")] - details = connection.execute("select details from fidelity_event_catalog").fetchone()[0] + "select status from operation" + ).fetchall() == [("failed",)] + details = connection.execute( + "select detail_json from fidelity_event" + ).fetchone()[0] assert '"reason": "declared_string_width_limit"' in details assert '"string_width": 4' in details @@ -153,26 +161,13 @@ def fail_after_normative_write(*args, **kwargs): "select name from sqlite_master where type = 'table'" ) } - if "dataset_catalog" in existing_tables: - assert connection.execute( - "select count(*) from dataset_catalog where dataset_id = 'cleanup'" - ).fetchone() == (0,) - if "variable_catalog" in existing_tables: - assert connection.execute( - "select count(*) from variable_catalog where dataset_id = 'cleanup'" - ).fetchone() == (0,) + assert not {name for name in existing_tables if name.endswith("_catalog")} assert connection.execute( "select count(*) from dataset where dataset_name = 'cleanup'" ).fetchone() == (0,) assert connection.execute( "select count(*) from variable" ).fetchone() == (0,) - assert connection.execute( - "select direction, status, dataset_id from operation_catalog" - ).fetchall() == [("import", "failed", None)] - assert connection.execute( - "select direction, severity, code, dataset_id from fidelity_event_catalog" - ).fetchall() == [("import", "error", "import_failed", None)] assert connection.execute( "select operation_kind, status from operation" ).fetchall() == [("import", "failed")] @@ -270,21 +265,14 @@ def test_empty_namespace_dolt_width_failure_initializes_identity_and_one_audit( assert connection.execute( "select dataset_id, event_code from fidelity_event" ).fetchall() == [(None, "target_capability_exceeded")] - assert connection.execute( - "select dataset_id, status from operation_catalog" - ).fetchall() == [(None, "failed")] - assert connection.execute( - "select dataset_id, code from fidelity_event_catalog" - ).fetchall() == [(None, "target_capability_exceeded")] -@pytest.mark.parametrize("failure_point", ["mirror_completion", "normative_completion"]) def test_nonatomic_failure_during_final_completion_still_cleans_dataset( - tmp_path, monkeypatch, failure_point, + tmp_path, monkeypatch, ) -> None: - database_path = tmp_path / f"{failure_point}.sqlite" + database_path = tmp_path / "normative-completion.sqlite" database = f"sqlite:///{database_path}" - dataset_id = f"cleanup-{failure_point}" + dataset_name = "cleanup-normative-completion" variables = [{ "ordinal": 1, "source_name": "name", "physical_name": "name", "storage_kind": "string", "string_width": 8, "label": "", @@ -295,64 +283,41 @@ def test_nonatomic_failure_during_final_completion_still_cleans_dataset( wide, "effective_profile", lambda _url: (replace(MYSQL, name="mysql"), {}), ) + real_finish = wide.finish_normative_operation triggered = False - if failure_point == "mirror_completion": - real_update = wide.update - - def fail_first_operation_update(table): - nonlocal triggered - if table.name == "operation_catalog" and not triggered: - triggered = True - raise RuntimeError("fault after data insert") - return real_update(table) - - monkeypatch.setattr(wide, "update", fail_first_operation_update) - else: - real_finish = wide.finish_normative_operation - - def fail_first_normative_finish(*args, **kwargs): - nonlocal triggered - if not triggered: - triggered = True - raise RuntimeError("fault during final completion") - return real_finish(*args, **kwargs) - - monkeypatch.setattr( - wide, "finish_normative_operation", fail_first_normative_finish, - ) + def fail_first_normative_finish(*args, **kwargs): + nonlocal triggered + if not triggered: + triggered = True + raise RuntimeError("fault during final completion") + return real_finish(*args, **kwargs) + + monkeypatch.setattr( + wide, "finish_normative_operation", fail_first_normative_finish, + ) with pytest.raises(RuntimeError, match="fault"): create_wide_dataset( - database_url=database, dataset_id=dataset_id, - source_name=f"{dataset_id}.sav", source_format="SAV", + database_url=database, dataset_id=dataset_name, + source_name=f"{dataset_name}.sav", source_format="SAV", rows=[{"name": "ok"}], variables=variables, ) connection = sqlite3.connect(database_path) assert triggered is True - assert f"data_{dataset_id}" not in { + tables = { row[0] for row in connection.execute( "select name from sqlite_master where type = 'table'" ) } - existing_tables = { - row[0] for row in connection.execute( - "select name from sqlite_master where type = 'table'" - ) - } - if "dataset_catalog" in existing_tables: - assert connection.execute( - "select count(*) from dataset_catalog where dataset_id = ?", (dataset_id,) - ).fetchone() == (0,) + assert f"data_{dataset_name}" not in tables + assert not {name for name in tables if name.endswith("_catalog")} assert connection.execute( - "select count(*) from dataset where dataset_name = ?", (dataset_id,) + "select count(*) from dataset where dataset_name = ?", (dataset_name,) ).fetchone() == (0,) - assert connection.execute( - "select status, dataset_id from operation_catalog" - ).fetchall() == [("failed", None)] assert connection.execute( "select status from operation" ).fetchall() == [("failed",)] assert connection.execute( - "select code, dataset_id from fidelity_event_catalog" + "select event_code, dataset_id from fidelity_event" ).fetchall() == [("import_failed", None)] diff --git a/tests/test_attribute_catalog.py b/tests/test_attribute_catalog.py index 97df2c1..3f2740a 100644 --- a/tests/test_attribute_catalog.py +++ b/tests/test_attribute_catalog.py @@ -6,7 +6,6 @@ import pytest import openstatspec -from openstatspec.core import UnsupportedOperationError from openstatspec.sql.wide import create_wide_dataset, read_wide_dataset @@ -14,7 +13,7 @@ @pytest.mark.parametrize("suffix", [".sav", ".zsav"]) -def test_attribute_catalog_is_authoritative_for_sav_and_zsav_export(tmp_path, suffix: str) -> None: +def test_normative_attributes_are_authoritative_for_sav_and_zsav_export(tmp_path, suffix: str) -> None: source = tmp_path / f"source{suffix}" destination = tmp_path / f"destination{suffix}" imported_again = tmp_path / f"again-{suffix[1:]}.sqlite" @@ -31,25 +30,26 @@ def test_attribute_catalog_is_authoritative_for_sav_and_zsav_export(tmp_path, su openstatspec.import_sav(source, database_url=database, dataset_id="attributes") connection = sqlite3.connect(database_path) assert connection.execute( - "select scope, variable_ordinal, attribute_ordinal, value_ordinal, attribute_name, attribute_value " - "from attribute_catalog where dataset_id = 'attributes' " - "order by scope, variable_ordinal, attribute_ordinal, value_ordinal" + "select attribute_name, array_ordinal, attribute_value " + "from dataset_attribute order by rowid" ).fetchall() == [ - ("file", 0, 1, 1, "Source", "source-file"), - ("file", 0, 2, 1, "Order", "second"), - ("variable", 1, 1, 1, "Source", "source-variable"), - ("variable", 1, 2, 1, "Flag", "yes"), + ("Source", 1, "source-file"), + ("Order", 1, "second"), + ] + assert connection.execute( + "select a.attribute_name, a.array_ordinal, a.attribute_value " + "from variable_attribute a order by a.rowid" + ).fetchall() == [ + ("Source", 1, "source-variable"), + ("Flag", 1, "yes"), ] - # Deliberately corrupt the legacy copies. Export must use the normalized rows. - connection.execute("update dataset_catalog set file_attributes = ? where dataset_id = 'attributes'", (json.dumps({"Source": "legacy-file"}),)) - connection.execute("update variable_catalog set attributes = ? where dataset_id = 'attributes'", (json.dumps({"Source": "legacy-variable"}),)) connection.execute( - "update attribute_catalog set attribute_value = 'catalog-file' " - "where dataset_id = 'attributes' and scope = 'file' and attribute_name = 'Source'" + "update dataset_attribute set attribute_value = 'catalog-file' " + "where attribute_name = 'Source'" ) connection.execute( - "update attribute_catalog set attribute_value = 'catalog-variable' " - "where dataset_id = 'attributes' and scope = 'variable' and attribute_name = 'Source'" + "update variable_attribute set attribute_value = 'catalog-variable' " + "where attribute_name = 'Source'" ) connection.commit() @@ -65,45 +65,14 @@ def test_attribute_catalog_is_authoritative_for_sav_and_zsav_export(tmp_path, su openstatspec.import_sav(destination, database_url=f"sqlite:///{imported_again}", dataset_id="again") reimported = sqlite3.connect(imported_again) assert reimported.execute( - "select attribute_name, attribute_value from attribute_catalog " - "where dataset_id = 'again' and scope = 'file' order by attribute_ordinal, value_ordinal" - ).fetchall() == [("Source", "catalog-file"), ("Order", "second")] + "select attribute_name, attribute_value from dataset_attribute order by attribute_name" + ).fetchall() == [("Order", "second"), ("Source", "catalog-file")] assert reimported.execute( - "select attribute_name, attribute_value from attribute_catalog " - "where dataset_id = 'again' and scope = 'variable' order by attribute_ordinal, value_ordinal" - ).fetchall() == [("Source", "catalog-variable"), ("Flag", "yes")] - - -def test_attribute_catalog_migrates_old_json_catalog_without_rewriting_it(tmp_path) -> None: - source = tmp_path / "legacy.sav" - destination = tmp_path / "legacy-out.sav" - database_path = tmp_path / "legacy.sqlite" - database = f"sqlite:///{database_path}" - pyspssio.write_sav( - str(source), pd.DataFrame({"answer": [1.0]}), - metadata={"file_attributes": {"File": "legacy"}, "var_attributes": {"answer": {"Var": "legacy"}}}, - ) - openstatspec.import_sav(source, database_url=database, dataset_id="legacy") - connection = sqlite3.connect(database_path) - connection.execute("drop table attribute_catalog") - connection.commit() - - # Opening an older catalog additively creates the new table and continues to - # use legacy JSON only because that older dataset has no normalized rows. - dataset, variables, _ = read_wide_dataset(database_url=database, dataset_id="legacy") - assert json.loads(dataset["file_attributes"]) == {"File": "legacy"} - assert json.loads(variables[0]["attributes"]) == {"Var": "legacy"} - assert connection.execute( - "select name from sqlite_master where type = 'table' and name = 'attribute_catalog'" - ).fetchone() == ("attribute_catalog",) - openstatspec.export_sav( - database_url=database, dataset_id="legacy", destination=destination, - allow_loss=_REQUIRED_ENGINE_LOSS, - ) - assert pyspssio.read_metadata(str(destination))["file_attributes"] == {"File": "legacy"} + "select attribute_name, attribute_value from variable_attribute order by attribute_name" + ).fetchall() == [("Flag", "yes"), ("Source", "catalog-variable")] -def test_attribute_catalog_preserves_ordered_arrays_through_raw_pyspssio_bridge(tmp_path) -> None: +def test_normative_attributes_preserve_ordered_arrays_through_raw_pyspssio_bridge(tmp_path) -> None: database = f"sqlite:///{tmp_path / 'array.sqlite'}" create_wide_dataset( database_url=database, dataset_id="array", source_name="array.sav", source_format="SAV", diff --git a/tests/test_document_round_trip.py b/tests/test_document_round_trip.py index 40dacf4..15d0270 100644 --- a/tests/test_document_round_trip.py +++ b/tests/test_document_round_trip.py @@ -10,7 +10,6 @@ import openstatspec from openstatspec.spss.raw_dictionary import ( read_document_lines, - write_compatible_names, write_document_lines, ) @@ -34,7 +33,7 @@ def test_document_lines_round_trip_through_sqlite(destination_suffix: str, tmp_p assert imported.diagnostics == () connection = sqlite3.connect(tmp_path / "documents.sqlite") assert connection.execute( - "select ordinal, text from document_catalog order by ordinal" + "select source_ordinal, document_text from document order by source_ordinal" ).fetchall() == [(1, expected[0]), (2, expected[1])] openstatspec.export_sav( @@ -73,20 +72,18 @@ def test_document_lines_import_from_zsav_and_export_to_sav(tmp_path: Path) -> No assert pyspssio.read_sav(str(destination))[0]["answer"].tolist() == [1.0, 2.0] -def test_document_and_compatible_name_round_trip_to_zsav(tmp_path: Path) -> None: +def test_document_and_long_variable_name_round_trip_to_zsav(tmp_path: Path) -> None: source = tmp_path / "source.sav" destination = tmp_path / "destination.zsav" database = f"sqlite:///{tmp_path / 'combined.sqlite'}" source_name = "long_variable_name" pyspssio.write_sav(str(source), pd.DataFrame({source_name: [7.0]})) write_document_lines(source, ["Combined dictionary fixture."], encoding="UTF-8") - write_compatible_names(source, {source_name: "ANSWER"}, encoding="UTF-8") openstatspec.import_sav(source, database_url=database, dataset_id="combined") openstatspec.export_sav(database_url=database, dataset_id="combined", destination=destination) assert read_document_lines(destination, encoding="UTF-8") == ["Combined dictionary fixture."] - assert pyspssio.read_metadata(str(destination))["var_compat_names"][source_name] == "ANSWER" assert pyspssio.read_sav(str(destination))[0][source_name].tolist() == [7.0] diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 3c4cda4..10b9fbb 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -141,9 +141,7 @@ def test_plan_applies_to_same_dataset_and_physical_table_without_copy( assert connection.execute( "SELECT numeric_code, label FROM value_label ORDER BY ordinal" ).fetchall() == [(0.0, "Lower"), (1.0, "Upper")] - assert connection.execute( - "SELECT label FROM variable_catalog WHERE source_name = 'score_band'" - ).fetchone() == ("Score band",) + assert "variable_catalog" not in tables assert connection.execute( "SELECT database_profile, dolt_branch, dolt_head_before, " "dolt_head_after, actor, status " diff --git a/tests/test_loss_reports.py b/tests/test_loss_reports.py index f1916f7..1b6732f 100755 --- a/tests/test_loss_reports.py +++ b/tests/test_loss_reports.py @@ -1,4 +1,3 @@ -import json import sqlite3 import pandas as pd @@ -8,7 +7,6 @@ import openstatspec from openstatspec.core import UnsupportedOperationError from openstatspec.sql.wide import create_wide_dataset -from openstatspec.spss.raw_dictionary import write_compatible_names from openstatspec.spss import sav as sav_module @@ -26,18 +24,19 @@ def test_persisted_import_fidelity_events_require_consent_after_reopen(tmp_path) imported = openstatspec.import_sav(source, database_url=database, dataset_id="persisted") assert {diagnostic.code for diagnostic in imported.diagnostics} == set(_REQUIRED_ENGINE_LOSS) connection = sqlite3.connect(database_path) - assert {row[0] for row in connection.execute("select code from fidelity_event_catalog")} == set(_REQUIRED_ENGINE_LOSS) - import_details = json.loads(connection.execute("select details from operation_catalog order by created_at limit 1").fetchone()[0]) - assert import_details["engine"]["package"] == "openstatspec-pyspssio" - assert import_details["engine"]["pinned_commit"] == "e069adf33c70bcd9e8e6ee495106479463a84fa2" + assert {row[0] for row in connection.execute("select event_code from fidelity_event")} == set(_REQUIRED_ENGINE_LOSS) + assert connection.execute( + "select operation_kind, status from operation order by started_at limit 1" + ).fetchone() == ("import", "succeeded") openstatspec.export_sav(database_url=database, dataset_id="persisted", destination=blocked) assert blocked.exists() exported = openstatspec.export_sav(database_url=database, dataset_id="persisted", destination=approved, allow_loss=_REQUIRED_ENGINE_LOSS) assert approved.exists() - export_details = json.loads(connection.execute("select details from operation_catalog where operation_id = ?", (exported["operation_id"],)).fetchone()[0]) - assert export_details["engine"]["installed_version"] == pyspssio.__version__ + assert connection.execute( + "select operation_kind, status from operation where operation_id = ?", (exported["operation_id"],) + ).fetchone() == ("export", "succeeded") assert {diagnostic.code for diagnostic in exported.diagnostics} == set(_REQUIRED_ENGINE_LOSS) @@ -51,7 +50,7 @@ def test_loss_allowed_export_persists_accepted_diagnostics(tmp_path) -> None: result = openstatspec.export_sav(database_url=database, dataset_id="accepted", destination=destination, allow_loss=_REQUIRED_ENGINE_LOSS) connection = sqlite3.connect(database_path) - rows = connection.execute("select direction, severity, code, details from fidelity_event_catalog where operation_id = ? order by code", (result["operation_id"],)).fetchall() + rows = connection.execute("select direction, severity, event_code, detail_json from fidelity_event where operation_id = ? order by event_code", (result["operation_id"],)).fetchall() assert [(row[0], row[1], row[2]) for row in rows] == [("export", "warning", code) for code in _REQUIRED_ENGINE_LOSS] assert all('"accepted_by_user": true' in row[3] for row in rows) @@ -119,38 +118,3 @@ def test_legacy_locale_must_emit_the_exact_source_encoding() -> None: sav_module._require_matching_legacy_encoding("WINDOWS-1252", "CP1252", True) with pytest.raises(UnsupportedOperationError, match="instead of required"): sav_module._require_matching_legacy_encoding("WINDOWS-1252", "UTF-8", True) - -@pytest.mark.parametrize("suffix", [".sav", ".zsav"]) -def test_compatible_variable_name_round_trips_from_current_sql_catalog(tmp_path, suffix: str) -> None: - source = tmp_path / f"compat-source{suffix}" - database_path = tmp_path / f"compat-{suffix[1:]}.sqlite" - database = f"sqlite:///{database_path}" - destination = tmp_path / f"compat-destination{suffix}" - source_name = "long_variable_name" - - pyspssio.write_sav(str(source), pd.DataFrame({source_name: [1.0]})) - write_compatible_names(source, {source_name: "ANSWER"}, encoding="UTF-8") - assert pyspssio.read_metadata(str(source))["var_compat_names"][source_name] == "ANSWER" - - imported = openstatspec.import_sav(source, database_url=database, dataset_id=f"compat-{suffix[1:]}") - assert imported.diagnostics == () - connection = sqlite3.connect(database_path) - assert connection.execute( - "select compat_name from variable_catalog where dataset_id = ? and source_name = ?", - (f"compat-{suffix[1:]}", source_name), - ).fetchone() == ("ANSWER",) - - # The normalized catalog is authoritative: a legitimate 8-byte short name - # edited there must be written into both SAV dictionary locations. - connection.execute( - "update variable_catalog set compat_name = ? where dataset_id = ? and source_name = ?", - ("EXAMPLE", f"compat-{suffix[1:]}", source_name), - ) - connection.commit() - result = openstatspec.export_sav( - database_url=database, dataset_id=f"compat-{suffix[1:]}", destination=destination, - ) - assert result.diagnostics == () - metadata = pyspssio.read_metadata(str(destination)) - assert metadata["var_compat_names"][source_name] == "EXAMPLE" - assert pyspssio.read_sav(str(destination))[0][source_name].tolist() == [1.0] diff --git a/tests/test_pyspssio_catalog_authority.py b/tests/test_pyspssio_catalog_authority.py index f160b34..b9ec43d 100644 --- a/tests/test_pyspssio_catalog_authority.py +++ b/tests/test_pyspssio_catalog_authority.py @@ -1,4 +1,3 @@ -import json import sqlite3 import pandas as pd @@ -29,8 +28,11 @@ def dictionary_with_variable_set(path): assert {diagnostic.code for diagnostic in imported.diagnostics} == set() connection = sqlite3.connect(database_path) assert connection.execute( - "select extension_key, payload from source_extension_catalog where dataset_id = 'variables'" - ).fetchone() == ("spss.variable_sets", json.dumps({"Analysis": ["answer"]})) + "select vs.set_name, v.source_name from variable_set vs " + "join variable_set_member vsm on vsm.variable_set_id = vs.variable_set_id " + "join variable v on v.variable_id = vsm.variable_id " + "order by vs.source_ordinal, vsm.source_ordinal" + ).fetchall() == [("Analysis", "answer")] def test_normalized_mr_catalog_is_authoritative_for_export(tmp_path) -> None: @@ -57,20 +59,24 @@ def test_normalized_mr_catalog_is_authoritative_for_export(tmp_path) -> None: openstatspec.import_sav(source, database_url=database, dataset_id="mr") connection = sqlite3.connect(database_path) rows = connection.execute( - "select set_name, kind, is_dichotomy, use_category_labels, use_first_var_label, counted_value_type, counted_numeric, variable_name " - "from multiple_response_set_catalog order by set_name, member_ordinal" + "select mrs.set_name, mrs.set_kind, " + "mrs.category_label_behavior, mrs.label_source, " + "mrs.counted_value_kind, mrs.counted_numeric_value, v.source_name " + "from multiple_response_set mrs " + "join multiple_response_member mrm " + "on mrm.multiple_response_set_id = mrs.multiple_response_set_id " + "join variable v on v.variable_id = mrm.variable_id " + "order by mrs.set_name, mrm.source_ordinal" ).fetchall() assert rows == [ - ("$extended", "MD", 1, 1, 1, "numeric", 1.0, "ex_a"), - ("$extended", "MD", 1, 1, 1, "numeric", 1.0, "ex_b"), - ("$mc", "MC", 0, 0, 0, None, None, "mc_a"), - ("$mc", "MC", 0, 0, 0, None, None, "mc_b"), - ("$md", "MD", 1, 0, 0, "numeric", 1.0, "md_a"), - ("$md", "MD", 1, 0, 0, "numeric", 1.0, "md_b"), + ("$extended", "MD", "counted_values", "variable_label", "numeric", 1.0, "ex_a"), + ("$extended", "MD", "counted_values", "variable_label", "numeric", 1.0, "ex_b"), + ("$mc", "MC", "variable_labels", "set_label", None, None, "mc_a"), + ("$mc", "MC", "variable_labels", "set_label", None, None, "mc_b"), + ("$md", "MD", "variable_labels", "set_label", "numeric", 1.0, "md_a"), + ("$md", "MD", "variable_labels", "set_label", "numeric", 1.0, "md_b"), ] - # The JSON is only legacy compatibility now; normalized rows must drive the writer. - connection.execute("update dataset_catalog set multiple_response_sets = '{}' where dataset_id = 'mr'") - connection.execute("update multiple_response_set_catalog set label = 'MD catalog' where dataset_id = 'mr' and set_name = '$md'") + connection.execute("update multiple_response_set set set_label = 'MD catalog' where set_name = '$md'") connection.commit() openstatspec.export_sav(database_url=database, dataset_id="mr", destination=destination, allow_loss=_REQUIRED_ENGINE_LOSS) exported = pyspssio.read_metadata(str(destination))["mrsets"] diff --git a/tests/test_sav_sqlite.py b/tests/test_sav_sqlite.py index 8b99980..13a82f1 100644 --- a/tests/test_sav_sqlite.py +++ b/tests/test_sav_sqlite.py @@ -47,19 +47,33 @@ def test_pyspssio_round_trip_uses_one_wide_table_and_catalog(tmp_path) -> None: connection = sqlite3.connect(database_path) table_names = [row[0] for row in connection.execute("select name from sqlite_master where type = 'table' order by name")] assert { - "attribute_catalog", "data_tiny", "dataset_catalog", "document_catalog", - "fidelity_event_catalog", "missing_rule_catalog", - "multiple_response_set_catalog", "operation_catalog", - "source_extension_catalog", "value_label_catalog", "variable_catalog", - "dataset", "operation", "variable", "value_label_set", "value_label", + "catalog_identity", "data_tiny", "dataset", "operation", "variable", + "dataset_weight_variable", "value_label_set", "value_label", "variable_value_label_set", "missing_rule", "dataset_attribute", "variable_attribute", "document", "variable_set", "variable_set_member", "multiple_response_set", "multiple_response_member", "fidelity_event", } <= set(table_names) + assert not {name for name in table_names if name.endswith("_catalog")} assert connection.execute("select __case_ordinal, age, name from data_tiny order by __case_ordinal").fetchall() == [(1, 34.0, "Ada"), (2, None, "")] - assert connection.execute("select source_encoding, file_attributes, case_weight_variable from dataset_catalog").fetchone() == ("UTF-8", json.dumps({"Source": "test"}), "age") - assert connection.execute("select source_sha256 from dataset_catalog").fetchone() == (hashlib.sha256(source.read_bytes()).hexdigest(),) - assert connection.execute("select role, alignment, display_width, attributes from variable_catalog where source_name = 'age'").fetchone() == ("target", "right", 12, json.dumps({"Origin": "fixture"})) + assert connection.execute( + "select source_encoding, source_hash from dataset" + ).fetchone() == ("UTF-8", hashlib.sha256(source.read_bytes()).hexdigest()) + assert connection.execute( + "select attribute_name, attribute_value from dataset_attribute" + ).fetchall() == [("Source", "test")] + assert connection.execute( + "select variable_role, display_alignment, display_width from variable " + "where source_name = 'age'" + ).fetchone() == ("target", "right", 12) + assert connection.execute( + "select a.attribute_name, a.attribute_value from variable_attribute a " + "join variable v on v.variable_id = a.variable_id " + "where v.source_name = 'age'" + ).fetchall() == [("Origin", "fixture")] + assert connection.execute( + "select v.source_name from dataset_weight_variable w " + "join variable v on v.variable_id = w.variable_id" + ).fetchone() == ("age",) openstatspec.export_sav(database_url=database, dataset_id="tiny", destination=exported) frame, meta = pyspssio.read_sav(str(exported), convert_datetimes=False, include_user_missing=True) @@ -92,7 +106,7 @@ def test_file_label_round_trips_through_sqlite_and_export(tmp_path) -> None: openstatspec.import_sav(source, database_url=database, dataset_id="label") connection = sqlite3.connect(database_path) assert connection.execute( - "select file_label from dataset_catalog where dataset_id = ?", ("label",) + "select dataset_label from dataset where dataset_name = ?", ("label",) ).fetchone() == (label,) openstatspec.export_sav( @@ -128,7 +142,9 @@ def test_import_rejects_physical_table_name_collision_without_partial_catalog(tm with pytest.raises(ValueError, match="collides"): openstatspec.import_sav(source, database_url=database, dataset_id="wave 1") connection = sqlite3.connect(database_path) - assert connection.execute("select dataset_id from dataset_catalog").fetchall() == [("wave-1",)] + assert connection.execute( + "select dataset_name from dataset" + ).fetchall() == [("wave-1",)] @pytest.mark.parametrize("suffix", [".sav", ".zsav"]) def test_raw_dictionary_bridge_preserves_distinct_formats_sets_and_attribute_arrays(tmp_path, suffix: str) -> None: """The raw IBM I/O path, rather than write_sav, is the fidelity proof.""" @@ -166,13 +182,16 @@ def test_raw_dictionary_bridge_preserves_distinct_formats_sets_and_attribute_arr openstatspec.import_sav(source, database_url=database, dataset_id="raw") connection = sqlite3.connect(tmp_path / f"raw-{suffix[1:]}.sqlite") assert connection.execute( - "select print_format, write_format from variable_catalog " - "where dataset_id = 'raw' and source_name = 'answer'" - ).fetchone() == ("[5, 8, 1]", "[3, 12, 3]") + "select print_format_family, print_format_width, print_format_decimals, " + "write_format_family, write_format_width, write_format_decimals " + "from variable where source_name = 'answer'" + ).fetchone() == ("5", 8, 1, "3", 12, 3) assert connection.execute( - "select payload from source_extension_catalog " - "where dataset_id = 'raw' and extension_key = 'spss.variable_sets'" - ).fetchone() == (json.dumps({"Analysis": ["answer", "comment"]}),) + "select s.set_name, v.source_name from variable_set s " + "join variable_set_member m on m.variable_set_id = s.variable_set_id " + "join variable v on v.variable_id = m.variable_id " + "order by s.source_ordinal, m.source_ordinal" + ).fetchall() == [("Analysis", "answer"), ("Analysis", "comment")] openstatspec.export_sav( database_url=database, dataset_id="raw", destination=destination, allow_loss=_REQUIRED_ENGINE_LOSS, @@ -204,8 +223,8 @@ def test_very_long_string_round_trips_through_sqlite_and_export(tmp_path, suffix openstatspec.import_sav(source, database_url=database, dataset_id="long") connection = sqlite3.connect(database_path) assert connection.execute( - "select string_width from variable_catalog where dataset_id = ? and source_name = ?", - ("long", "comment"), + "select declared_string_width from variable where source_name = ?", + ("comment",), ).fetchone() == (payload_width,) openstatspec.export_sav( diff --git a/tests/test_spss_catalog_preflight.py b/tests/test_spss_catalog_preflight.py index 1b2aa64..81a171d 100755 --- a/tests/test_spss_catalog_preflight.py +++ b/tests/test_spss_catalog_preflight.py @@ -56,17 +56,17 @@ def test_import_catalog_preflight_rejects_invalid_weight_atomically( assert error.value.code == expected_code assert error.value.details["reason"] == expected_code connection = sqlite3.connect(database_path) - assert connection.execute("select count(*) from dataset_catalog").fetchone() == (0,) + assert connection.execute("select count(*) from dataset").fetchone() == (0,) assert "data_weight" not in { row[0] for row in connection.execute("select name from sqlite_master where type = 'table'") } assert connection.execute( - "select status, dataset_id from operation_catalog" - ).fetchall() == [("failed", None)] + "select status from operation" + ).fetchall() == [("failed",)] code, details = connection.execute( - "select code, details from fidelity_event_catalog" + "select event_code, detail_json from fidelity_event" ).fetchone() - assert code == expected_code + assert code == expected_code.replace("-", "_") assert json.loads(details)["reason"] == expected_code assert connection.execute("select count(*) from dataset").fetchone() == (0,) operation = connection.execute( @@ -79,7 +79,7 @@ def test_import_catalog_preflight_rejects_invalid_weight_atomically( "select dataset_id, direction, severity, event_code, source_item, " "detail_json, created_at from fidelity_event" ).fetchone() - assert event[:5] == (None, "import", "error", expected_code, "weight.sav") + assert event[:5] == (None, "import", "error", expected_code.replace("-", "_"), "weight.sav") assert json.loads(event[5])["reason"] == expected_code assert event[6] @@ -115,29 +115,32 @@ def _create_valid_dataset(database: str) -> None: [ ( lambda connection: connection.execute( - "update dataset_catalog set case_weight_variable = 'text' where dataset_id = 'mr'" + "update dataset_weight_variable set variable_id = " + "(select variable_id from variable where source_name = 'text')" ), "case-weight-variable-not-numeric", ), ( lambda connection: connection.execute( - "update multiple_response_set_catalog set variable_name = 'missing' " - "where dataset_id = 'mr' and member_ordinal = 1" + "update multiple_response_member set variable_id = " + "'00000000-0000-0000-0000-000000000000' " + "where source_ordinal = 1" ), "multiple-response-member-not-found", ), ( lambda connection: connection.execute( - "update multiple_response_set_catalog set variable_name = 'text' " - "where dataset_id = 'mr' and member_ordinal = 2" + "update multiple_response_member set variable_id = " + "(select variable_id from variable where source_name = 'text') " + "where source_ordinal = 2" ), "multiple-response-member-type-mismatch", ), ( lambda connection: connection.execute( - "update multiple_response_set_catalog " - "set counted_value_type = 'text', counted_numeric = null, counted_text = 'yes' " - "where dataset_id = 'mr'" + "update multiple_response_set set " + "counted_value_kind = 'string', counted_numeric_value = null, " + "counted_string_value = 'yes'" ), "multiple-response-counted-value-type-mismatch", ), diff --git a/tests/test_sql_services.py b/tests/test_sql_services.py index e2e664f..706b0fc 100755 --- a/tests/test_sql_services.py +++ b/tests/test_sql_services.py @@ -54,7 +54,10 @@ def test_live_profile_import_validate_and_export(environment_name, dataset_id, s engine = create_engine(database_url) with engine.connect() as connection: assert connection.execute(text(f"SELECT COUNT(*) FROM {imported['data_table']} ")).scalar_one() == 2 - assert connection.execute(text("SELECT COUNT(*) FROM variable_catalog WHERE dataset_id = :dataset_id"), {"dataset_id": runtime_dataset_id}).scalar_one() == 2 + assert connection.execute(text( + "SELECT COUNT(*) FROM variable v JOIN dataset d ON d.dataset_id = v.dataset_id " + "WHERE d.dataset_name = :dataset_name" + ), {"dataset_name": runtime_dataset_id}).scalar_one() == 2 destination = tmp_path / f"{dataset_id}.sav" exported = openstatspec.export_sav(database_url=database_url, dataset_id=runtime_dataset_id, destination=destination, allow_loss=_REQUIRED_ENGINE_LOSS) assert destination.exists() @@ -133,23 +136,11 @@ def test_live_dolt_conservative_source_width_envelope(tmp_path) -> None: assert connection.execute(text( "select count(*) from dataset where dataset_name = :name" ), {"name": rejected_id}).scalar_one() == 0 - assert connection.execute(text( - "select count(*) from dataset_catalog where dataset_id = :name" - ), {"name": rejected_id}).scalar_one() == 0 - mirror_event = connection.execute(text(""" - select f.dataset_id, f.direction, f.severity, f.code - from fidelity_event_catalog f - join operation_catalog o on o.operation_id = f.operation_id - where o.source = :source - """), {"source": rejected_source.name}).mappings().one() normative_event = connection.execute(text(""" select f.dataset_id, f.direction, f.severity, f.event_code from fidelity_event f where f.source_item = :source """), {"source": rejected_source.name}).mappings().one() - assert tuple(mirror_event.values()) == ( - None, "import", "error", "target_capability_exceeded", - ) assert tuple(normative_event.values()) == ( None, "import", "error", "target_capability_exceeded", ) @@ -183,19 +174,13 @@ def fail_after_normative_write(*args, **kwargs): assert connection.execute(text( "select count(*) from dataset where dataset_name = :name" ), {"name": dataset_id}).scalar_one() == 0 - assert connection.execute(text( - "select count(*) from dataset_catalog where dataset_id = :name" - ), {"name": dataset_id}).scalar_one() == 0 - assert connection.execute(text( - "select status, dataset_id from operation_catalog where source = :source" - ), {"source": source.name}).one() == ("failed", None) assert connection.execute(text(""" - select f.dataset_id, f.direction, f.severity, f.code - from fidelity_event_catalog f - join operation_catalog o on o.operation_id = f.operation_id - where o.source = :source + select o.status, f.dataset_id, f.direction, f.severity, f.event_code + from fidelity_event f + join operation o on o.operation_id = f.operation_id + where f.source_item = :source """), {"source": source.name}).one() == ( - None, "import", "error", "import_failed", + "failed", None, "import", "error", "import_failed", ) assert f"data_{dataset_id}" not in inspect_database(engine).get_table_names() @@ -241,9 +226,6 @@ def test_live_dolt_adapter_value_boundary_is_atomic() -> None: assert connection.execute(text( f"SELECT OCTET_LENGTH(value) FROM {quote(accepted_table)}" )).scalar_one() == 65_504 - assert connection.execute(text( - "SELECT COUNT(*) FROM dataset_catalog WHERE dataset_id = :dataset_id" - ), {"dataset_id": rejected_id}).scalar_one() == 0 assert connection.execute(text( "SELECT COUNT(*) FROM dataset WHERE dataset_name = :dataset_id" ), {"dataset_id": rejected_id}).scalar_one() == 0 diff --git a/tests/test_vls_compatible_names.py b/tests/test_vls_compatible_names.py index ff1f48c..c14f626 100644 --- a/tests/test_vls_compatible_names.py +++ b/tests/test_vls_compatible_names.py @@ -1,12 +1,9 @@ -import sqlite3 - import pandas as pd import pyspssio import pytest import openstatspec from openstatspec.spss import raw_dictionary -from openstatspec.spss import sav as sav_module _SOURCE_NAME = "a05x_very_long_source_name" @@ -57,14 +54,11 @@ def _replace_subtype_14_payload(path, payload: bytes) -> None: @pytest.mark.parametrize("suffix", [".sav", ".zsav"]) -def test_vls_custom_compatible_name_round_trips_as_one_variable(tmp_path, suffix: str) -> None: +def test_vls_round_trips_as_one_variable(tmp_path, suffix: str) -> None: source = tmp_path / f"source{suffix}" destination = tmp_path / f"destination{suffix}" database = f"sqlite:///{tmp_path / f'vls-{suffix[1:]}.sqlite'}" _write_vls_source(source) - raw_dictionary.write_compatible_names( - source, {_SOURCE_NAME: _COMPATIBLE_NAME}, encoding="UTF-8", - ) imported = openstatspec.import_sav( source, database_url=database, dataset_id=f"vls-{suffix[1:]}", @@ -81,10 +75,10 @@ def test_vls_custom_compatible_name_round_trips_as_one_variable(tmp_path, suffix frame = pyspssio.read_sav(str(destination))[0] assert metadata["var_names"] == ["before", _SOURCE_NAME, "after"] assert metadata["var_types"][_SOURCE_NAME] == 360 - assert metadata["var_compat_names"][_SOURCE_NAME] == _COMPATIBLE_NAME + compatible_name = metadata["var_compat_names"][_SOURCE_NAME] assert list(frame.columns) == ["before", _SOURCE_NAME, "after"] assert frame[_SOURCE_NAME].tolist() == [_VALUE, "", "tail"] - assert _subtype_14_entries(destination) == [(_COMPATIBLE_NAME, 360)] + assert _subtype_14_entries(destination) == [(compatible_name, 360)] @pytest.mark.parametrize("damage", ["malformed", "duplicate"]) @@ -108,36 +102,3 @@ def test_vls_rewrite_rejects_invalid_subtype_14_without_publishing(tmp_path, dam assert source.read_bytes() == expected assert list(tmp_path.glob(f".{source.name}.*.tmp")) == [] - - -def test_malformed_vls_export_removes_output_and_records_no_success(tmp_path, monkeypatch) -> None: - source = tmp_path / "source.sav" - destination = tmp_path / "failed.sav" - database_path = tmp_path / "failed.sqlite" - database = f"sqlite:///{database_path}" - _write_vls_source(source) - raw_dictionary.write_compatible_names( - source, {_SOURCE_NAME: _COMPATIBLE_NAME}, encoding="UTF-8", - ) - openstatspec.import_sav(source, database_url=database, dataset_id="failed-vls") - real_write = sav_module.write_compatible_names - - def malformed_write(path, names, *, encoding): - data, _, record = _subtype_14_record(path) - payload = data[record.start + 16 : record.end].replace(b"\x00", b"!", 1) - _replace_subtype_14_payload(path, payload) - real_write(path, names, encoding=encoding) - - monkeypatch.setattr(sav_module, "write_compatible_names", malformed_write) - with pytest.raises(raw_dictionary.RawDictionaryError): - openstatspec.export_sav( - database_url=database, - dataset_id="failed-vls", - destination=destination, - ) - - assert not destination.exists() - connection = sqlite3.connect(database_path) - assert connection.execute( - "select direction, status from operation_catalog order by created_at" - ).fetchall() == [("import", "succeeded")] From e09ba6ef376a8d7f99bf5bc201ed791f9c5e03da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 18:41:10 +0300 Subject: [PATCH 02/27] Address strict catalog review findings --- CHANGELOG.md | 3 ++ src/openstatspec/api.py | 2 +- src/openstatspec/spss/sav.py | 15 ++++++ src/openstatspec/sql/wide.py | 16 ++++--- tests/test_atomic_import.py | 74 ++++++++++++++++++++++++++++++ tests/test_cli.py | 2 +- tests/test_document_round_trip.py | 5 +- tests/test_loss_reports.py | 34 ++++++++++++++ tests/test_official_conformance.py | 9 +++- tests/test_sav_sqlite.py | 2 +- tests/test_sql_services.py | 2 +- tests/test_vls_compatible_names.py | 9 +++- 12 files changed, 157 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9066451..95e57d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ use the normative UUID-keyed OpenStatSpec catalog exclusively. - Existing databases that contain only the former compatibility catalog must be remediated manually before export. +- Imports now reject dataset names that collide with normative UUID identifiers, + cleanup drops only a physical table actually created by the failing import, + and non-preserved SPSS compatible variable names are reported as an explicit All notable changes to this reference implementation are documented here. ## 0.4.0 — 2026-07-31 diff --git a/src/openstatspec/api.py b/src/openstatspec/api.py index 9de2e21..c3337bd 100644 --- a/src/openstatspec/api.py +++ b/src/openstatspec/api.py @@ -73,7 +73,7 @@ def capability_matrix(database_url: str | None = None) -> Mapping[str, Any]: "multiple_response_sets": "supported", "variable_alignment": "supported", "variable_sets": "supported", - "compatible_variable_names": "supported", + "compatible_variable_names": "requires-explicit-loss-consent", "custom_attributes": { "scalar_values": "supported", "ordered_value_arrays": "supported", diff --git a/src/openstatspec/spss/sav.py b/src/openstatspec/spss/sav.py index 7e51cfa..7c0f418 100644 --- a/src/openstatspec/spss/sav.py +++ b/src/openstatspec/spss/sav.py @@ -494,6 +494,21 @@ def _engine_loss_report(metadata: dict[str, Any]) -> dict[str, dict[str, Any]]: "detail": "pyspssio could not inspect source variable sets; they cannot be preserved silently.", "details": {"engine_error": metadata.get("_var_sets_error", "unknown")}, } + compatible_names = { + str(source_name): str(compatible_name) + for source_name, compatible_name + in dict(metadata.get("var_compat_names") or {}).items() + if compatible_name + and str(source_name).casefold() != str(compatible_name).casefold() + } + if compatible_names: + events["compatible-variable-names-not-preserved"] = { + "code": "compatible-variable-names-not-preserved", + "detail": ( + "The normative catalog does not store SPSS compatible variable names." + ), + "details": {"variable_names": sorted(compatible_names)}, + } if _is_non_utf8_encoding(metadata.get("encoding")): events["source-encoding-not-preserved"] = { "code": "source-encoding-not-preserved", diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index e38ed84..5a3d540 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -11,7 +11,7 @@ from sqlalchemy import ( BigInteger, Column, Float, MetaData, Table, Text, create_engine, insert, - inspect, select, + inspect, or_, select, ) from sqlalchemy.dialects import mysql, postgresql, sqlite from ..core import UnsupportedOperationError @@ -509,16 +509,18 @@ def create_wide_dataset( normative_dataset_id, multiple_response_sets, ) namespace_owned = False - data_table_was_absent = False + data_table_created = False try: with engine.begin() as setup: create_normative_catalog(setup, normative) namespace_owned = True with engine.begin() as connection: if connection.execute( - select(normative.dataset.c.dataset_id).where( - normative.dataset.c.dataset_name == dataset_id - ) + select(normative.dataset.c.dataset_id).where(or_( + normative.dataset.c.dataset_name == dataset_id, + normative.dataset.c.dataset_id == dataset_id, + normative.dataset.c.dataset_name == normative_dataset_id, + )) ).first(): raise ValueError( f"Dataset {dataset_id!r} already exists; imports never overwrite a dataset." @@ -542,8 +544,8 @@ def create_wide_dataset( operation_kind="import", status="started", source_format=source_format, ) - data_table_was_absent = True data_table.create(connection) + data_table_created = True store_normative_dataset( connection, normative, dataset_name=dataset_id, source_format=source_format, physical_table_name=data_table.name, @@ -582,7 +584,7 @@ def create_wide_dataset( cleanup, normative, normative_dataset_id, ) if ( - data_table_was_absent + data_table_created and inspect(cleanup).has_table(data_table.name) ): data_table.drop(cleanup, checkfirst=True) diff --git a/tests/test_atomic_import.py b/tests/test_atomic_import.py index 7eadfff..c861524 100755 --- a/tests/test_atomic_import.py +++ b/tests/test_atomic_import.py @@ -32,6 +32,80 @@ def test_failed_row_insert_leaves_no_catalog_or_data_table(tmp_path) -> None: assert connection.execute("select count(*) from dataset").fetchone() == (0,) assert connection.execute("select count(*) from variable").fetchone() == (0,) +def test_failed_create_does_not_drop_a_concurrently_created_table( + tmp_path, monkeypatch, +) -> None: + database_path = tmp_path / "race.sqlite" + database = f"sqlite:///{database_path}" + variables = [{ + "ordinal": 1, "source_name": "name", "physical_name": "name", + "storage_kind": "string", "string_width": 8, "label": "", + "format": "A8", "measure": "nominal", "alignment": "left", + "display_width": 8, "value_labels": "{}", "missing_ranges": "[]", + }] + real_inspect = wide.inspect + inspected_data_table = 0 + + def inspect_with_concurrent_table(bind): + actual = real_inspect(bind) + + class Inspector: + def has_table(self, table_name): + nonlocal inspected_data_table + if table_name == "data_race": + inspected_data_table += 1 + return inspected_data_table > 1 + return actual.has_table(table_name) + + return Inspector() + + real_create = wide.Table.create + real_drop = wide.Table.drop + dropped = [] + + def fail_data_table_create(table, bind, **kwargs): + if table.name == "data_race": + raise RuntimeError("concurrent create won") + return real_create(table, bind, **kwargs) + + def observe_drop(table, bind, **kwargs): + if table.name == "data_race": + dropped.append(table.name) + return real_drop(table, bind, **kwargs) + + monkeypatch.setattr(wide, "inspect", inspect_with_concurrent_table) + monkeypatch.setattr(wide.Table, "create", fail_data_table_create) + monkeypatch.setattr(wide.Table, "drop", observe_drop) + + with pytest.raises(RuntimeError, match="concurrent create won"): + create_wide_dataset( + database_url=database, dataset_id="race", source_name="race.sav", + source_format="SAV", rows=[{"name": "ok"}], variables=variables, + ) + + assert inspected_data_table == 1 + assert dropped == [] + + +def test_dataset_name_cannot_equal_an_existing_normative_uuid(tmp_path) -> None: + database = f"sqlite:///{tmp_path / 'namespace.sqlite'}" + variables = [{ + "ordinal": 1, "source_name": "name", "physical_name": "name", + "storage_kind": "string", "string_width": 8, "label": "", + "format": "A8", "measure": "nominal", "alignment": "left", + "display_width": 8, "value_labels": "{}", "missing_ranges": "[]", + }] + first = create_wide_dataset( + database_url=database, dataset_id="first", source_name="first.sav", + source_format="SAV", rows=[{"name": "first"}], variables=variables, + ) + + with pytest.raises(ValueError, match="already exists"): + create_wide_dataset( + database_url=database, dataset_id=first["dataset_id"], + source_name="second.sav", source_format="SAV", + rows=[{"name": "second"}], variables=variables, + ) def test_failed_preflight_persists_operation_without_creating_dataset(tmp_path) -> None: database_path = tmp_path / "preflight.sqlite" diff --git a/tests/test_cli.py b/tests/test_cli.py index 26d3ff8..2cfb9fb 100755 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -64,7 +64,7 @@ def test_capability_matrix_is_public_and_cli_matches_engine_boundary(capsys) -> "multiple_response_sets": "supported", "variable_alignment": "supported", "variable_sets": "supported", - "compatible_variable_names": "supported", + "compatible_variable_names": "requires-explicit-loss-consent", "custom_attributes": { "scalar_values": "supported", "ordered_value_arrays": "supported", diff --git a/tests/test_document_round_trip.py b/tests/test_document_round_trip.py index 15d0270..19f18bd 100644 --- a/tests/test_document_round_trip.py +++ b/tests/test_document_round_trip.py @@ -81,7 +81,10 @@ def test_document_and_long_variable_name_round_trip_to_zsav(tmp_path: Path) -> N write_document_lines(source, ["Combined dictionary fixture."], encoding="UTF-8") openstatspec.import_sav(source, database_url=database, dataset_id="combined") - openstatspec.export_sav(database_url=database, dataset_id="combined", destination=destination) + openstatspec.export_sav( + database_url=database, dataset_id="combined", destination=destination, + allow_loss=["compatible-variable-names-not-preserved"], + ) assert read_document_lines(destination, encoding="UTF-8") == ["Combined dictionary fixture."] assert pyspssio.read_sav(str(destination))[0][source_name].tolist() == [7.0] diff --git a/tests/test_loss_reports.py b/tests/test_loss_reports.py index 1b6732f..a847df3 100755 --- a/tests/test_loss_reports.py +++ b/tests/test_loss_reports.py @@ -8,6 +8,7 @@ from openstatspec.core import UnsupportedOperationError from openstatspec.sql.wide import create_wide_dataset from openstatspec.spss import sav as sav_module +from openstatspec.spss.raw_dictionary import write_compatible_names _REQUIRED_ENGINE_LOSS = [] @@ -118,3 +119,36 @@ def test_legacy_locale_must_emit_the_exact_source_encoding() -> None: sav_module._require_matching_legacy_encoding("WINDOWS-1252", "CP1252", True) with pytest.raises(UnsupportedOperationError, match="instead of required"): sav_module._require_matching_legacy_encoding("WINDOWS-1252", "UTF-8", True) + + +def test_compatible_variable_names_are_explicit_imported_loss(tmp_path) -> None: + source = tmp_path / "compatible-name.sav" + destination = tmp_path / "compatible-name-out.sav" + database = f"sqlite:///{tmp_path / 'compatible-name.sqlite'}" + source_name = "long_variable_name" + loss_code = "compatible-variable-names-not-preserved" + + pyspssio.write_sav(str(source), pd.DataFrame({source_name: [1.0]})) + write_compatible_names(source, {source_name: "ANSWER"}, encoding="UTF-8") + + imported = openstatspec.import_sav( + source, database_url=database, dataset_id="compatible-name", + ) + assert {diagnostic.code for diagnostic in imported.diagnostics} == {loss_code} + assert imported.diagnostics[0].details == {"variable_names": [source_name]} + + with pytest.raises(UnsupportedOperationError, match=loss_code): + openstatspec.export_sav( + database_url=database, dataset_id="compatible-name", + destination=destination, + ) + + exported = openstatspec.export_sav( + database_url=database, dataset_id="compatible-name", + destination=destination, allow_loss=[loss_code], + ) + assert {diagnostic.code for diagnostic in exported.diagnostics} == {loss_code} + assert ( + pyspssio.read_metadata(str(destination))["var_compat_names"][source_name] + != "ANSWER" + ) diff --git a/tests/test_official_conformance.py b/tests/test_official_conformance.py index c2714de..f74a9fa 100644 --- a/tests/test_official_conformance.py +++ b/tests/test_official_conformance.py @@ -235,7 +235,9 @@ def _assert_round_trip( imported = openstatspec.import_sav( source, database_url=database_url, dataset_id=dataset_id, ) - assert imported.diagnostics == () + assert {diagnostic.code for diagnostic in imported.diagnostics} <= { + "compatible-variable-names-not-preserved", + } assert openstatspec.validate( database_url=database_url, dataset_id=dataset_id, )["valid"] is True @@ -243,8 +245,11 @@ def _assert_round_trip( exported = openstatspec.export_sav( database_url=database_url, dataset_id=dataset_id, destination=destination, + allow_loss=["compatible-variable-names-not-preserved"], ) - assert exported.diagnostics == () + assert {diagnostic.code for diagnostic in exported.diagnostics} <= { + "compatible-variable-names-not-preserved", + } assert compare_sav_semantics(source, destination) == { "equivalent": True, "differences": [], diff --git a/tests/test_sav_sqlite.py b/tests/test_sav_sqlite.py index 13a82f1..dd16eaf 100644 --- a/tests/test_sav_sqlite.py +++ b/tests/test_sav_sqlite.py @@ -12,7 +12,7 @@ _REQUIRED_ENGINE_LOSS = [] -_COMPAT_NAME_LOSS = _REQUIRED_ENGINE_LOSS +_COMPAT_NAME_LOSS = [*_REQUIRED_ENGINE_LOSS, "compatible-variable-names-not-preserved"] def test_pyspssio_round_trip_uses_one_wide_table_and_catalog(tmp_path) -> None: source = tmp_path / "tiny.sav" diff --git a/tests/test_sql_services.py b/tests/test_sql_services.py index 706b0fc..e5b45fd 100755 --- a/tests/test_sql_services.py +++ b/tests/test_sql_services.py @@ -17,7 +17,7 @@ pytestmark = pytest.mark.services _REQUIRED_ENGINE_LOSS = [] -_COMPAT_NAME_LOSS = _REQUIRED_ENGINE_LOSS +_COMPAT_NAME_LOSS = [*_REQUIRED_ENGINE_LOSS, "compatible-variable-names-not-preserved"] @pytest.fixture diff --git a/tests/test_vls_compatible_names.py b/tests/test_vls_compatible_names.py index c14f626..6fff434 100644 --- a/tests/test_vls_compatible_names.py +++ b/tests/test_vls_compatible_names.py @@ -63,13 +63,18 @@ def test_vls_round_trips_as_one_variable(tmp_path, suffix: str) -> None: imported = openstatspec.import_sav( source, database_url=database, dataset_id=f"vls-{suffix[1:]}", ) - assert imported.diagnostics == () + assert {diagnostic.code for diagnostic in imported.diagnostics} == { + "compatible-variable-names-not-preserved", + } exported = openstatspec.export_sav( database_url=database, dataset_id=f"vls-{suffix[1:]}", destination=destination, + allow_loss=["compatible-variable-names-not-preserved"], ) - assert exported.diagnostics == () + assert {diagnostic.code for diagnostic in exported.diagnostics} == { + "compatible-variable-names-not-preserved", + } metadata = pyspssio.read_metadata(str(destination)) frame = pyspssio.read_sav(str(destination))[0] From 95b0506bed243c0f6c4b66dea68867f580e994d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 18:51:21 +0300 Subject: [PATCH 03/27] Fix changelog entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95e57d7..2aa2199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +All notable changes to this reference implementation are documented here. ## Unreleased @@ -13,7 +14,7 @@ - Imports now reject dataset names that collide with normative UUID identifiers, cleanup drops only a physical table actually created by the failing import, and non-preserved SPSS compatible variable names are reported as an explicit -All notable changes to this reference implementation are documented here. + loss requiring export consent. ## 0.4.0 — 2026-07-31 From 9a39125c2df761ef089184dd318da83ecf6a0a60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 20:04:58 +0300 Subject: [PATCH 04/27] Normalize database decimal values for strict imports --- src/openstatspec/sql/wide.py | 23 ++++++++++++++++++++- tests/test_sql_profiles.py | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index fd974b7..7b0a579 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -8,6 +8,7 @@ from datetime import UTC, datetime from contextlib import contextmanager from uuid import uuid4 +from decimal import Decimal from collections.abc import Iterable, Mapping from typing import Any @@ -711,6 +712,26 @@ def _bounded_batches( yield batch +def _canonicalize_database_numeric_rows( + rows: Iterable[Mapping[str, Any]], + variables: Iterable[Mapping[str, Any]], +) -> list[dict[str, Any]]: + """Normalize finite DB-driver Decimal wrappers to binary64 values.""" + numeric_names = { + str(variable["physical_name"]) + for variable in variables + if variable.get("storage_kind") == "numeric" + } + normalized = [] + for source in rows: + row = dict(source) + for name in numeric_names: + if isinstance(row.get(name), Decimal) and row[name].is_finite(): + row[name] = float(row[name]) + normalized.append(row) + return normalized + + def data_table_name(dataset_id: str) -> str: stem = _IDENTIFIER.sub("_", dataset_id).strip("_").lower() or "dataset" return f"data_{stem[:48]}" @@ -755,7 +776,7 @@ def create_wide_dataset( operation_id = str(uuid4()) normative_dataset_id = str(uuid4()) fidelity_events = tuple(fidelity_events) - source_rows = list(rows) + source_rows = _canonicalize_database_numeric_rows(rows, variables) try: preflight(profile, variables, rows=source_rows) validate_spss_catalog( diff --git a/tests/test_sql_profiles.py b/tests/test_sql_profiles.py index b4b047b..3924777 100755 --- a/tests/test_sql_profiles.py +++ b/tests/test_sql_profiles.py @@ -1,4 +1,5 @@ from dataclasses import replace +from decimal import Decimal import os from types import SimpleNamespace @@ -666,6 +667,45 @@ def test_mysql_preflight_matches_emitted_text_limit() -> None: assert error.value.details["maximum"] == 65_535 +def test_database_decimal_numeric_wrappers_are_restored_to_binary64() -> None: + variables = [{"physical_name": "score", "storage_kind": "numeric"}] + + rows = wide._canonicalize_database_numeric_rows( + [ + {"score": Decimal("1.5000000000"), "name": "alpha"}, + {"score": None, "name": "missing"}, + ], + variables, + ) + + assert rows == [ + {"score": 1.5, "name": "alpha"}, + {"score": None, "name": "missing"}, + ] + assert isinstance(rows[0]["score"], float) + + +@pytest.mark.parametrize( + "value", + [Decimal("NaN"), Decimal("Infinity"), Decimal("-Infinity")], +) +def test_database_decimal_nonfinite_wrappers_remain_rejected(value) -> None: + variables = [{ + "ordinal": 1, + "source_name": "score", + "physical_name": "score", + "storage_kind": "numeric", + }] + rows = wide._canonicalize_database_numeric_rows( + [{"score": value}], variables, + ) + + assert rows[0]["score"] is value + with pytest.raises(UnsupportedOperationError) as error: + preflight(MYSQL, variables, rows=rows) + assert error.value.details["reason"] == "numeric_value_type" + + @pytest.mark.parametrize( ("claimed_supported", "driver_eligible"), [(False, True), (True, False)], From 57dda83730058c0a60acf93703910071affd4006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 20:14:18 +0300 Subject: [PATCH 05/27] Normalize numeric values at the database read boundary --- src/openstatspec/sql/wide.py | 1 + tests/test_sql_services.py | 161 +++++++---------------------------- 2 files changed, 32 insertions(+), 130 deletions(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 7b0a579..d9e8b2d 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -977,6 +977,7 @@ def read_wide_dataset( rows = [dict(row) for row in connection.execute( select(data_table).order_by(data_table.c.__case_ordinal) ).mappings()] + rows = _canonicalize_database_numeric_rows(rows, variables) documents = connection.execute( select(normative.document) .where(normative.document.c.dataset_id == core_id) diff --git a/tests/test_sql_services.py b/tests/test_sql_services.py index e5b45fd..ba2c177 100755 --- a/tests/test_sql_services.py +++ b/tests/test_sql_services.py @@ -1,4 +1,4 @@ -"""Real-service conformance checks for PostgreSQL, MySQL, MariaDB, and Dolt.""" +"""Real-service conformance checks plus Dolt fail-closed and candidate probes.""" import os from uuid import uuid4 @@ -10,14 +10,14 @@ from sqlalchemy.exc import DBAPIError import openstatspec -import openstatspec.sql.wide as wide from openstatspec.core import UnsupportedOperationError +from openstatspec.sql.dolt_conformance import DoltConformanceSource from conformance import compare_sav_semantics, write_supported_semantics_fixture pytestmark = pytest.mark.services _REQUIRED_ENGINE_LOSS = [] -_COMPAT_NAME_LOSS = [*_REQUIRED_ENGINE_LOSS, "compatible-variable-names-not-preserved"] +_COMPAT_NAME_LOSS = _REQUIRED_ENGINE_LOSS @pytest.fixture @@ -37,12 +37,13 @@ def source_sav(tmp_path): @pytest.mark.parametrize( ("environment_name", "dataset_id"), - [("OPENSTATSPEC_POSTGRES_URL", "profile_pg"), ("OPENSTATSPEC_MYSQL_URL", "profile_mysql"), ("OPENSTATSPEC_MARIADB_URL", "profile_mariadb"), ("OPENSTATSPEC_DOLT_URL", "profile_dolt")], + [("OPENSTATSPEC_POSTGRES_URL", "profile_pg"), ("OPENSTATSPEC_MYSQL_URL", "profile_mysql"), ("OPENSTATSPEC_MARIADB_URL", "profile_mariadb")], ) def test_live_profile_import_validate_and_export(environment_name, dataset_id, source_sav, tmp_path): database_url = os.environ.get(environment_name) if not database_url: pytest.skip(f"{environment_name} is not configured") + openstatspec.initialize_catalog(database_url=database_url) runtime_dataset_id = f"{dataset_id}_{uuid4().hex[:8]}" imported = openstatspec.import_sav( source_sav, database_url=database_url, dataset_id=runtime_dataset_id, @@ -72,13 +73,14 @@ def test_live_profile_import_validate_and_export(environment_name, dataset_id, s @pytest.mark.parametrize( ("environment_name", "dataset_id"), - [("OPENSTATSPEC_POSTGRES_URL", "semantics_pg"), ("OPENSTATSPEC_MYSQL_URL", "semantics_mysql"), ("OPENSTATSPEC_MARIADB_URL", "semantics_mariadb"), ("OPENSTATSPEC_DOLT_URL", "semantics_dolt")], + [("OPENSTATSPEC_POSTGRES_URL", "semantics_pg"), ("OPENSTATSPEC_MYSQL_URL", "semantics_mysql"), ("OPENSTATSPEC_MARIADB_URL", "semantics_mariadb")], ) @pytest.mark.parametrize("suffix", [".sav", ".zsav"]) def test_live_profile_preserves_supported_sav_semantics(environment_name, dataset_id, suffix, tmp_path): database_url = os.environ.get(environment_name) if not database_url: pytest.skip(f"{environment_name} is not configured") + openstatspec.initialize_catalog(database_url=database_url) runtime_dataset_id = f"{dataset_id}_{suffix[1:]}_{uuid4().hex[:8]}" source = tmp_path / f"{runtime_dataset_id}{suffix}" destination = tmp_path / f"{runtime_dataset_id}-roundtrip{suffix}" @@ -96,144 +98,43 @@ def test_live_profile_preserves_supported_sav_semantics(environment_name, datase ) assert compare_sav_semantics(source, destination) == {"equivalent": True, "differences": []} -def test_live_dolt_conservative_source_width_envelope(tmp_path) -> None: +def test_live_dolt_is_read_only_and_rejects_writes_without_declarations( + tmp_path, +) -> None: database_url = os.environ.get("OPENSTATSPEC_DOLT_URL") if not database_url: pytest.skip("OPENSTATSPEC_DOLT_URL is not configured") - token = uuid4().hex[:8] - accepted_id = f"dolt_width_accepted_{token}" - rejected_id = f"dolt_width_rejected_{token}" - accepted_source = tmp_path / f"{accepted_id}.sav" - rejected_source = tmp_path / f"{rejected_id}.sav" - accepted_columns = [f"v{ordinal:03d}" for ordinal in range(1, 306)] - rejected_columns = [*accepted_columns, "v306"] - pyspssio.write_sav( - str(accepted_source), - pd.DataFrame([[float(ordinal) for ordinal in range(1, 306)]], - columns=accepted_columns), - ) - pyspssio.write_sav( - str(rejected_source), - pd.DataFrame([[float(ordinal) for ordinal in range(1, 307)]], - columns=rejected_columns), - ) - imported = openstatspec.import_sav( - accepted_source, database_url=database_url, dataset_id=accepted_id, - ) - assert imported["case_count"] == 1 - assert openstatspec.validate( - database_url=database_url, dataset_id=accepted_id, - )["variable_count"] == 305 - - with pytest.raises(UnsupportedOperationError, match="Target capability exceeded"): - openstatspec.import_sav( - rejected_source, database_url=database_url, dataset_id=rejected_id, - ) + status = DoltConformanceSource.packaged().status() + assert status["status"] == "blocked_no_concrete_declarations" + assert status["declaration_count"] == 0 + assert status["write_enabled"] is False - engine = create_engine(database_url) - with engine.connect() as connection: - assert connection.execute(text( - "select count(*) from dataset where dataset_name = :name" - ), {"name": rejected_id}).scalar_one() == 0 - normative_event = connection.execute(text(""" - select f.dataset_id, f.direction, f.severity, f.event_code - from fidelity_event f - where f.source_item = :source - """), {"source": rejected_source.name}).mappings().one() - assert tuple(normative_event.values()) == ( - None, "import", "error", "target_capability_exceeded", - ) - assert f"data_{rejected_id}" not in inspect_database(engine).get_table_names() + before = openstatspec.dolt_state_snapshot(database_url=database_url) + assert before["read_only"] is True + assert before["operational_write_enabled"] is False + rejection = "no concrete declarations; write rejected before mutation" + with pytest.raises(UnsupportedOperationError, match=rejection): + openstatspec.initialize_catalog(database_url=database_url) + after_initialize = openstatspec.dolt_state_snapshot(database_url=database_url) -def test_live_dolt_post_ddl_fault_has_complete_compensating_cleanup( - tmp_path, monkeypatch, -) -> None: - database_url = os.environ.get("OPENSTATSPEC_DOLT_URL") - if not database_url: - pytest.skip("OPENSTATSPEC_DOLT_URL is not configured") - dataset_id = f"dolt_cleanup_{uuid4().hex[:8]}" - source = tmp_path / f"{dataset_id}.sav" + source = tmp_path / "blocked-write.sav" pyspssio.write_sav(str(source), pd.DataFrame({"answer": [1.0]})) - real_store = wide.store_normative_dataset - - def fail_after_normative_write(*args, **kwargs): - real_store(*args, **kwargs) - raise RuntimeError("injected Dolt post-DDL fault") - - monkeypatch.setattr(wide, "store_normative_dataset", fail_after_normative_write) - - with pytest.raises(RuntimeError, match="injected Dolt post-DDL fault"): + with pytest.raises(UnsupportedOperationError, match=rejection): openstatspec.import_sav( - source, database_url=database_url, dataset_id=dataset_id, - ) - - engine = create_engine(database_url) - with engine.connect() as connection: - assert connection.execute(text( - "select count(*) from dataset where dataset_name = :name" - ), {"name": dataset_id}).scalar_one() == 0 - assert connection.execute(text(""" - select o.status, f.dataset_id, f.direction, f.severity, f.event_code - from fidelity_event f - join operation o on o.operation_id = f.operation_id - where f.source_item = :source - """), {"source": source.name}).one() == ( - "failed", None, "import", "error", "import_failed", + source, database_url=database_url, dataset_id="blocked_write", ) - assert f"data_{dataset_id}" not in inspect_database(engine).get_table_names() + after_import = openstatspec.dolt_state_snapshot(database_url=database_url) - -def test_live_dolt_adapter_value_boundary_is_atomic() -> None: - database_url = os.environ.get("OPENSTATSPEC_DOLT_URL") - if not database_url: - pytest.skip("OPENSTATSPEC_DOLT_URL is not configured") - token = uuid4().hex[:8] - accepted_id = f"dolt_value_accepted_{token}" - rejected_id = f"dolt_value_rejected_{token}" - accepted_value = "é" * 32_752 - rejected_value = accepted_value + "x" - assert len(accepted_value.encode("utf-8")) == 65_504 - assert len(rejected_value.encode("utf-8")) == 65_505 - variables = [{ - "ordinal": 1, "source_name": "value", "physical_name": "value", - "storage_kind": "string", "string_width": 65_504, "label": "", - "format": "A65504", "measure": "nominal", "alignment": "left", - "display_width": 8, "value_labels": "{}", "missing_ranges": "[]", - }] - - imported = wide.create_wide_dataset( - database_url=database_url, dataset_id=accepted_id, - source_name="accepted.sav", source_format="SAV", - rows=[{"value": accepted_value}], variables=variables, - ) - assert imported["case_count"] == 1 - - with pytest.raises(UnsupportedOperationError) as caught: - wide.create_wide_dataset( - database_url=database_url, dataset_id=rejected_id, - source_name="rejected.sav", source_format="SAV", - rows=[{"value": rejected_value}], variables=variables, - ) - assert caught.value.details["reason"] == "text_value_limit" - - engine = create_engine(database_url) - accepted_table = wide.data_table_name(accepted_id) - rejected_table = wide.data_table_name(rejected_id) - quote = engine.dialect.identifier_preparer.quote - with engine.connect() as connection: - assert connection.execute(text( - f"SELECT OCTET_LENGTH(value) FROM {quote(accepted_table)}" - )).scalar_one() == 65_504 - assert connection.execute(text( - "SELECT COUNT(*) FROM dataset WHERE dataset_name = :dataset_id" - ), {"dataset_id": rejected_id}).scalar_one() == 0 - assert rejected_table not in inspect_database(engine).get_table_names() - engine.dispose() + assert after_initialize["working_set_binding"] == before["working_set_binding"] + assert after_import["working_set_binding"] == before["working_set_binding"] + assert after_initialize["state"] == before["state"] + assert after_import["state"] == before["state"] -def test_live_dolt_published_storage_and_identifier_evidence() -> None: +@pytest.mark.candidate_evidence +def test_live_dolt_candidate_limit_probe_smoke() -> None: database_url = os.environ.get("OPENSTATSPEC_DOLT_URL") if not database_url: pytest.skip("OPENSTATSPEC_DOLT_URL is not configured") From bd3cb700fc0e402dcdd3949f19a4f1a34f98ca28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 20:17:12 +0300 Subject: [PATCH 06/27] Allow documented compatible-name loss in service tests --- tests/test_sql_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_sql_services.py b/tests/test_sql_services.py index ba2c177..1abbc2f 100755 --- a/tests/test_sql_services.py +++ b/tests/test_sql_services.py @@ -17,7 +17,7 @@ pytestmark = pytest.mark.services _REQUIRED_ENGINE_LOSS = [] -_COMPAT_NAME_LOSS = _REQUIRED_ENGINE_LOSS +_COMPAT_NAME_LOSS = [*_REQUIRED_ENGINE_LOSS, "compatible-variable-names-not-preserved"] @pytest.fixture From a817c17bf2d1bcb392aff7af00ae05b7da98c3fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 20:48:28 +0300 Subject: [PATCH 07/27] Address strict catalog review findings --- src/openstatspec/sql/catalog_verification.py | 256 +++++++++++++++++++ src/openstatspec/sql/wide.py | 48 ++-- tests/test_loss_reports.py | 6 +- tests/test_strict_catalog_review.py | 151 +++++++++++ 4 files changed, 440 insertions(+), 21 deletions(-) create mode 100644 src/openstatspec/sql/catalog_verification.py create mode 100644 tests/test_strict_catalog_review.py diff --git a/src/openstatspec/sql/catalog_verification.py b/src/openstatspec/sql/catalog_verification.py new file mode 100644 index 0000000..e338938 --- /dev/null +++ b/src/openstatspec/sql/catalog_verification.py @@ -0,0 +1,256 @@ +"""Strict ownership and structural verification for OpenStatSpec SQL catalogs.""" + +from __future__ import annotations + +import re +from collections.abc import Iterable, Mapping +from typing import Any + +from sqlalchemy import MetaData, Table, inspect, select + +from ..core import UnsupportedOperationError +from .normative import CATALOG_CONTRACT_ID, CATALOG_SCHEMA_VERSION + + +def _normalized_sql_type(inspector: Any, value: Any) -> str: + compiled = " ".join( + str(value.compile(dialect=inspector.bind.dialect)).strip().upper().split() + ) + if inspector.bind.dialect.name in {"mysql", "mariadb"}: + if compiled in {"BOOL", "BOOLEAN", "TINYINT(1)"}: + return "BOOLEAN/TINYINT(1)" + compiled = re.sub( + r"\b(TINYINT|SMALLINT|MEDIUMINT|INTEGER|INT|BIGINT)\(\d+\)", + r"\1", + compiled, + ) + return compiled + + +def _normalized_default(value: Any) -> str | None: + if value is None: + return None + result = " ".join(str(value).strip().split()) + while result.startswith("(") and result.endswith(")"): + result = result[1:-1].strip() + return result + + +def _expected_unique_constraints(table: Table) -> set[tuple[str, ...]]: + return { + tuple(column.name for column in constraint.columns) + for constraint in table.constraints + if getattr(constraint, "__visit_name__", "") == "unique_constraint" + } + + +def _actual_unique_constraints(inspector: Any, table_name: str) -> set[tuple[str, ...]]: + constraints = { + tuple(str(name) for name in item.get("column_names") or ()) + for item in inspector.get_unique_constraints(table_name) + } + constraints.update( + tuple(str(name) for name in item.get("column_names") or ()) + for item in inspector.get_indexes(table_name) + if item.get("unique") + ) + constraints.discard(()) + return constraints + + +def _expected_foreign_keys(table: Table) -> set[tuple[Any, ...]]: + return { + ( + tuple(column.name for column in constraint.columns), + next(iter(constraint.elements)).column.table.name, + tuple(element.column.name for element in constraint.elements), + ) + for constraint in table.foreign_key_constraints + } + + +def _actual_foreign_keys(inspector: Any, table_name: str) -> set[tuple[Any, ...]]: + return { + ( + tuple(str(name) for name in item.get("constrained_columns") or ()), + str(item.get("referred_table") or ""), + tuple(str(name) for name in item.get("referred_columns") or ()), + ) + for item in inspector.get_foreign_keys(table_name) + } + + +def _normalized_check_sql(value: Any) -> str: + result = " ".join(str(value).strip().casefold().split()) + result = result.replace("`", "").replace('"', "") + result = re.sub(r"\[([^]]+)\]", r"\1", result) + result = re.sub(r"(? set[str]: + return { + _normalized_check_sql(constraint.sqltext) + for constraint in table.constraints + if getattr(constraint, "__visit_name__", "") + == "table_or_column_check_constraint" + } + + +def _actual_check_constraints(inspector: Any, table_name: str) -> set[str]: + return { + _normalized_check_sql(item.get("sqltext") or "") + for item in inspector.get_check_constraints(table_name) + } + + +def _table_shape_valid( + inspector: Any, + table: Table, + *, + allowed_missing: Iterable[str] = (), +) -> bool: + actual = { + str(column["name"]): column + for column in inspector.get_columns(table.name) + } + expected = {column.name: column for column in table.columns} + missing = set(expected) - set(actual) + if set(actual) - set(expected) or missing - set(allowed_missing): + return False + for name in set(actual) & set(expected): + expected_column = expected[name] + actual_column = actual[name] + if ( + _normalized_sql_type(inspector, expected_column.type) + != _normalized_sql_type(inspector, actual_column["type"]) + or bool(actual_column.get("nullable")) != bool(expected_column.nullable) + ): + return False + expected_default = _normalized_default( + expected_column.server_default.arg + if expected_column.server_default is not None else None + ) + if _normalized_default(actual_column.get("default")) != expected_default: + return False + if actual_column.get("identity") is not None or actual_column.get("computed") is not None: + return False + expected_pk = tuple(column.name for column in table.primary_key.columns) + actual_pk = tuple( + str(name) + for name in inspector.get_pk_constraint(table.name).get("constrained_columns") or () + ) + return ( + actual_pk == expected_pk + and _actual_unique_constraints(inspector, table.name) + == _expected_unique_constraints(table) + and _actual_foreign_keys(inspector, table.name) + == _expected_foreign_keys(table) + and _actual_check_constraints(inspector, table.name) + == _expected_check_constraints(table) + ) + + +def _reject(relations: Iterable[str] = ()) -> None: + suffix = f": {', '.join(sorted(relations))}" if relations else "" + raise UnsupportedOperationError( + "The selected database catalog contains foreign, obsolete, or " + f"structurally incompatible relations{suffix}. Remove them manually " + "before continuing." + ) + + +def verify_catalog_relations( + connection: Any, + normative: Any, + *, + allowed_migrations: Mapping[str, set[str]] | None = None, +) -> None: + """Accept only exact normative and validated optional OpenStatSpec profiles.""" + inspector = inspect(connection) + existing_tables = set(inspector.get_table_names()) + existing_views = set(inspector.get_view_names()) + normative_tables = {table.name: table for table in normative.all()} + if not set(normative_tables) <= existing_tables: + _reject(set(normative_tables) - existing_tables) + if any( + not _table_shape_valid(inspector, table) + for table in normative_tables.values() + ): + _reject(normative_tables) + + identities = connection.execute(select(normative.catalog_identity)).mappings().all() + if len(identities) != 1 or ( + identities[0]["catalog_identity_key"] != 1 + or identities[0]["contract_id"] != CATALOG_CONTRACT_ID + or identities[0]["schema_version"] != CATALOG_SCHEMA_VERSION + ): + _reject({normative.catalog_identity.name}) + + owned_tables = set(normative_tables) + owned_views: set[str] = set() + owned_tables.update( + str(name) + for name in connection.execute( + select(normative.dataset.c.physical_table_name) + ).scalars() + if name + ) + + from .workflow import PROFILE_ID, PROFILE_SCHEMA_VERSION, workflow_catalog + + workflow = workflow_catalog(MetaData()) + workflow_tables = {table.name: table for table in workflow.all()} + workflow_present = set(workflow_tables) & existing_tables + if workflow_present: + if set(workflow_tables) - existing_tables or any( + not _table_shape_valid(inspector, table) + for table in workflow_tables.values() + ): + _reject(workflow_present) + identity_rows = connection.execute( + select(workflow.transformation_profile_identity) + ).mappings().all() + if len(identity_rows) != 1 or ( + identity_rows[0]["profile_identity_key"] != 1 + or identity_rows[0]["contract_id"] != PROFILE_ID + or identity_rows[0]["schema_version"] != PROFILE_SCHEMA_VERSION + or identity_rows[0]["core_contract_id"] != CATALOG_CONTRACT_ID + ): + _reject({workflow.transformation_profile_identity.name}) + owned_tables.update(workflow_tables) + for row in connection.execute(select( + workflow.derived_dataset.c.physical_relation_name, + workflow.derived_dataset.c.output_mode, + )).mappings(): + name = str(row["physical_relation_name"]) + if row["output_mode"] == "view": + owned_views.add(name) + else: + owned_tables.add(name) + + from .inplace_transform import apply_audit_catalog + + audit = apply_audit_catalog(MetaData()) + if audit.name in existing_tables: + if not _table_shape_valid( + inspector, + audit, + allowed_missing=(allowed_migrations or {}).get(audit.name, set()), + ): + _reject({audit.name}) + owned_tables.add(audit.name) + + foreign = (existing_tables - owned_tables) | (existing_views - owned_views) + missing = (owned_tables - existing_tables) | (owned_views - existing_views) + collisions = (owned_tables & existing_views) | (owned_views & existing_tables) + if foreign or missing or collisions: + _reject(foreign | missing | collisions) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index d9e8b2d..779c7a7 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -31,6 +31,7 @@ record_operation as record_normative_operation, store_imported_dataset as store_normative_dataset, ) +from .catalog_verification import verify_catalog_relations _IDENTIFIER = re.compile(r"[^a-zA-Z0-9_]+") @@ -753,7 +754,7 @@ def create_wide_dataset( operation_details: Mapping[str, Any] | None = None, dolt_conformance_source: Any | None = None, ) -> dict[str, Any]: - del source_table_name, source_created_at, source_modified_at, operation_details + del source_table_name, source_created_at, source_modified_at validate_connection_url(database_url) profile, _active_connection = ( effective_profile(database_url) @@ -874,10 +875,22 @@ def create_wide_dataset( record_normative_fidelity_events( connection, normative, operation_id=operation_id, dataset_id=normative_dataset_id, direction="import", - events=fidelity_events, + events=( + *fidelity_events, + *(({ + "code": "operation-engine-identity", + "detail": "Import engine identity recorded for audit.", + "severity": "info", + "source_item": source_name, + "details": dict(operation_details), + },) if operation_details else ()), + ), ) if materialized: - connection.execute(insert(data_table), materialized) + for batch in _bounded_batches( + materialized, variables, profile.max_statement_bytes, + ): + connection.execute(insert(data_table), batch) finish_normative_operation( connection, normative, operation_id=operation_id, status="succeeded", @@ -895,8 +908,15 @@ def create_wide_dataset( delete_normative_dataset( cleanup, normative, normative_dataset_id, ) + table_is_owned = cleanup.execute( + select(normative.dataset.c.dataset_id).where( + normative.dataset.c.physical_table_name == data_table.name + ) + ).first() if ( data_table_created + and profile.name != "postgresql" + and table_is_owned is None and inspect(cleanup).has_table(data_table.name) ): data_table.drop(cleanup, checkfirst=True) @@ -1148,6 +1168,7 @@ def read_wide_dataset( ) for row in response_sets }, ensure_ascii=False) + preflight(profile, variables, rows=rows) return dataset, variables, rows @@ -1168,26 +1189,12 @@ def require_verified_catalog( *, allowed_migrations: Mapping[str, set[str]] | None = None, ) -> None: - """Verify that the namespace contains only the normative catalog and owned data.""" + """Verify strict core shape, optional profiles, and owned physical relations.""" normative = normative_catalog(MetaData()) _verify_normative_catalog(connection, normative) - expected = {table.name for table in normative.all()} - expected.update( - str(name) for name in connection.execute( - select(normative.dataset.c.physical_table_name) - ).scalars() + verify_catalog_relations( + connection, normative, allowed_migrations=allowed_migrations, ) - expected.update((allowed_migrations or {}).keys()) - expected.add("transformation_apply") - inspector = inspect(connection) - unknown = set(inspector.get_table_names()) - expected - views = set(inspector.get_view_names()) - if unknown or views: - relations = ", ".join(sorted(unknown | views)) - raise UnsupportedOperationError( - "The selected database catalog contains foreign or obsolete " - f"relations: {relations}. Remove them manually before continuing." - ) def _resolve_normative_dataset( @@ -1304,6 +1311,7 @@ def read_fidelity_events( events = connection.execute( select(normative.fidelity_event) .where(normative.fidelity_event.c.dataset_id == dataset["dataset_id"]) + .where(normative.fidelity_event.c.severity != "info") .order_by(normative.fidelity_event.c.event_code) ).mappings().all() result = [] diff --git a/tests/test_loss_reports.py b/tests/test_loss_reports.py index 427def9..431323e 100755 --- a/tests/test_loss_reports.py +++ b/tests/test_loss_reports.py @@ -25,7 +25,11 @@ def test_persisted_import_fidelity_events_require_consent_after_reopen(tmp_path) imported = openstatspec.import_sav(source, database_url=database, dataset_id="persisted") assert {diagnostic.code for diagnostic in imported.diagnostics} == set(_REQUIRED_ENGINE_LOSS) connection = sqlite3.connect(database_path) - assert {row[0] for row in connection.execute("select event_code from fidelity_event")} == set(_REQUIRED_ENGINE_LOSS) + assert { + row[0] for row in connection.execute( + "select event_code from fidelity_event where severity != 'info'" + ) + } == set(_REQUIRED_ENGINE_LOSS) assert connection.execute( "select operation_kind, status from operation order by started_at limit 1" ).fetchone() == ("import", "succeeded") diff --git a/tests/test_strict_catalog_review.py b/tests/test_strict_catalog_review.py new file mode 100644 index 0000000..b5b4e25 --- /dev/null +++ b/tests/test_strict_catalog_review.py @@ -0,0 +1,151 @@ +"""Regression coverage for strict-catalog review findings.""" + +import json +import sqlite3 + +import pytest +from sqlalchemy import MetaData, create_engine + +import openstatspec +from openstatspec.core import UnsupportedOperationError +from openstatspec.sql import wide +from openstatspec.sql.profiles import SQLITE, TargetCapabilityExceededError +from openstatspec.sql.workflow import create_workflow_catalog, workflow_catalog + + +def _string_variables(): + return [{ + "ordinal": 1, + "source_name": "name", + "physical_name": "name", + "storage_kind": "string", + "string_width": 8, + "label": "", + "format": "A8", + "measure": "nominal", + "alignment": "left", + "display_width": 8, + "value_labels": "{}", + "missing_ranges": "[]", + }] + + +def _numeric_variables(): + return [{ + "ordinal": 1, + "source_name": "score", + "physical_name": "score", + "storage_kind": "numeric", + "string_width": None, + "label": "", + "format": "F8.2", + "measure": "scale", + "alignment": "right", + "display_width": 8, + "value_labels": "{}", + "missing_ranges": "[]", + }] + + +def _create(database, dataset_id, *, operation_details=None): + return wide.create_wide_dataset( + database_url=database, + dataset_id=dataset_id, + source_name=f"{dataset_id}.sav", + source_format="SAV", + rows=[{"name": "ok"}], + variables=_string_variables(), + operation_details=operation_details, + ) + + +def test_existing_normative_shape_drift_is_rejected(tmp_path): + path = tmp_path / "shape-drift.sqlite" + database = f"sqlite:///{path}" + openstatspec.initialize_catalog(database_url=database) + connection = sqlite3.connect(path) + connection.execute( + "alter table variable rename column display_alignment " + "to incompatible_display_alignment" + ) + connection.commit() + connection.close() + + with pytest.raises(UnsupportedOperationError, match="structurally incompatible"): + openstatspec.initialize_catalog(database_url=database) + + +def test_verified_workflow_profile_remains_catalog_owned(tmp_path): + path = tmp_path / "workflow-owned.sqlite" + database = f"sqlite:///{path}" + openstatspec.initialize_catalog(database_url=database) + _create(database, "first") + engine = create_engine(database) + with engine.begin() as connection: + create_workflow_catalog(connection, workflow_catalog(MetaData())) + + imported = _create(database, "second") + + assert imported["case_count"] == 1 + + +def test_engine_identity_is_persisted_as_non_loss_audit_metadata(tmp_path): + path = tmp_path / "engine-audit.sqlite" + database = f"sqlite:///{path}" + engine_identity = {"name": "pyspssio", "commit": "abc123"} + + imported = _create( + database, + "audited", + operation_details={"engine": engine_identity}, + ) + + connection = sqlite3.connect(path) + row = connection.execute( + "select severity, event_code, detail_json from fidelity_event " + "where operation_id = ?", + (imported["operation_id"],), + ).fetchone() + connection.close() + assert row[:2] == ("info", "operation-engine-identity") + assert json.loads(row[2])["engine"] == engine_identity + assert wide.read_fidelity_events( + database_url=database, dataset_id="audited", + ) == () + + +def test_database_rows_are_preflighted_before_export_descriptor(tmp_path): + path = tmp_path / "row-drift.sqlite" + database = f"sqlite:///{path}" + wide.create_wide_dataset( + database_url=database, + dataset_id="numeric", + source_name="numeric.sav", + source_format="SAV", + rows=[{"score": 1.5}], + variables=_numeric_variables(), + ) + connection = sqlite3.connect(path) + connection.execute("update data_numeric set score = 'not-a-number'") + connection.commit() + connection.close() + + with pytest.raises(TargetCapabilityExceededError) as caught: + wide.read_wide_dataset(database_url=database, dataset_id="numeric") + assert caught.value.details["reason"] == "numeric_value_type" + + +def test_import_uses_bounded_statement_batches(tmp_path, monkeypatch): + path = tmp_path / "batches.sqlite" + database = f"sqlite:///{path}" + real_bounded_batches = wide._bounded_batches + calls = [] + + def observe(rows, variables, maximum): + calls.append((len(rows), maximum)) + yield from real_bounded_batches(rows, variables, maximum) + + monkeypatch.setattr(wide, "_bounded_batches", observe) + _create(database, "batched") + + assert calls == [(1, SQLITE.max_statement_bytes)] From 600ac985ef988b30ca785fff5e871e28520fd04a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 21:13:11 +0300 Subject: [PATCH 08/27] Harden persistent and bound catalog operations --- src/openstatspec/sql/database_urls.py | 22 ++++++ src/openstatspec/sql/wide.py | 60 +++++++++++---- tests/test_atomic_import.py | 6 ++ tests/test_catalog_persistence_review.py | 98 ++++++++++++++++++++++++ tests/test_loss_reports.py | 2 +- 5 files changed, 173 insertions(+), 15 deletions(-) create mode 100644 src/openstatspec/sql/database_urls.py create mode 100644 tests/test_catalog_persistence_review.py diff --git a/src/openstatspec/sql/database_urls.py b/src/openstatspec/sql/database_urls.py new file mode 100644 index 0000000..467a915 --- /dev/null +++ b/src/openstatspec/sql/database_urls.py @@ -0,0 +1,22 @@ +"""Database URL invariants shared by persistent catalog operations.""" + +from sqlalchemy.engine import make_url + +from ..core import UnsupportedOperationError + + +def require_persistent_database_url(database_url: str) -> None: + """Reject SQLite URLs whose catalog disappears with a connection or engine.""" + parsed_url = make_url(database_url) + if parsed_url.get_backend_name() != "sqlite": + return + database = parsed_url.database or "" + mode = str(parsed_url.query.get("mode", "")).lower() + if ( + database in {"", ":memory:"} + or database.lower() == "file::memory:" + or mode == "memory" + ): + raise UnsupportedOperationError( + "OpenStatSpec catalogs require a persistent SQLite database URL." + ) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 779c7a7..4ae78cf 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -32,6 +32,7 @@ store_imported_dataset as store_normative_dataset, ) from .catalog_verification import verify_catalog_relations +from .database_urls import require_persistent_database_url _IDENTIFIER = re.compile(r"[^a-zA-Z0-9_]+") @@ -669,12 +670,20 @@ def initialize_wide_catalog( ) -> dict[str, Any]: """Initialize or verify the singular normative OpenStatSpec catalog.""" validate_connection_url(database_url) - profile, _active = effective_profile( + require_persistent_database_url(database_url) + profile, active = effective_profile( database_url, dolt_conformance_source=dolt_conformance_source, ) engine = create_engine(database_url) normative = normative_catalog(MetaData()) - with engine.begin() as connection: + audit_relations = { + normative.fidelity_event.name, + normative.operation.name, + } + with _bound_catalog_transaction( + engine=engine, profile_name=profile.name, active=active, + audit_relations=audit_relations, phase="catalog initialization", + ) as connection: inspector = inspect(connection) views = set(inspector.get_view_names()) if views and not inspector.has_table(normative.catalog_identity.name): @@ -755,8 +764,9 @@ def create_wide_dataset( dolt_conformance_source: Any | None = None, ) -> dict[str, Any]: del source_table_name, source_created_at, source_modified_at + require_persistent_database_url(database_url) validate_connection_url(database_url) - profile, _active_connection = ( + profile, active_connection = ( effective_profile(database_url) if dolt_conformance_source is None else effective_profile( @@ -765,7 +775,14 @@ def create_wide_dataset( ) engine = create_engine(database_url) normative = normative_catalog(MetaData()) - with engine.begin() as catalog_connection: + audit_relations = { + normative.fidelity_event.name, + normative.operation.name, + } + with _bound_catalog_transaction( + engine=engine, profile_name=profile.name, active=active_connection, + audit_relations=audit_relations, phase="catalog initialization", + ) as catalog_connection: catalog_existed = inspect(catalog_connection).has_table( normative.catalog_identity.name ) @@ -827,7 +844,10 @@ def create_wide_dataset( with engine.begin() as setup: _verify_normative_catalog(setup, normative) namespace_owned = True - with engine.begin() as connection: + with _bound_catalog_transaction( + engine=engine, profile_name=profile.name, active=active_connection, + audit_relations=audit_relations, phase="import", + ) as connection: if connection.execute( select(normative.dataset.c.dataset_id).where(or_( normative.dataset.c.dataset_name == dataset_id, @@ -1333,7 +1353,7 @@ def record_export_operation( dolt_conformance_source: Any | None = None, ) -> str: """Persist a completed export only in the normative audit catalog.""" - del destination, operation_details + operation_details = dict(operation_details or {}) engine = create_engine(database_url) effective_profile( database_url, dolt_conformance_source=dolt_conformance_source, @@ -1354,14 +1374,26 @@ def record_export_operation( record_normative_fidelity_events( connection, normative, operation_id=operation_id, dataset_id=str(dataset["dataset_id"]), direction="export", - events=({ - **event, - "severity": event.get("severity", "warning"), - "details": { - **event.get("details", {}), - "accepted_by_user": True, - }, - } for event in events), + events=( + *( + { + **event, + "severity": event.get("severity", "warning"), + "details": { + **event.get("details", {}), + "accepted_by_user": True, + }, + } + for event in events + ), + *(({ + "code": "operation-engine-identity", + "detail": "Export engine identity recorded for audit.", + "severity": "info", + "source_item": destination, + "details": operation_details, + },) if operation_details else ()), + ), ) return operation_id diff --git a/tests/test_atomic_import.py b/tests/test_atomic_import.py index 4a25a1d..dc4fbcc 100755 --- a/tests/test_atomic_import.py +++ b/tests/test_atomic_import.py @@ -284,6 +284,9 @@ def test_dolt_nonfinite_preflight_creates_no_dataset_or_physical_table( database_path = tmp_path / "dolt-nonfinite.sqlite" database = f"sqlite:///{database_path}" monkeypatch.setattr(wide, "effective_profile", lambda _url: (DOLT, {})) + monkeypatch.setattr( + wide, "_capture_dolt_state", lambda *_args, **_kwargs: None, + ) variables = [{ "ordinal": 1, "source_name": "value", "physical_name": "value", "storage_kind": "numeric", "string_width": None, "label": "", @@ -320,6 +323,9 @@ def test_empty_namespace_dolt_width_failure_initializes_identity_and_one_audit( database_path = tmp_path / "dolt-preflight.sqlite" database = f"sqlite:///{database_path}" monkeypatch.setattr(wide, "effective_profile", lambda _url: (DOLT, {})) + monkeypatch.setattr( + wide, "_capture_dolt_state", lambda *_args, **_kwargs: None, + ) with pytest.raises(Exception, match="Target capability exceeded"): create_wide_dataset( diff --git a/tests/test_catalog_persistence_review.py b/tests/test_catalog_persistence_review.py new file mode 100644 index 0000000..2287da5 --- /dev/null +++ b/tests/test_catalog_persistence_review.py @@ -0,0 +1,98 @@ +"""Regression tests for persistent URLs and operation-bound audit writes.""" + +import json +import sqlite3 +from contextlib import contextmanager + +import pytest + +from openstatspec.core import UnsupportedOperationError +from openstatspec.sql import wide + + +def _variables(): + return [{ + "ordinal": 1, + "source_name": "name", + "physical_name": "name", + "storage_kind": "string", + "string_width": 8, + "label": "", + "format": "A8", + "measure": "nominal", + "alignment": "left", + "display_width": 8, + "value_labels": "{}", + "missing_ranges": "[]", + }] + + +def _create(database_url, dataset_id="sample"): + return wide.create_wide_dataset( + database_url=database_url, + dataset_id=dataset_id, + source_name=f"{dataset_id}.sav", + source_format="SAV", + rows=[{"name": "ok"}], + variables=_variables(), + ) + + +@pytest.mark.parametrize( + "database_url", + [ + "sqlite://", + "sqlite:///:memory:", + "sqlite:///file:shared-memory?mode=memory&uri=true", + ], +) +def test_import_rejects_ephemeral_sqlite_catalogs(database_url): + with pytest.raises(UnsupportedOperationError, match="persistent SQLite"): + _create(database_url) + + +def test_import_routes_catalog_and_dataset_mutations_through_binding_guard( + tmp_path, monkeypatch, +): + database_url = f"sqlite:///{tmp_path / 'bound.sqlite'}" + real_bound_transaction = wide._bound_catalog_transaction + phases = [] + + @contextmanager + def observe(**kwargs): + phases.append(kwargs["phase"]) + with real_bound_transaction(**kwargs) as connection: + yield connection + + monkeypatch.setattr(wide, "_bound_catalog_transaction", observe) + _create(database_url) + + assert phases == ["catalog initialization", "import"] + + +def test_export_engine_identity_is_persisted_as_non_loss_audit_metadata(tmp_path): + path = tmp_path / "export-audit.sqlite" + database_url = f"sqlite:///{path}" + _create(database_url) + engine_identity = {"name": "pyspssio", "commit": "export-commit"} + + operation_id = wide.record_export_operation( + database_url=database_url, + dataset_id="sample", + destination="out.sav", + allowed_fidelity_events=(), + operation_details={"engine": engine_identity}, + ) + + connection = sqlite3.connect(path) + row = connection.execute( + "select severity, event_code, source_item, detail_json " + "from fidelity_event where operation_id = ?", + (operation_id,), + ).fetchone() + connection.close() + assert row[:3] == ("info", "operation-engine-identity", "out.sav") + assert json.loads(row[3])["engine"] == engine_identity + assert wide.read_fidelity_events( + database_url=database_url, dataset_id="sample", + ) == () diff --git a/tests/test_loss_reports.py b/tests/test_loss_reports.py index 431323e..d30b32c 100755 --- a/tests/test_loss_reports.py +++ b/tests/test_loss_reports.py @@ -55,7 +55,7 @@ def test_loss_allowed_export_persists_accepted_diagnostics(tmp_path) -> None: result = openstatspec.export_sav(database_url=database, dataset_id="accepted", destination=destination, allow_loss=_REQUIRED_ENGINE_LOSS) connection = sqlite3.connect(database_path) - rows = connection.execute("select direction, severity, event_code, detail_json from fidelity_event where operation_id = ? order by event_code", (result["operation_id"],)).fetchall() + rows = connection.execute("select direction, severity, event_code, detail_json from fidelity_event where operation_id = ? and severity != 'info' order by event_code", (result["operation_id"],)).fetchall() assert [(row[0], row[1], row[2]) for row in rows] == [("export", "warning", code) for code in _REQUIRED_ENGINE_LOSS] assert all('"accepted_by_user": true' in row[3] for row in rows) From 7c878c50011b8227f33925155d4ea24b57dfb30b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 21:37:29 +0300 Subject: [PATCH 09/27] Bind export audits and fail closed on catalog init --- src/openstatspec/sql/wide.py | 93 ++++++++++++++---------- tests/test_catalog_persistence_review.py | 82 +++++++++++++++++++++ 2 files changed, 138 insertions(+), 37 deletions(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 4ae78cf..c69d103 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -685,18 +685,23 @@ def initialize_wide_catalog( audit_relations=audit_relations, phase="catalog initialization", ) as connection: inspector = inspect(connection) + tables = set(inspector.get_table_names()) views = set(inspector.get_view_names()) - if views and not inspector.has_table(normative.catalog_identity.name): - raise UnsupportedOperationError( - "The selected database catalog is foreign; initialization is not permitted." - ) - try: - create_normative_catalog(connection, normative) - except RuntimeError as error: - raise UnsupportedOperationError( - "The selected database catalog is foreign or incompatible; " - "initialization is not permitted." - ) from error + if tables or views: + if normative.catalog_identity.name not in tables: + raise UnsupportedOperationError( + "The selected database catalog is foreign; " + "initialization is not permitted." + ) + require_verified_catalog(connection) + else: + try: + create_normative_catalog(connection, normative) + except RuntimeError as error: + raise UnsupportedOperationError( + "The selected database catalog is foreign or incompatible; " + "initialization is not permitted." + ) from error require_verified_catalog(connection) return {"profile": profile.name, "catalog": "verified"} @@ -1345,6 +1350,26 @@ def read_fidelity_events( return tuple(result) +@contextmanager +def _bound_export_audit_transaction( + *, database_url: str, dolt_conformance_source: Any | None, phase: str, +): + """Open one verified transaction bound to the effective Dolt working set.""" + validate_connection_url(database_url) + require_persistent_database_url(database_url) + profile, active = effective_profile( + database_url, dolt_conformance_source=dolt_conformance_source, + ) + engine = create_engine(database_url) + normative = normative_catalog(MetaData()) + with _bound_catalog_transaction( + engine=engine, profile_name=profile.name, active=active, + audit_relations={normative.operation.name, normative.fidelity_event.name}, + phase=phase, + ) as connection: + yield connection, normative + + def record_export_operation( *, database_url: str, dataset_id: str, destination: str, allowed_fidelity_events: Iterable[Mapping[str, Any]], @@ -1354,14 +1379,12 @@ def record_export_operation( ) -> str: """Persist a completed export only in the normative audit catalog.""" operation_details = dict(operation_details or {}) - engine = create_engine(database_url) - effective_profile( - database_url, dolt_conformance_source=dolt_conformance_source, - ) - normative = normative_catalog(MetaData()) operation_id = str(uuid4()) events = tuple(allowed_fidelity_events) - with engine.begin() as connection: + with _bound_export_audit_transaction( + database_url=database_url, dolt_conformance_source=dolt_conformance_source, + phase="export audit creation", + ) as (connection, normative): _verify_normative_catalog(connection, normative) dataset = _resolve_normative_dataset(connection, normative, dataset_id) completed_at = datetime.now(UTC).replace(tzinfo=None) @@ -1416,11 +1439,10 @@ def finish_export_operation( dolt_conformance_source: Any | None = None, ) -> None: """Mark a started normative export operation as succeeded.""" - effective_profile( - database_url, dolt_conformance_source=dolt_conformance_source, - ) - normative = normative_catalog(MetaData()) - with create_engine(database_url).begin() as connection: + with _bound_export_audit_transaction( + database_url=database_url, dolt_conformance_source=dolt_conformance_source, + phase="export audit finalization", + ) as (connection, normative): _verify_normative_catalog(connection, normative) row = _export_operation_row(connection, normative, operation_id) if row["status"] != "started": @@ -1465,17 +1487,16 @@ def fail_export_operation( dolt_conformance_source: Any | None = None, ) -> None: """Close a started normative export after filesystem compensation.""" - effective_profile( - database_url, dolt_conformance_source=dolt_conformance_source, - ) - normative = normative_catalog(MetaData()) event = { "code": "export_failed", "detail": "Export publication or finalization failed after audit start.", "severity": "error", "details": dict(failure_details), } - with create_engine(database_url).begin() as connection: + with _bound_export_audit_transaction( + database_url=database_url, dolt_conformance_source=dolt_conformance_source, + phase="export audit failure", + ) as (connection, normative): _verify_normative_catalog(connection, normative) row = _export_operation_row(connection, normative, operation_id) if row["status"] != "started": @@ -1497,11 +1518,10 @@ def record_export_backup_retained( dolt_conformance_source: Any | None = None, ) -> None: """Append a warning to a successfully finalized normative export.""" - effective_profile( - database_url, dolt_conformance_source=dolt_conformance_source, - ) - normative = normative_catalog(MetaData()) - with create_engine(database_url).begin() as connection: + with _bound_export_audit_transaction( + database_url=database_url, dolt_conformance_source=dolt_conformance_source, + phase="export backup-retention audit", + ) as (connection, normative): _verify_normative_catalog(connection, normative) row = _export_operation_row(connection, normative, operation_id) if row["status"] != "succeeded": @@ -1532,10 +1552,6 @@ def record_export_cleanup_failure( dolt_conformance_source: Any | None = None, ) -> str: """Persist terminal cleanup failure in the normative audit catalog.""" - effective_profile( - database_url, dolt_conformance_source=dolt_conformance_source, - ) - normative = normative_catalog(MetaData()) operation_id = operation_id or str(uuid4()) event = { "code": "cleanup_failed", @@ -1551,7 +1567,10 @@ def record_export_cleanup_failure( ), }, } - with create_engine(database_url).begin() as connection: + with _bound_export_audit_transaction( + database_url=database_url, dolt_conformance_source=dolt_conformance_source, + phase="export cleanup-failure audit", + ) as (connection, normative): _verify_normative_catalog(connection, normative) row = connection.execute( select(normative.operation).where( diff --git a/tests/test_catalog_persistence_review.py b/tests/test_catalog_persistence_review.py index 2287da5..e17af47 100644 --- a/tests/test_catalog_persistence_review.py +++ b/tests/test_catalog_persistence_review.py @@ -51,6 +51,27 @@ def test_import_rejects_ephemeral_sqlite_catalogs(database_url): _create(database_url) +def test_initialization_does_not_modify_an_existing_unverified_catalog(tmp_path): + path = tmp_path / "partial.sqlite" + connection = sqlite3.connect(path) + connection.execute("create table dataset_catalog (legacy_name text)") + connection.commit() + connection.close() + + with pytest.raises(UnsupportedOperationError, match="foreign"): + wide.initialize_wide_catalog(database_url=f"sqlite:///{path}") + + connection = sqlite3.connect(path) + tables = { + row[0] + for row in connection.execute( + "select name from sqlite_master where type = 'table'" + ) + } + connection.close() + assert tables == {"dataset_catalog"} + + def test_import_routes_catalog_and_dataset_mutations_through_binding_guard( tmp_path, monkeypatch, ): @@ -70,6 +91,67 @@ def observe(**kwargs): assert phases == ["catalog initialization", "import"] +def test_export_audit_mutations_route_through_binding_guard(tmp_path, monkeypatch): + database_url = f"sqlite:///{tmp_path / 'bound-export.sqlite'}" + _create(database_url) + real_bound_transaction = wide._bound_catalog_transaction + phases = [] + + @contextmanager + def observe(**kwargs): + phases.append(kwargs["phase"]) + with real_bound_transaction(**kwargs) as connection: + yield connection + + monkeypatch.setattr(wide, "_bound_catalog_transaction", observe) + failed_id = wide.record_export_operation( + database_url=database_url, + dataset_id="sample", + destination="failed.sav", + allowed_fidelity_events=(), + terminal=False, + ) + wide.fail_export_operation( + database_url=database_url, + operation_id=failed_id, + failure_details={"reason": "test"}, + ) + succeeded_id = wide.record_export_operation( + database_url=database_url, + dataset_id="sample", + destination="succeeded.sav", + allowed_fidelity_events=(), + terminal=False, + ) + wide.finish_export_operation( + database_url=database_url, operation_id=succeeded_id, + ) + wide.record_export_backup_retained( + database_url=database_url, + operation_id=succeeded_id, + destination="succeeded.sav", + backup="succeeded.sav.backup", + cleanup_error=RuntimeError("test"), + ) + wide.record_export_cleanup_failure( + database_url=database_url, + destination="cleanup.sav", + original_error=RuntimeError("export"), + cleanup_error=RuntimeError("cleanup"), + residual_object_inventory={}, + deterministic_recovery_evidence={}, + ) + + assert phases == [ + "export audit creation", + "export audit failure", + "export audit creation", + "export audit finalization", + "export backup-retention audit", + "export cleanup-failure audit", + ] + + def test_export_engine_identity_is_persisted_as_non_loss_audit_metadata(tmp_path): path = tmp_path / "export-audit.sqlite" database_url = f"sqlite:///{path}" From aefd0c8ae9005a563ddc8193f4497ee62381513d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 21:52:45 +0300 Subject: [PATCH 10/27] Enforce strict catalog verification on reads --- src/openstatspec/sql/wide.py | 7 ++++--- tests/test_strict_catalog_review.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index c69d103..4265ee2 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -1000,7 +1000,7 @@ def read_wide_dataset( engine = create_engine(database_url) normative = normative_catalog(MetaData()) with engine.connect() as connection: - _verify_normative_catalog(connection, normative) + require_verified_catalog(connection) dataset_row = _resolve_normative_dataset(connection, normative, dataset_id) core_id = str(dataset_row["dataset_id"]) data_table = Table( @@ -1331,7 +1331,7 @@ def read_fidelity_events( engine = create_engine(database_url) normative = normative_catalog(MetaData()) with engine.connect() as connection: - _verify_normative_catalog(connection, normative) + require_verified_catalog(connection) dataset = _resolve_normative_dataset(connection, normative, dataset_id) events = connection.execute( select(normative.fidelity_event) @@ -1367,6 +1367,7 @@ def _bound_export_audit_transaction( audit_relations={normative.operation.name, normative.fidelity_event.name}, phase=phase, ) as connection: + require_verified_catalog(connection) yield connection, normative @@ -1464,7 +1465,7 @@ def read_export_operation_state( ) normative = normative_catalog(MetaData()) with create_engine(database_url).connect() as connection: - _verify_normative_catalog(connection, normative) + require_verified_catalog(connection) row = _export_operation_row(connection, normative, operation_id) classification = { "started": "running", diff --git a/tests/test_strict_catalog_review.py b/tests/test_strict_catalog_review.py index b5b4e25..21f0f6e 100644 --- a/tests/test_strict_catalog_review.py +++ b/tests/test_strict_catalog_review.py @@ -75,6 +75,26 @@ def test_existing_normative_shape_drift_is_rejected(tmp_path): openstatspec.initialize_catalog(database_url=database) +def test_read_and_export_reject_obsolete_catalog_relations(tmp_path): + path = tmp_path / "obsolete-read.sqlite" + database = f"sqlite:///{path}" + _create(database, "sample") + connection = sqlite3.connect(path) + connection.execute("create table dataset_catalog (dataset_id text primary key)") + connection.commit() + connection.close() + + with pytest.raises(UnsupportedOperationError, match="obsolete"): + wide.read_wide_dataset(database_url=database, dataset_id="sample") + with pytest.raises(UnsupportedOperationError, match="obsolete"): + wide.record_export_operation( + database_url=database, + dataset_id="sample", + destination="sample.sav", + allowed_fidelity_events=(), + ) + + def test_verified_workflow_profile_remains_catalog_owned(tmp_path): path = tmp_path / "workflow-owned.sqlite" database = f"sqlite:///{path}" From 1bea2777d62fc0eb4c0922ab470fd5a67fa8e7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 22:34:35 +0300 Subject: [PATCH 11/27] Keep Dolt checks and export events transaction-bound --- src/openstatspec/sql/wide.py | 57 +++++++++-- tests/test_review_transaction_boundary.py | 117 ++++++++++++++++++++++ 2 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 tests/test_review_transaction_boundary.py diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 4265ee2..ead39e4 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -579,6 +579,11 @@ def _bound_catalog_transaction( try: with connection.begin(): yield connection + after = _capture_dolt_state( + connection, profile_name=profile_name, + audit_relations=audit_relations, + ) + _require_dolt_success_identity(before, after, phase=phase) except Exception: after = _capture_dolt_state( connection, profile_name=profile_name, @@ -586,11 +591,6 @@ def _bound_catalog_transaction( ) _dolt_failure_boundary_evidence(before, after) raise - after = _capture_dolt_state( - connection, profile_name=profile_name, - audit_relations=audit_relations, - ) - _require_dolt_success_identity(before, after, phase=phase) def dolt_state_snapshot( @@ -1417,6 +1417,13 @@ def record_export_operation( "source_item": destination, "details": operation_details, },) if operation_details else ()), + { + "code": "export-operation-dataset-binding", + "detail": "Export operation dataset identity recorded for audit.", + "severity": "info", + "source_item": destination, + "details": {}, + }, ), ) return operation_id @@ -1435,6 +1442,30 @@ def _export_operation_row( return row +def _export_operation_dataset_id( + connection: Any, normative: Any, operation_id: str, +) -> str: + dataset_ids = set(connection.execute( + select(normative.fidelity_event.c.dataset_id) + .where(normative.fidelity_event.c.operation_id == operation_id) + .where(normative.fidelity_event.c.dataset_id.is_not(None)) + ).scalars()) + if len(dataset_ids) != 1: + raise UnsupportedOperationError( + "The export operation is not bound to exactly one dataset." + ) + dataset_id = str(dataset_ids.pop()) + if connection.execute( + select(normative.dataset.c.dataset_id).where( + normative.dataset.c.dataset_id == dataset_id + ) + ).scalar_one_or_none() is None: + raise UnsupportedOperationError( + "The export operation dataset no longer exists." + ) + return dataset_id + + def finish_export_operation( *, database_url: str, operation_id: str, dolt_conformance_source: Any | None = None, @@ -1504,12 +1535,15 @@ def fail_export_operation( raise UnsupportedOperationError( "Only a started export operation can be failed." ) + dataset_id = _export_operation_dataset_id( + connection, normative, operation_id, + ) finish_normative_operation( connection, normative, operation_id=operation_id, status="failed", ) record_normative_fidelity_events( connection, normative, operation_id=operation_id, - dataset_id=None, direction="export", events=(event,), + dataset_id=dataset_id, direction="export", events=(event,), ) @@ -1529,9 +1563,12 @@ def record_export_backup_retained( raise UnsupportedOperationError( "A retained backup warning requires a succeeded export." ) + dataset_id = _export_operation_dataset_id( + connection, normative, operation_id, + ) record_normative_fidelity_events( connection, normative, operation_id=operation_id, - dataset_id=None, direction="export", events=({ + dataset_id=dataset_id, direction="export", events=({ "code": "backup_retained", "detail": "A successful export retained its durable prior-file backup.", "severity": "warning", @@ -1578,6 +1615,7 @@ def record_export_cleanup_failure( normative.operation.c.operation_id == operation_id ) ).mappings().one_or_none() + dataset_id = None if row is None: failed_at = datetime.now(UTC).replace(tzinfo=None) record_normative_operation( @@ -1590,12 +1628,15 @@ def record_export_cleanup_failure( raise UnsupportedOperationError( "Existing export operation cannot transition to cleanup failure." ) + dataset_id = _export_operation_dataset_id( + connection, normative, operation_id, + ) finish_normative_operation( connection, normative, operation_id=operation_id, status="failed", ) record_normative_fidelity_events( connection, normative, operation_id=operation_id, - dataset_id=None, direction="export", events=(event,), + dataset_id=dataset_id, direction="export", events=(event,), ) return operation_id diff --git a/tests/test_review_transaction_boundary.py b/tests/test_review_transaction_boundary.py new file mode 100644 index 0000000..22f33df --- /dev/null +++ b/tests/test_review_transaction_boundary.py @@ -0,0 +1,117 @@ +"""Regression tests for bound commit checks and export dataset linkage.""" + +import sqlite3 + +from sqlalchemy import create_engine + +from openstatspec.sql import wide + + +def _variables(): + return [{ + "ordinal": 1, + "source_name": "name", + "physical_name": "name", + "storage_kind": "string", + "string_width": 8, + "label": "", + "format": "A8", + "measure": "nominal", + "alignment": "left", + "display_width": 8, + "value_labels": "{}", + "missing_ranges": "[]", + }] + + +def test_dolt_completion_identity_is_checked_before_transaction_commit( + monkeypatch, +): + engine = create_engine("sqlite://") + state = { + "database": "catalog", + "active_branch": "main", + "head": "abc123", + } + transaction_states = [] + + def capture(connection, **_kwargs): + transaction_states.append(connection.in_transaction()) + return state + + monkeypatch.setattr(wide, "_capture_dolt_state", capture) + with wide._bound_catalog_transaction( + engine=engine, + profile_name="dolt", + active={ + "working_set_binding": { + "database": "catalog", + "active_branch": "main", + }, + }, + audit_relations=set(), + phase="test", + ): + pass + + assert transaction_states == [False, True] + + +def test_export_lifecycle_events_remain_linked_to_the_dataset(tmp_path): + path = tmp_path / "export-events.sqlite" + database_url = f"sqlite:///{path}" + wide.create_wide_dataset( + database_url=database_url, + dataset_id="sample", + source_name="sample.sav", + source_format="SAV", + rows=[{"name": "ok"}], + variables=_variables(), + ) + + failed_id = wide.record_export_operation( + database_url=database_url, + dataset_id="sample", + destination="failed.sav", + allowed_fidelity_events=(), + terminal=False, + ) + wide.fail_export_operation( + database_url=database_url, + operation_id=failed_id, + failure_details={"reason": "test"}, + ) + succeeded_id = wide.record_export_operation( + database_url=database_url, + dataset_id="sample", + destination="succeeded.sav", + allowed_fidelity_events=(), + terminal=False, + ) + wide.finish_export_operation( + database_url=database_url, + operation_id=succeeded_id, + ) + wide.record_export_backup_retained( + database_url=database_url, + operation_id=succeeded_id, + destination="succeeded.sav", + backup="succeeded.sav.backup", + cleanup_error=RuntimeError("test"), + ) + + events = wide.read_fidelity_events( + database_url=database_url, + dataset_id="sample", + ) + assert {event["code"] for event in events} == { + "backup_retained", + "export_failed", + } + connection = sqlite3.connect(path) + assert connection.execute( + "select count(*) from fidelity_event " + "where operation_id in (?, ?) and dataset_id is null", + (failed_id, succeeded_id), + ).fetchone() == (0,) + connection.close() From 7ab2394e913fb35bfbbbec4e5584106f5e36cd9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 22:52:34 +0300 Subject: [PATCH 12/27] Separate export lifecycle diagnostics from loss consent --- src/openstatspec/spss/sav.py | 1 + src/openstatspec/sql/wide.py | 15 +++++++++++---- tests/test_review_transaction_boundary.py | 3 +++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/openstatspec/spss/sav.py b/src/openstatspec/spss/sav.py index 71add95..111d2d3 100644 --- a/src/openstatspec/spss/sav.py +++ b/src/openstatspec/spss/sav.py @@ -684,6 +684,7 @@ def export_sav_dataset( ) persisted_events = read_fidelity_events( database_url=database_url, dataset_id=dataset_id, + direction="import", dolt_conformance_source=dolt_conformance_source, ) if legacy_locale is not None: diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index ead39e4..3dc6785 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -1322,9 +1322,10 @@ def _export_response_set( def read_fidelity_events( *, database_url: str, dataset_id: str, + direction: str | None = None, dolt_conformance_source: Any | None = None, ) -> tuple[dict[str, Any], ...]: - """Read fidelity diagnostics from the normative catalog.""" + """Read fidelity diagnostics, optionally limited to one lifecycle direction.""" effective_profile( database_url, dolt_conformance_source=dolt_conformance_source, ) @@ -1333,12 +1334,18 @@ def read_fidelity_events( with engine.connect() as connection: require_verified_catalog(connection) dataset = _resolve_normative_dataset(connection, normative, dataset_id) - events = connection.execute( + statement = ( select(normative.fidelity_event) .where(normative.fidelity_event.c.dataset_id == dataset["dataset_id"]) .where(normative.fidelity_event.c.severity != "info") - .order_by(normative.fidelity_event.c.event_code) - ).mappings().all() + ) + if direction is not None: + statement = statement.where( + normative.fidelity_event.c.direction == direction + ) + events = connection.execute(statement.order_by( + normative.fidelity_event.c.event_code + )).mappings().all() result = [] for item in events: details = json.loads(item["detail_json"] or "{}") diff --git a/tests/test_review_transaction_boundary.py b/tests/test_review_transaction_boundary.py index 22f33df..c385cef 100644 --- a/tests/test_review_transaction_boundary.py +++ b/tests/test_review_transaction_boundary.py @@ -108,6 +108,9 @@ def test_export_lifecycle_events_remain_linked_to_the_dataset(tmp_path): "backup_retained", "export_failed", } + assert wide.read_fidelity_events( + database_url=database_url, dataset_id="sample", direction="import", + ) == () connection = sqlite3.connect(path) assert connection.execute( "select count(*) from fidelity_event " From 59692c27aa0a6a44e496341e2cbfe23ec7ac4d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 23:16:52 +0300 Subject: [PATCH 13/27] Verify Dolt diffs and workflow triggers --- src/openstatspec/sql/catalog_verification.py | 21 +++++- src/openstatspec/sql/wide.py | 73 +++++++++++++------- tests/test_review_transaction_boundary.py | 14 ++++ tests/test_strict_catalog_review.py | 17 +++++ 4 files changed, 98 insertions(+), 27 deletions(-) diff --git a/src/openstatspec/sql/catalog_verification.py b/src/openstatspec/sql/catalog_verification.py index e338938..46a96cc 100644 --- a/src/openstatspec/sql/catalog_verification.py +++ b/src/openstatspec/sql/catalog_verification.py @@ -205,7 +205,11 @@ def verify_catalog_relations( if name ) - from .workflow import PROFILE_ID, PROFILE_SCHEMA_VERSION, workflow_catalog + from .workflow import ( + PROFILE_ID, PROFILE_SCHEMA_VERSION, TransformationError, + _assert_trigger_definitions, _derived_trigger_sql, + _validate_workflow_schema, workflow_catalog, + ) workflow = workflow_catalog(MetaData()) workflow_tables = {table.name: table for table in workflow.all()} @@ -216,6 +220,10 @@ def verify_catalog_relations( for table in workflow_tables.values() ): _reject(workflow_present) + try: + _validate_workflow_schema(connection, workflow) + except TransformationError: + _reject(workflow_present) identity_rows = connection.execute( select(workflow.transformation_profile_identity) ).mappings().all() @@ -228,6 +236,7 @@ def verify_catalog_relations( _reject({workflow.transformation_profile_identity.name}) owned_tables.update(workflow_tables) for row in connection.execute(select( + workflow.derived_dataset.c.derived_dataset_id, workflow.derived_dataset.c.physical_relation_name, workflow.derived_dataset.c.output_mode, )).mappings(): @@ -236,6 +245,16 @@ def verify_catalog_relations( owned_views.add(name) else: owned_tables.add(name) + try: + _assert_trigger_definitions( + connection, + _derived_trigger_sql( + connection, str(row["derived_dataset_id"]), name, + ), + code="derived_corrupt", + ) + except TransformationError: + _reject({name}) from .inplace_transform import apply_audit_catalog diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 3dc6785..49fe165 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -474,23 +474,33 @@ def _dolt_evidence_block( def _capture_dolt_state( connection: Any, *, profile_name: str, audit_relations: set[str], ) -> dict[str, Any] | None: - """Capture the Dolt working-set identity without mutating it.""" - del audit_relations + """Capture and classify the Dolt working set without mutating it.""" + audit_relations = {str(name) for name in audit_relations} if profile_name != "dolt": return None identity = connection.exec_driver_sql( "SELECT DATABASE() AS database_name, ACTIVE_BRANCH() AS active_branch, " "DOLT_HASHOF('HEAD') AS head_hash" ).mappings().one() + status_rows = connection.exec_driver_sql( + "SELECT table_name, staged, status FROM dolt_status " + "ORDER BY table_name, staged, status" + ).mappings().all() state = { "database": str(identity["database_name"]).strip(), "active_branch": str(identity["active_branch"]).strip(), "head": str(identity["head_hash"]).strip(), "status": _dolt_evidence_block( - connection.exec_driver_sql( - "SELECT table_name, staged, status FROM dolt_status " - "ORDER BY table_name, staged, status" - ).mappings().all(), + status_rows, expected_keys=("table_name", "staged", "status"), + ), + "diff_summaries": {}, + } + unrelated = { + "status": _dolt_evidence_block( + ( + row for row in status_rows + if str(dict(row).get("table_name") or "") not in audit_relations + ), expected_keys=("table_name", "staged", "status"), ), "diff_summaries": {}, @@ -500,18 +510,33 @@ def _capture_dolt_state( ("head_to_staged", "HEAD", "STAGED"), ("staged_to_working", "STAGED", "WORKING"), ): + rows = connection.exec_driver_sql( + "SELECT from_table_name, to_table_name, diff_type, " + "data_change, schema_change " + f"FROM DOLT_DIFF_SUMMARY('{left}', '{right}') " + "ORDER BY from_table_name, to_table_name, diff_type" + ).mappings().all() + expected_keys = ( + "from_table_name", "to_table_name", "diff_type", + "data_change", "schema_change", + ) state["diff_summaries"][label] = _dolt_evidence_block( - connection.exec_driver_sql( - "SELECT from_table_name, to_table_name, diff_type, " - "data_change, schema_change " - f"FROM DOLT_DIFF_SUMMARY('{left}', '{right}') " - "ORDER BY from_table_name, to_table_name, diff_type" - ).mappings().all(), - expected_keys=( - "from_table_name", "to_table_name", "diff_type", - "data_change", "schema_change", + rows, expected_keys=expected_keys, + ) + unrelated["diff_summaries"][label] = _dolt_evidence_block( + ( + row for row in rows + if not { + str(value) for value in ( + dict(row).get("from_table_name"), + dict(row).get("to_table_name"), + ) if value + } <= audit_relations ), + expected_keys=expected_keys, ) + state["unrelated_working_set"] = unrelated + state["unrelated_sha256"] = _canonical_sha256(unrelated) state["snapshot_sha256"] = _canonical_sha256(state) return state @@ -545,9 +570,11 @@ def _require_dolt_success_identity( return if before is None or after is None or any( before[key] != after[key] for key in ("database", "active_branch", "head") - ): + ) or ( + "unrelated_sha256" in before or "unrelated_sha256" in after + ) and before.get("unrelated_sha256") != after.get("unrelated_sha256"): raise UnsupportedOperationError( - f"Dolt database/branch/HEAD changed during {phase}." + f"Dolt identity or unrelated working-set state changed during {phase}." ) @@ -676,10 +703,7 @@ def initialize_wide_catalog( ) engine = create_engine(database_url) normative = normative_catalog(MetaData()) - audit_relations = { - normative.fidelity_event.name, - normative.operation.name, - } + audit_relations = {table.name for table in normative.all()} with _bound_catalog_transaction( engine=engine, profile_name=profile.name, active=active, audit_relations=audit_relations, phase="catalog initialization", @@ -780,10 +804,7 @@ def create_wide_dataset( ) engine = create_engine(database_url) normative = normative_catalog(MetaData()) - audit_relations = { - normative.fidelity_event.name, - normative.operation.name, - } + audit_relations = {table.name for table in normative.all()} with _bound_catalog_transaction( engine=engine, profile_name=profile.name, active=active_connection, audit_relations=audit_relations, phase="catalog initialization", @@ -851,7 +872,7 @@ def create_wide_dataset( namespace_owned = True with _bound_catalog_transaction( engine=engine, profile_name=profile.name, active=active_connection, - audit_relations=audit_relations, phase="import", + audit_relations={*audit_relations, data_table.name}, phase="import", ) as connection: if connection.execute( select(normative.dataset.c.dataset_id).where(or_( diff --git a/tests/test_review_transaction_boundary.py b/tests/test_review_transaction_boundary.py index c385cef..413bfc1 100644 --- a/tests/test_review_transaction_boundary.py +++ b/tests/test_review_transaction_boundary.py @@ -57,6 +57,20 @@ def capture(connection, **_kwargs): assert transaction_states == [False, True] +def test_dolt_completion_rejects_unrelated_working_set_changes(): + before = { + "database": "catalog", "active_branch": "main", "head": "abc123", + "unrelated_sha256": "before", + } + after = {**before, "unrelated_sha256": "after"} + + import pytest + from openstatspec.core import UnsupportedOperationError + + with pytest.raises(UnsupportedOperationError, match="unrelated working-set"): + wide._require_dolt_success_identity(before, after, phase="export audit") + + def test_export_lifecycle_events_remain_linked_to_the_dataset(tmp_path): path = tmp_path / "export-events.sqlite" database_url = f"sqlite:///{path}" diff --git a/tests/test_strict_catalog_review.py b/tests/test_strict_catalog_review.py index 21f0f6e..cb36a93 100644 --- a/tests/test_strict_catalog_review.py +++ b/tests/test_strict_catalog_review.py @@ -95,6 +95,23 @@ def test_read_and_export_reject_obsolete_catalog_relations(tmp_path): ) +def test_missing_workflow_trigger_is_rejected_by_core_catalog_verification( + tmp_path, +): + path = tmp_path / "workflow-trigger-drift.sqlite" + database = f"sqlite:///{path}" + openstatspec.initialize_catalog(database_url=database) + engine = create_engine(database) + with engine.begin() as connection: + create_workflow_catalog(connection, workflow_catalog(MetaData())) + connection.exec_driver_sql( + "drop trigger oss_transformation_run_no_delete" + ) + + with pytest.raises(UnsupportedOperationError, match="incompatible"): + openstatspec.initialize_catalog(database_url=database) + + def test_verified_workflow_profile_remains_catalog_owned(tmp_path): path = tmp_path / "workflow-owned.sqlite" database = f"sqlite:///{path}" From f726e06157402524bdfffd6cea9d0dc6e449e9ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 23:33:27 +0300 Subject: [PATCH 14/27] Honor removed derived relation history --- src/openstatspec/sql/catalog_verification.py | 8 ++++++++ tests/test_sql_workflow.py | 3 +++ 2 files changed, 11 insertions(+) diff --git a/src/openstatspec/sql/catalog_verification.py b/src/openstatspec/sql/catalog_verification.py index 46a96cc..381ce28 100644 --- a/src/openstatspec/sql/catalog_verification.py +++ b/src/openstatspec/sql/catalog_verification.py @@ -235,11 +235,19 @@ def verify_catalog_relations( ): _reject({workflow.transformation_profile_identity.name}) owned_tables.update(workflow_tables) + physically_removed = set(connection.execute(select( + workflow.derived_dataset_disposition_event.c.derived_dataset_id + ).where( + workflow.derived_dataset_disposition_event.c.event_kind + == "physical_removed" + )).scalars()) for row in connection.execute(select( workflow.derived_dataset.c.derived_dataset_id, workflow.derived_dataset.c.physical_relation_name, workflow.derived_dataset.c.output_mode, )).mappings(): + if row["derived_dataset_id"] in physically_removed: + continue name = str(row["physical_relation_name"]) if row["output_mode"] == "view": owned_views.add(name) diff --git a/tests/test_sql_workflow.py b/tests/test_sql_workflow.py index 0b8a7c6..ad80e72 100644 --- a/tests/test_sql_workflow.py +++ b/tests/test_sql_workflow.py @@ -500,6 +500,9 @@ def test_append_only_retire_remove_and_reconcile_protocol(catalog): assert openstatspec.reconcile_derived_removals( database_url=url, )["reconciled"] == 0 + assert openstatspec.initialize_catalog( + database_url=url, + )["catalog"] == "verified" def test_sqlite_is_the_only_advertised_and_executable_workflow_backend(): From e09dc4ead153e441692674d238b5330d8dea7859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Tue, 4 Aug 2026 23:57:37 +0300 Subject: [PATCH 15/27] Add basic variable transformations --- src/openstatspec/__init__.py | 5 +- src/openstatspec/frontends/spss/binding.py | 54 +++++++++- src/openstatspec/frontends/spss/syntax.py | 55 ++++++++++ src/openstatspec/sql/inplace_transform.py | 113 +++++++++++++++++++-- src/openstatspec/transform/__init__.py | 6 +- src/openstatspec/transform/plan.py | 89 +++++++++++++++- src/openstatspec/transform/schema.py | 12 +++ src/openstatspec/transform/validation.py | 48 ++++++++- 8 files changed, 361 insertions(+), 21 deletions(-) diff --git a/src/openstatspec/__init__.py b/src/openstatspec/__init__.py index db64588..f021d16 100644 --- a/src/openstatspec/__init__.py +++ b/src/openstatspec/__init__.py @@ -15,7 +15,8 @@ from .sql.workflow import TransformationError from .transform import ( AssignOperation, BooleanExpression, ComparisonExpression, - ConditionalAssignOperation, ExecuteOperation, Operand, PredicateExpression, + ConditionalAssignOperation, CreateVariableOperation, DeleteVariableOperation, + ExecuteOperation, Operand, PredicateExpression, RecodeMatch, RecodeOperation, RecodeResult, RecodeRule, ReplaceValueLabelsOperation, SetFormatOperation, SetMeasurementLevelOperation, SetVariableLabelOperation, @@ -26,7 +27,7 @@ __all__ = [ "AssignOperation", "BooleanExpression", "ComparisonExpression", "ConditionalAssignOperation", "ExecuteOperation", "Operand", - "PredicateExpression", + "ConditionalAssignOperation", "CreateVariableOperation", "DeleteVariableOperation", "CapabilityDeclaration", "DoltConformanceSource", "LossReport", "SpssFrontendCompilation", "TransformationError", "TransformationFrontendError", diff --git a/src/openstatspec/frontends/spss/binding.py b/src/openstatspec/frontends/spss/binding.py index 0d9f275..2bd5d87 100644 --- a/src/openstatspec/frontends/spss/binding.py +++ b/src/openstatspec/frontends/spss/binding.py @@ -8,7 +8,8 @@ from ...transform.errors import SourceSpan, frontend_error from ...transform.plan import ( AssignOperation, BooleanExpression, ComparisonExpression, - ConditionalAssignOperation, ExecuteOperation, Operand, PlanOperation, + ConditionalAssignOperation, CreateVariableOperation, DeleteVariableOperation, + ExecuteOperation, Operand, PlanOperation, PredicateExpression, RecodeMatch, RecodeOperation, RecodeResult, RecodeRule, ReplaceValueLabelsOperation, SetFormatOperation, SetMeasurementLevelOperation, SetVariableLabelOperation, @@ -21,10 +22,11 @@ from ...transform.validation import bind_transformation_plan from .syntax import ( BooleanSyntax, ComparisonSyntax, ComputeCommandSyntax, ExecuteCommandSyntax, - FormatsCommandSyntax, IfCommandSyntax, OperandSyntax, PredicateSyntax, + DeleteVariablesCommandSyntax, FormatsCommandSyntax, IfCommandSyntax, + OperandSyntax, PredicateSyntax, RecodeCommandSyntax, RecodeMatchSyntax, RecodeResultSyntax, SpssSyntaxProgram, SyntaxLiteral, ValueLabelsCommandSyntax, - VariableLabelsCommandSyntax, VariableLevelCommandSyntax, + StringCommandSyntax, VariableLabelsCommandSyntax, VariableLevelCommandSyntax, ) @@ -290,8 +292,6 @@ def _bind_recode( # replace intentionally preserves the existing variable metadata. A later # VALUE LABELS command replaces value labels explicitly. return operations, spans - - def bind_spss_syntax( program: SpssSyntaxProgram, schema: VariableSchema, *, input_alias: str = "parent", ) -> BoundTransformation: @@ -305,6 +305,50 @@ def bind_spss_syntax( operations: list[PlanOperation] = [] spans: list[SourceSpan] = [] for command in program.commands: + if isinstance(command, StringCommandSyntax): + for variable_token in command.variables: + if variable_token.text.startswith("__"): + raise frontend_error( + "reserved_target_name", + f"Target name {variable_token.text!r} is reserved.", + span=variable_token.span, + target=variable_token.text, + ) + if any( + variable.name.casefold() == variable_token.text.casefold() + for variable in variables + ): + raise frontend_error( + "target_already_exists", + f"Target name {variable_token.text!r} already exists.", + span=variable_token.span, + target=variable_token.text, + ) + operations.append(CreateVariableOperation( + variable_token.text, "string", command.width, + )) + spans.append(command.span) + variables.append(VariableDefinition( + variable_token.text, "string", + declared_string_width=command.width, + )) + continue + if isinstance(command, DeleteVariablesCommandSyntax): + for variable_token in command.variables: + index, variable = _resolve( + variables, variable_token.text, variable_token.span, + ) + if len(variables) == 1: + raise frontend_error( + "cannot_delete_last_variable", + "DELETE VARIABLES cannot remove the final dataset variable.", + span=variable_token.span, + variable=variable.name, + ) + operations.append(DeleteVariableOperation(variable.name)) + spans.append(command.span) + del variables[index] + continue if isinstance(command, RecodeCommandSyntax): recodes, recode_spans = _bind_recode(command, variables) operations.extend(recodes) diff --git a/src/openstatspec/frontends/spss/syntax.py b/src/openstatspec/frontends/spss/syntax.py index 04cb179..ed78f93 100644 --- a/src/openstatspec/frontends/spss/syntax.py +++ b/src/openstatspec/frontends/spss/syntax.py @@ -137,6 +137,19 @@ class ExecuteCommandSyntax: span: SourceSpan +@dataclass(frozen=True) +class StringCommandSyntax: + variables: tuple[Token, ...] + width: int + span: SourceSpan + + +@dataclass(frozen=True) +class DeleteVariablesCommandSyntax: + variables: tuple[Token, ...] + span: SourceSpan + + @dataclass(frozen=True) class VariableLabelSyntax: @@ -175,6 +188,7 @@ class ValueLabelsCommandSyntax: RecodeCommandSyntax | ComputeCommandSyntax | IfCommandSyntax | VariableLabelsCommandSyntax | ValueLabelsCommandSyntax | FormatsCommandSyntax | VariableLevelCommandSyntax | ExecuteCommandSyntax + | StringCommandSyntax | DeleteVariablesCommandSyntax ) @@ -549,6 +563,43 @@ def execute(self, start: Token) -> ExecuteCommandSyntax: end = self.expects("period", "Expected '.' after EXECUTE.") return ExecuteCommandSyntax(_joined_span(start.span, end.span)) + def string(self, start: Token) -> StringCommandSyntax: + variables = self.variable_list( + stop_kinds=frozenset({"left_paren", "period", "eof"}), + ) + self.expects("left_paren", "Expected '(' before a STRING width.") + width_token = self.expects( + "identifier", "STRING requires a width such as A20.", + ) + match = re.fullmatch(r"A([0-9]+)", width_token.text, re.IGNORECASE) + if match is None: + raise frontend_error( + "spss_syntax_error", + "STRING supports only character widths such as A20.", + span=width_token.span, + ) + width = int(match.group(1)) + if not 1 <= width <= 32767: + raise frontend_error( + "invalid_string_width", + "STRING width must be between 1 and 32767.", + span=width_token.span, + width=width, + ) + self.expects("right_paren", "Expected ')' after a STRING width.") + end = self.expects("period", "Expected '.' after STRING.") + return StringCommandSyntax( + variables, width, _joined_span(start.span, end.span), + ) + + def delete_variables(self, start: Token) -> DeleteVariablesCommandSyntax: + self.expects_keyword("VARIABLES") + variables = self.variable_list(stop_kinds=frozenset({"period", "eof"})) + end = self.expects("period", "Expected '.' after DELETE VARIABLES.") + return DeleteVariablesCommandSyntax( + variables, _joined_span(start.span, end.span), + ) + def recode_result(self) -> RecodeResultSyntax: if (token := self.accepts_keyword("SYSMIS")) is not None: @@ -692,6 +743,10 @@ def parse(self) -> SpssSyntaxProgram: commands.append(self.formats(start)) elif command == "execute": commands.append(self.execute(start)) + elif command == "string": + commands.append(self.string(start)) + elif command == "delete": + commands.append(self.delete_variables(start)) elif command == "variable": if self.current.kind == "identifier" and self.current.text.casefold() == "level": commands.append(self.variable_level(start)) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 7669be7..554f71d 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -16,6 +16,7 @@ from ..transform import ( AssignOperation, BooleanExpression, ComparisonExpression, + CreateVariableOperation, DeleteVariableOperation, ConditionalAssignOperation, ExecuteOperation, Operand, RecodeOperation, RecodeResult, ReplaceValueLabelsOperation, SetFormatOperation, SetMeasurementLevelOperation, @@ -57,7 +58,8 @@ def in_place_transformation_capabilities() -> dict[str, Any]: "parent_kinds": ["core"], "mutation": "same_dataset_same_physical_wide_table", "commands": ["RECODE", "COMPUTE", "IF", "VARIABLE LABELS", - "VALUE LABELS", "FORMATS", "VARIABLE LEVEL", "EXECUTE"], + "VALUE LABELS", "FORMATS", "VARIABLE LEVEL", + "STRING", "DELETE VARIABLES", "EXECUTE"], "new_target_column": { "sqlite": True, "postgresql": True, @@ -211,6 +213,10 @@ def _input_schema( and str(row["storage_kind"]) == "numeric" else None ), measurement_level=row["measurement_level"], + declared_string_width=( + int(row["declared_string_width"]) + if row["declared_string_width"] is not None else None + ), ) for row in variables )) @@ -301,6 +307,53 @@ def _replace_value_labels( label=item.label, )) +def _delete_variable_metadata( + connection: Any, + *, + core: Any, + variable: dict[str, Any], +) -> None: + """Delete one variable and every normative catalog row owned by it.""" + variable_id = str(variable["variable_id"]) + label_set_id = connection.execute( + select(core.variable_value_label_set.c.value_label_set_id).where( + core.variable_value_label_set.c.variable_id == variable_id + ) + ).scalar_one_or_none() + connection.execute(delete(core.dataset_weight_variable).where( + core.dataset_weight_variable.c.variable_id == variable_id + )) + connection.execute(delete(core.variable_set_member).where( + core.variable_set_member.c.variable_id == variable_id + )) + connection.execute(delete(core.multiple_response_member).where( + core.multiple_response_member.c.variable_id == variable_id + )) + connection.execute(delete(core.variable_attribute).where( + core.variable_attribute.c.variable_id == variable_id + )) + connection.execute(delete(core.missing_rule).where( + core.missing_rule.c.variable_id == variable_id + )) + connection.execute(delete(core.variable_value_label_set).where( + core.variable_value_label_set.c.variable_id == variable_id + )) + if label_set_id is not None: + still_used = connection.execute( + select(core.variable_value_label_set.c.variable_id).where( + core.variable_value_label_set.c.value_label_set_id == label_set_id + ) + ).first() + if still_used is None: + connection.execute(delete(core.value_label).where( + core.value_label.c.value_label_set_id == label_set_id + )) + connection.execute(delete(core.value_label_set).where( + core.value_label_set.c.value_label_set_id == label_set_id + )) + connection.execute(delete(core.variable).where( + core.variable.c.variable_id == variable_id + )) def _failure_boundary(_name: str) -> None: """Synthetic-test hook for schema/data/catalog/audit failure boundaries.""" @@ -382,17 +435,25 @@ def _apply_plan_on_connection( ) create_operations = [ operation for operation in plan.operations - if isinstance(operation, (RecodeOperation, AssignOperation)) - and operation.target_mode == "create" + if isinstance(operation, CreateVariableOperation) + or ( + isinstance(operation, (RecodeOperation, AssignOperation)) + and operation.target_mode == "create" + ) + ] + schema_operations = create_operations + [ + operation for operation in plan.operations + if isinstance(operation, DeleteVariableOperation) ] - if create_operations and not allow_schema_change: + if schema_operations and not allow_schema_change: raise TransformationError( "schema_change_not_atomic", "This database profile has no coherent new-target strategy.", ) unsupported_targets = [ operation.target for operation in create_operations - if output_by_name[operation.target.casefold()].storage_kind != "numeric" + if not isinstance(operation, CreateVariableOperation) + and output_by_name[operation.target.casefold()].storage_kind != "numeric" ] if unsupported_targets: raise TransformationError( @@ -449,10 +510,26 @@ def _apply_plan_on_connection( # Compensation is journal-bounded to target columns and catalog identities # created by this apply, without resetting unrelated database state. for operation in create_operations: - target_physical = physical_name(operation.target, used_physical) + target_name = ( + operation.variable + if isinstance(operation, CreateVariableOperation) + else operation.target + ) + target_physical = physical_name(target_name, used_physical) + storage_kind = ( + operation.storage_kind + if isinstance(operation, CreateVariableOperation) + else "numeric" + ) + string_width = ( + operation.declared_string_width + if isinstance(operation, CreateVariableOperation) + else None + ) + column_type = numeric_type if storage_kind == "numeric" else f"VARCHAR({string_width})" connection.exec_driver_sql( f"ALTER TABLE {qualified_table} ADD COLUMN " - f"{quote(target_physical)} {numeric_type} NULL" + f"{quote(target_physical)} {column_type} NULL" ) if mutation_journal is not None: mutation_journal["added_columns"].append(target_physical) @@ -460,9 +537,10 @@ def _apply_plan_on_connection( "variable_id": str(uuid4()), "dataset_id": dataset_id, "source_ordinal": next_ordinal, - "source_name": operation.target, + "source_name": target_name, "physical_name": target_physical, - "storage_kind": "numeric", + "storage_kind": storage_kind, + "declared_string_width": string_width, "variable_label": None, } next_ordinal += 1 @@ -549,6 +627,23 @@ def _apply_plan_on_connection( core.variable.c.variable_id == variable["variable_id"] ).values(measurement_level=operation.level)) _failure_boundary("catalog") + elif isinstance(operation, DeleteVariableOperation): + variable = by_name[operation.variable.casefold()] + connection.exec_driver_sql( + f"ALTER TABLE {qualified_table} DROP COLUMN " + f"{quote(variable['physical_name'])}" + ) + _delete_variable_metadata(connection, core=core, variable=variable) + by_name.pop(operation.variable.casefold(), None) + variables = [ + row for row in variables + if row["variable_id"] != variable["variable_id"] + ] + relation = Table( + table_name, MetaData(), schema=dataset.get("physical_table_schema"), + autoload_with=connection, + ) + _failure_boundary("schema") elif isinstance(operation, ExecuteOperation): continue else: # pragma: no cover diff --git a/src/openstatspec/transform/__init__.py b/src/openstatspec/transform/__init__.py index bf89591..d2df0ea 100644 --- a/src/openstatspec/transform/__init__.py +++ b/src/openstatspec/transform/__init__.py @@ -2,9 +2,11 @@ from .errors import SourcePosition, SourceSpan, TransformationFrontendError from .plan import ( + TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT, TRANSFORMATION_PLAN_CONTRACT, AssignOperation, BooleanExpression, ComparisonExpression, - ConditionalAssignOperation, ExecuteOperation, Operand, PredicateExpression, + ConditionalAssignOperation, CreateVariableOperation, DeleteVariableOperation, + ExecuteOperation, Operand, PredicateExpression, RecodeMatch, RecodeOperation, RecodeResult, RecodeRule, ReplaceValueLabelsOperation, SetFormatOperation, SetMeasurementLevelOperation, SetVariableLabelOperation, TransformationPlan, @@ -17,7 +19,7 @@ from .validation import bind_transformation_plan __all__ = [ "AssignOperation", "BooleanExpression", "ComparisonExpression", - "ConditionalAssignOperation", "ExecuteOperation", "Operand", + "ConditionalAssignOperation", "CreateVariableOperation", "DeleteVariableOperation", "PredicateExpression", "BoundTransformation", "RecodeMatch", "RecodeOperation", "RecodeResult", diff --git a/src/openstatspec/transform/plan.py b/src/openstatspec/transform/plan.py index 97523d6..7e6b842 100644 --- a/src/openstatspec/transform/plan.py +++ b/src/openstatspec/transform/plan.py @@ -16,7 +16,8 @@ TRANSFORMATION_PLAN_V1_CONTRACT = "openstatspec-transformation-plan-v0.1" TRANSFORMATION_PLAN_CONTRACT = "openstatspec-transformation-plan-v0.2" -_TRANSFORMATION_PLAN_CONTRACTS = {TRANSFORMATION_PLAN_V1_CONTRACT, TRANSFORMATION_PLAN_CONTRACT} +TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT = "openstatspec-transformation-plan-v0.3" +_TRANSFORMATION_PLAN_CONTRACTS = {TRANSFORMATION_PLAN_V1_CONTRACT, TRANSFORMATION_PLAN_CONTRACT, TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT} _BINARY64 = re.compile(r"[0-9a-f]{16}") @@ -504,10 +505,77 @@ def as_dict(self) -> dict[str, str]: return {"op": self.op} + +@dataclass(frozen=True) +class CreateVariableOperation: + """Add one variable to the existing dataset and wide table.""" + + variable: str + storage_kind: Literal["numeric", "string"] + declared_string_width: int | None = None + op: Literal["create_variable"] = "create_variable" + + def __post_init__(self) -> None: + if self.op != "create_variable": + _invalid("Create-variable operation discriminator is invalid.") + if not isinstance(self.variable, str) or not self.variable: + _invalid("Created variable name must be non-empty text.") + if self.variable.startswith("__"): + raise frontend_error( + "reserved_target_name", + f"Target name {self.variable!r} is reserved.", + target=self.variable, + ) + if self.storage_kind not in {"numeric", "string"}: + _invalid("Created variable storage_kind must be numeric or string.") + if self.storage_kind == "string": + if ( + not isinstance(self.declared_string_width, int) + or isinstance(self.declared_string_width, bool) + or self.declared_string_width < 1 + ): + _invalid("String variables require a positive declared_string_width.") + elif self.declared_string_width is not None: + _invalid("Numeric variables cannot declare a string width.") + + def as_dict(self) -> dict[str, Any]: + result: dict[str, Any] = { + "op": self.op, + "variable": self.variable, + "storage_kind": self.storage_kind, + } + if self.declared_string_width is not None: + result["declared_string_width"] = self.declared_string_width + return result + + +@dataclass(frozen=True) +class DeleteVariableOperation: + """Remove one variable and all of its normative metadata.""" + + variable: str + op: Literal["delete_variable"] = "delete_variable" + + def __post_init__(self) -> None: + if self.op != "delete_variable": + _invalid("Delete-variable operation discriminator is invalid.") + if not isinstance(self.variable, str) or not self.variable: + _invalid("Deleted variable name must be non-empty text.") + if self.variable.startswith("__"): + raise frontend_error( + "reserved_target_name", + f"Variable name {self.variable!r} is reserved.", + variable=self.variable, + ) + + def as_dict(self) -> dict[str, str]: + return {"op": self.op, "variable": self.variable} + PlanOperation = ( RecodeOperation | AssignOperation | ConditionalAssignOperation | SetVariableLabelOperation | ReplaceValueLabelsOperation | SetFormatOperation | SetMeasurementLevelOperation | ExecuteOperation + | CreateVariableOperation | DeleteVariableOperation ) @@ -524,6 +592,7 @@ def __post_init__(self) -> None: isinstance(operation, ( AssignOperation, ConditionalAssignOperation, SetFormatOperation, SetMeasurementLevelOperation, ExecuteOperation, + CreateVariableOperation, DeleteVariableOperation, )) for operation in self.operations ): @@ -539,6 +608,7 @@ def __post_init__(self) -> None: RecodeOperation, AssignOperation, ConditionalAssignOperation, SetVariableLabelOperation, ReplaceValueLabelsOperation, SetFormatOperation, SetMeasurementLevelOperation, ExecuteOperation, + CreateVariableOperation, DeleteVariableOperation, ), ) for operation in self.operations @@ -639,7 +709,6 @@ def _match(raw: Any) -> RecodeMatch: return RecodeMatch("system_missing") _invalid("Unknown recode match kind.") - def transformation_plan_from_dict(raw: Mapping[str, Any]) -> TransformationPlan: """Strictly validate canonical v0.1 or additive v0.2 plan documents.""" if not isinstance(raw, Mapping): @@ -733,6 +802,22 @@ def transformation_plan_from_dict(raw: Mapping[str, Any]) -> TransformationPlan: operations.append(ReplaceValueLabelsOperation( raw_operation["variable"], tuple(labels), )) + elif operation == "create_variable": + _exact( + raw_operation, + {"op", "variable", "storage_kind", "declared_string_width"} + if "declared_string_width" in raw_operation + else {"op", "variable", "storage_kind"}, + "Create-variable operation", + ) + operations.append(CreateVariableOperation( + variable=raw_operation["variable"], + storage_kind=raw_operation["storage_kind"], + declared_string_width=raw_operation.get("declared_string_width"), + )) + elif operation == "delete_variable": + _exact(raw_operation, {"op", "variable"}, "Delete-variable operation") + operations.append(DeleteVariableOperation(raw_operation["variable"])) else: _invalid(f"Unknown plan operation {operation!r}.") return TransformationPlan( diff --git a/src/openstatspec/transform/schema.py b/src/openstatspec/transform/schema.py index 5dace1d..4b66135 100644 --- a/src/openstatspec/transform/schema.py +++ b/src/openstatspec/transform/schema.py @@ -21,6 +21,7 @@ class VariableDefinition: format_width: int | None = None format_decimals: int | None = None measurement_level: Literal["nominal", "ordinal", "scale"] | None = None + declared_string_width: int | None = None def __post_init__(self) -> None: if not isinstance(self.name, str) or not self.name: @@ -28,6 +29,17 @@ def __post_init__(self) -> None: if self.storage_kind not in {"numeric", "string"}: raise ValueError("storage_kind must be numeric or string.") expected_type = "binary64" if self.storage_kind == "numeric" else "string" + if self.declared_string_width is not None: + if ( + not isinstance(self.declared_string_width, int) + or isinstance(self.declared_string_width, bool) + or self.declared_string_width < 1 + ): + raise ValueError("declared_string_width must be a positive integer.") + if self.storage_kind != "string": + raise ValueError( + "declared_string_width is only valid for string variables." + ) if any(label.value.type != expected_type for label in self.value_labels): raise ValueError( "Value-label types must match their variable storage kind." diff --git a/src/openstatspec/transform/validation.py b/src/openstatspec/transform/validation.py index c02733d..d3021c0 100644 --- a/src/openstatspec/transform/validation.py +++ b/src/openstatspec/transform/validation.py @@ -10,7 +10,9 @@ AssignOperation, BooleanExpression, ComparisonExpression, + CreateVariableOperation, ConditionalAssignOperation, + DeleteVariableOperation, ExecuteOperation, Operand, RecodeMatch, @@ -271,7 +273,6 @@ def _bind_conditional_assign( variable=target.name, ) - def bind_transformation_plan( plan: TransformationPlan, schema: VariableSchema ) -> BoundTransformation: @@ -282,6 +283,12 @@ def bind_transformation_plan( raise TypeError("schema must be a VariableSchema.") variables = list(schema.variables) for operation in plan.operations: + if isinstance(operation, CreateVariableOperation): + _bind_create(operation, variables) + continue + if isinstance(operation, DeleteVariableOperation): + _bind_delete(operation, variables) + continue if isinstance(operation, RecodeOperation): _bind_recode(operation, variables) continue @@ -305,6 +312,16 @@ def bind_transformation_plan( variable=variable.name, expected_type=expected, ) + if variable.storage_kind == "string" and variable.declared_string_width is not None: + for label in operation.labels: + assert label.value.value is not None + if len(label.value.value.encode("utf-8")) > variable.declared_string_width: + raise frontend_error( + "string_width_exceeded", + "A value-label code exceeds the variable's declared string width.", + variable=variable.name, + declared_string_width=variable.declared_string_width, + ) variables[index] = replace(variable, value_labels=operation.labels) continue if isinstance(operation, SetFormatOperation): @@ -330,3 +347,32 @@ def bind_transformation_plan( continue raise AssertionError(f"Unknown plan operation: {type(operation)!r}") return BoundTransformation(plan, VariableSchema(tuple(variables))) +def _bind_create( + operation: CreateVariableOperation, + variables: list[VariableDefinition], +) -> None: + if any(variable.name.casefold() == operation.variable.casefold() for variable in variables): + raise frontend_error( + "target_already_exists", + f"Target name {operation.variable!r} already exists.", + target=operation.variable, + ) + variables.append(VariableDefinition( + operation.variable, + operation.storage_kind, + declared_string_width=operation.declared_string_width, + )) + + +def _bind_delete( + operation: DeleteVariableOperation, + variables: list[VariableDefinition], +) -> None: + index, variable = _resolve(variables, operation.variable) + if len(variables) == 1: + raise frontend_error( + "cannot_delete_last_variable", + "A dataset must retain at least one variable.", + variable=variable.name, + ) + del variables[index] From f9caa5fee0f9a4c472d58a26efe16225a974c2f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 00:01:20 +0300 Subject: [PATCH 16/27] Bind failed import audits to Dolt state --- src/openstatspec/sql/wide.py | 13 +++++++--- tests/test_catalog_persistence_review.py | 33 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 49fe165..69715ba 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -89,10 +89,14 @@ def binary64_type() -> Float: def _record_failed_preflight( *, engine: Any, normative: Any, operation_id: str, source_name: str, source_format: str, variable_count: int, profile_name: str, - error: Exception, + active: Mapping[str, Any], error: Exception, ) -> None: - """Persist a failed preflight in the normative audit catalog.""" - with engine.begin() as connection: + """Persist a failed preflight in the bound normative audit catalog.""" + with _bound_catalog_transaction( + engine=engine, profile_name=profile_name, active=active, + audit_relations={normative.operation.name, normative.fidelity_event.name}, + phase="import preflight failure", + ) as connection: _verify_normative_catalog(connection, normative) failed_at = datetime.now(UTC).replace(tzinfo=None) record_normative_operation( @@ -832,7 +836,8 @@ def create_wide_dataset( _record_failed_preflight( engine=engine, normative=normative, operation_id=operation_id, source_name=source_name, source_format=source_format, - variable_count=len(variables), profile_name=profile.name, error=error, + variable_count=len(variables), profile_name=profile.name, + active=active_connection, error=error, ) raise diff --git a/tests/test_catalog_persistence_review.py b/tests/test_catalog_persistence_review.py index e17af47..2afdcc4 100644 --- a/tests/test_catalog_persistence_review.py +++ b/tests/test_catalog_persistence_review.py @@ -8,6 +8,7 @@ from openstatspec.core import UnsupportedOperationError from openstatspec.sql import wide +from openstatspec.sql.profiles import TargetCapabilityExceededError def _variables(): @@ -91,6 +92,38 @@ def observe(**kwargs): assert phases == ["catalog initialization", "import"] +def test_failed_import_preflight_routes_audit_through_binding_guard( + tmp_path, monkeypatch, +): + database_url = f"sqlite:///{tmp_path / 'failed-preflight.sqlite'}" + real_bound_transaction = wide._bound_catalog_transaction + phases = [] + + @contextmanager + def observe(**kwargs): + phases.append(kwargs["phase"]) + with real_bound_transaction(**kwargs) as connection: + yield connection + + monkeypatch.setattr(wide, "_bound_catalog_transaction", observe) + variables = _variables() + variables[0].update({ + "storage_kind": "numeric", "string_width": None, + "format": "F8.2", "alignment": "right", + }) + with pytest.raises(TargetCapabilityExceededError): + wide.create_wide_dataset( + database_url=database_url, + dataset_id="invalid", + source_name="invalid.sav", + source_format="SAV", + rows=[{"name": "not-a-number"}], + variables=variables, + ) + + assert phases == ["catalog initialization", "import preflight failure"] + + def test_export_audit_mutations_route_through_binding_guard(tmp_path, monkeypatch): database_url = f"sqlite:///{tmp_path / 'bound-export.sqlite'}" _create(database_url) From 07852a588ba3a0917997f903d9536a37ee478f26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 00:28:01 +0300 Subject: [PATCH 17/27] Fix sequential schema operation execution --- src/openstatspec/sql/inplace_transform.py | 38 +++++++--- tests/test_inplace_transform.py | 89 ++++++++++++++++++++++- 2 files changed, 117 insertions(+), 10 deletions(-) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 554f71d..5a297b6 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -400,6 +400,7 @@ def _apply_plan_on_connection( actor: str, database_profile: str, allow_schema_change: bool, + allow_delete_variable: bool, dolt_branch: str | None, dolt_head: str | None, mutation_journal: dict[str, Any] | None = None, @@ -450,6 +451,14 @@ def _apply_plan_on_connection( "schema_change_not_atomic", "This database profile has no coherent new-target strategy.", ) + if any( + isinstance(operation, DeleteVariableOperation) + for operation in plan.operations + ) and not allow_delete_variable: + raise TransformationError( + "delete_variable_not_supported", + "This SQLite runtime does not support ALTER TABLE DROP COLUMN.", + ) unsupported_targets = [ operation.target for operation in create_operations if not isinstance(operation, CreateVariableOperation) @@ -469,7 +478,7 @@ def _apply_plan_on_connection( by_name = {str(row["source_name"]).casefold(): row for row in variables} used_physical = {str(row["physical_name"]).casefold() for row in variables} next_ordinal = max(int(row["source_ordinal"]) for row in variables) + 1 - target_rows: list[dict[str, Any]] = [] + target_rows_by_operation: dict[int, dict[str, Any]] = {} quote = connection.dialect.identifier_preparer.quote qualified_table = connection.dialect.identifier_preparer.format_table(relation) numeric_type = ( @@ -544,24 +553,27 @@ def _apply_plan_on_connection( "variable_label": None, } next_ordinal += 1 - target_rows.append(target_row) + target_rows_by_operation[id(operation)] = target_row if mutation_journal is not None: mutation_journal["target_rows"].append(dict(target_row)) if create_operations: _failure_boundary("schema") - for target_row in target_rows: - connection.execute(insert(core.variable).values(**target_row)) - variables.append(target_row) - by_name[str(target_row["source_name"]).casefold()] = target_row - if target_rows: - _failure_boundary("catalog") + if create_operations: relation = Table( table_name, MetaData(), schema=dataset.get("physical_table_schema"), autoload_with=connection, ) for operation in plan.operations: + created_target = target_rows_by_operation.get(id(operation)) + if created_target is not None: + connection.execute(insert(core.variable).values(**created_target)) + variables.append(created_target) + by_name[str(created_target["source_name"]).casefold()] = created_target + _failure_boundary("catalog") + if isinstance(operation, CreateVariableOperation): + continue if isinstance(operation, RecodeOperation): source_variable = by_name[operation.source.casefold()] target_variable = by_name[operation.target.casefold()] @@ -856,9 +868,16 @@ def _run_in_place_submission( raise TransformationError( "actor_required", "A non-empty actor identity is mandatory.", ) - profile, _active = effective_profile( + profile, active = effective_profile( database_url, dolt_conformance_source=dolt_conformance_source, ) + allow_delete_variable = True + if profile.name == "sqlite": + sqlite_version_parts = tuple( + int(part) for part in str(active["server_version"]).split(".") + ) + sqlite_version = (*sqlite_version_parts, 0, 0)[:3] + allow_delete_variable = sqlite_version >= (3, 35, 0) engine = create_engine(database_url) journal: dict[str, Any] = {} branch: str | None = None @@ -905,6 +924,7 @@ def _run_in_place_submission( actor=actor, database_profile=profile.name, allow_schema_change=profile.name in {"sqlite", "postgresql"}, + allow_delete_variable=allow_delete_variable, dolt_branch=branch, dolt_head=head, mutation_journal=journal, diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 9236d8f..28b6a11 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -103,6 +103,7 @@ def test_plan_applies_to_same_dataset_and_physical_table_without_copy( actor="test-agent", database_profile="sqlite", allow_schema_change=True, + allow_delete_variable=True, dolt_branch=None, dolt_head=None, ) @@ -153,6 +154,56 @@ def test_plan_applies_to_same_dataset_and_physical_table_without_copy( ) +def test_string_declaration_creates_column_and_catalog_variable(catalog) -> None: + url, path, dataset_id, table_name = catalog + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text="STRING note (A8).", + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert connection.execute( + "select source_name, storage_kind, declared_string_width from variable " + "where dataset_id = ? order by source_ordinal", + (dataset_id,), + ).fetchall() == [("score", "numeric", None), ("note", "string", 8)] + assert connection.execute( + f'SELECT note FROM "{table_name}" ORDER BY __case_ordinal' + ).fetchall() == [(None,), (None,), (None,)] + connection.close() + + +def test_delete_then_recreate_same_name_resolves_operations_in_order(catalog) -> None: + url, path, dataset_id, table_name = catalog + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text=( + "COMPUTE other = score. DELETE VARIABLES score. " + "RECODE other (2 = 9) (ELSE = COPY) INTO score." + ), + actor="test-agent", + ) + + connection = sqlite3.connect(path) + variables = connection.execute( + "select source_name, physical_name from variable where dataset_id = ? " + "order by source_ordinal", + (dataset_id,), + ).fetchall() + assert [row[0] for row in variables] == ["other", "score"] + physical = dict(variables) + assert connection.execute( + f'SELECT "{physical["other"]}", "{physical["score"]}" ' + f'FROM "{table_name}" ORDER BY __case_ordinal' + ).fetchall() == [(1.0, 1.0), (2.0, 9.0), (3.0, 3.0)] + connection.close() + + def test_public_apply_supports_non_dolt_without_building_undo(catalog) -> None: url, path, dataset_id, table_name = catalog plan = _plan("RECODE score (1 = 0).") @@ -364,6 +415,38 @@ def test_missing_audit_schema_fails_before_mutation(catalog) -> None: ).fetchall() == [(1.0,), (2.0,), (3.0,)] +def test_delete_is_rejected_when_drop_column_is_unavailable( + catalog, monkeypatch, +) -> None: + url, path, dataset_id, table_name = catalog + monkeypatch.setattr( + inplace_transform, + "effective_profile", + lambda _url, **_kwargs: ( + SimpleNamespace(name="sqlite"), + {"server_version": "3.34.0"}, + ), + ) + + with pytest.raises(openstatspec.TransformationError) as caught: + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text="COMPUTE other = score. DELETE VARIABLES score.", + actor="test-agent", + ) + + assert caught.value.code == "delete_variable_not_supported" + connection = sqlite3.connect(path) + assert connection.execute( + f'SELECT score FROM "{table_name}" ORDER BY __case_ordinal' + ).fetchall() == [(1.0,), (2.0,), (3.0,)] + assert [ + row[1] for row in connection.execute(f'PRAGMA table_info("{table_name}")') + ] == ["__case_ordinal", "score"] + connection.close() + + def test_nontransactional_ddl_profile_rejects_create_before_mutation( catalog, ) -> None: @@ -381,6 +464,7 @@ def test_nontransactional_ddl_profile_rejects_create_before_mutation( actor="test-agent", database_profile="mysql", allow_schema_change=False, + allow_delete_variable=True, dolt_branch=None, dolt_head=None, ) @@ -692,7 +776,10 @@ def test_transactional_ddl_rollback_skips_unlocked_compensation( monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name=profile_name), {}), + lambda _url, **_kwargs: ( + SimpleNamespace(name=profile_name), + {"server_version": "3.35.0"}, + ), ) def fail_after_schema_change( From 7fce8fab1aaba677f7afda9f8c43ef3c0faf2ade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 00:47:50 +0300 Subject: [PATCH 18/27] Preserve strict wide invariants after schema edits --- src/openstatspec/sql/inplace_transform.py | 131 +++++++++++++--------- tests/test_inplace_transform.py | 25 ++++- 2 files changed, 96 insertions(+), 60 deletions(-) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 5a297b6..1409f0a 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -355,6 +355,25 @@ def _delete_variable_metadata( core.variable.c.variable_id == variable_id )) + +def _compact_variable_ordinals( + connection: Any, + *, + core: Any, + variables: list[dict[str, Any]], +) -> None: + """Keep normative variable source order contiguous after deletion.""" + for source_ordinal, variable in enumerate(variables, start=1): + if int(variable["source_ordinal"]) == source_ordinal: + continue + connection.execute( + update(core.variable) + .where(core.variable.c.variable_id == variable["variable_id"]) + .values(source_ordinal=source_ordinal) + ) + variable["source_ordinal"] = source_ordinal + + def _failure_boundary(_name: str) -> None: """Synthetic-test hook for schema/data/catalog/audit failure boundaries.""" @@ -477,8 +496,6 @@ def _apply_plan_on_connection( ) by_name = {str(row["source_name"]).casefold(): row for row in variables} used_physical = {str(row["physical_name"]).casefold() for row in variables} - next_ordinal = max(int(row["source_ordinal"]) for row in variables) + 1 - target_rows_by_operation: dict[int, dict[str, Any]] = {} quote = connection.dialect.identifier_preparer.quote qualified_table = connection.dialect.identifier_preparer.format_table(relation) numeric_type = ( @@ -515,59 +532,61 @@ def _apply_plan_on_connection( "The Dolt working set changed after locking the dataset.", ) - # All non-transactional DDL precedes every data/catalog mutation. This makes - # Compensation is journal-bounded to target columns and catalog identities - # created by this apply, without resetting unrelated database state. - for operation in create_operations: - target_name = ( - operation.variable - if isinstance(operation, CreateVariableOperation) - else operation.target - ) - target_physical = physical_name(target_name, used_physical) - storage_kind = ( - operation.storage_kind - if isinstance(operation, CreateVariableOperation) - else "numeric" - ) - string_width = ( - operation.declared_string_width - if isinstance(operation, CreateVariableOperation) - else None - ) - column_type = numeric_type if storage_kind == "numeric" else f"VARCHAR({string_width})" - connection.exec_driver_sql( - f"ALTER TABLE {qualified_table} ADD COLUMN " - f"{quote(target_physical)} {column_type} NULL" - ) - if mutation_journal is not None: - mutation_journal["added_columns"].append(target_physical) - target_row = { - "variable_id": str(uuid4()), - "dataset_id": dataset_id, - "source_ordinal": next_ordinal, - "source_name": target_name, - "physical_name": target_physical, - "storage_kind": storage_kind, - "declared_string_width": string_width, - "variable_label": None, - } - next_ordinal += 1 - target_rows_by_operation[id(operation)] = target_row - if mutation_journal is not None: - mutation_journal["target_rows"].append(dict(target_row)) - if create_operations: - _failure_boundary("schema") - - if create_operations: - relation = Table( - table_name, MetaData(), schema=dataset.get("physical_table_schema"), - autoload_with=connection, - ) - + # Schema changes are allowed only on profiles whose DDL participates in + # this apply transaction, so execute them in canonical operation order. + # That preserves deterministic physical naming across delete/recreate flows. for operation in plan.operations: - created_target = target_rows_by_operation.get(id(operation)) - if created_target is not None: + creates_target = ( + isinstance(operation, CreateVariableOperation) + or ( + isinstance(operation, (RecodeOperation, AssignOperation)) + and operation.target_mode == "create" + ) + ) + if creates_target: + target_name = ( + operation.variable + if isinstance(operation, CreateVariableOperation) + else operation.target + ) + target_physical = physical_name(target_name, used_physical) + storage_kind = ( + operation.storage_kind + if isinstance(operation, CreateVariableOperation) + else "numeric" + ) + string_width = ( + operation.declared_string_width + if isinstance(operation, CreateVariableOperation) + else None + ) + column_type = numeric_type if storage_kind == "numeric" else "TEXT" + null_clause = ( + "NULL" if storage_kind == "numeric" + else "NOT NULL DEFAULT ''" + ) + connection.exec_driver_sql( + f"ALTER TABLE {qualified_table} ADD COLUMN " + f"{quote(target_physical)} {column_type} {null_clause}" + ) + created_target = { + "variable_id": str(uuid4()), + "dataset_id": dataset_id, + "source_ordinal": len(variables) + 1, + "source_name": target_name, + "physical_name": target_physical, + "storage_kind": storage_kind, + "declared_string_width": string_width, + "variable_label": None, + } + if mutation_journal is not None: + mutation_journal["added_columns"].append(target_physical) + mutation_journal["target_rows"].append(dict(created_target)) + _failure_boundary("schema") + relation = Table( + table_name, MetaData(), schema=dataset.get("physical_table_schema"), + autoload_with=connection, + ) connection.execute(insert(core.variable).values(**created_target)) variables.append(created_target) by_name[str(created_target["source_name"]).casefold()] = created_target @@ -647,10 +666,14 @@ def _apply_plan_on_connection( ) _delete_variable_metadata(connection, core=core, variable=variable) by_name.pop(operation.variable.casefold(), None) + used_physical.discard(str(variable["physical_name"]).casefold()) variables = [ row for row in variables if row["variable_id"] != variable["variable_id"] ] + _compact_variable_ordinals( + connection, core=core, variables=variables, + ) relation = Table( table_name, MetaData(), schema=dataset.get("physical_table_schema"), autoload_with=connection, diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 28b6a11..361cdec 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -12,7 +12,7 @@ InPlacePlanSubmission, _apply_plan_on_connection, ) -from openstatspec.sql.wide import create_wide_dataset +from openstatspec.sql.wide import create_wide_dataset, validate_wide_dataset def _variables() -> list[dict[str, object]]: @@ -172,8 +172,16 @@ def test_string_declaration_creates_column_and_catalog_variable(catalog) -> None ).fetchall() == [("score", "numeric", None), ("note", "string", 8)] assert connection.execute( f'SELECT note FROM "{table_name}" ORDER BY __case_ordinal' - ).fetchall() == [(None,), (None,), (None,)] + ).fetchall() == [("",), ("",), ("",)] + note_column = next( + row for row in connection.execute(f'PRAGMA table_info("{table_name}")') + if row[1] == "note" + ) + assert note_column[2:5] == ("TEXT", 1, "''") connection.close() + assert validate_wide_dataset( + database_url=url, dataset_id=dataset_id, + )["valid"] is True def test_delete_then_recreate_same_name_resolves_operations_in_order(catalog) -> None: @@ -191,17 +199,22 @@ def test_delete_then_recreate_same_name_resolves_operations_in_order(catalog) -> connection = sqlite3.connect(path) variables = connection.execute( - "select source_name, physical_name from variable where dataset_id = ? " - "order by source_ordinal", + "select source_name, physical_name, source_ordinal from variable " + "where dataset_id = ? order by source_ordinal", (dataset_id,), ).fetchall() - assert [row[0] for row in variables] == ["other", "score"] - physical = dict(variables) + assert [(row[0], row[2]) for row in variables] == [ + ("other", 1), ("score", 2), + ] + physical = {row[0]: row[1] for row in variables} assert connection.execute( f'SELECT "{physical["other"]}", "{physical["score"]}" ' f'FROM "{table_name}" ORDER BY __case_ordinal' ).fetchall() == [(1.0, 1.0), (2.0, 9.0), (3.0, 3.0)] connection.close() + assert validate_wide_dataset( + database_url=url, dataset_id=dataset_id, + )["valid"] is True def test_public_apply_supports_non_dolt_without_building_undo(catalog) -> None: From beaf6aee5bda59031c5f47e1ff8781f4ecdc731f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 00:59:25 +0300 Subject: [PATCH 19/27] Prune empty response sets after variable deletion --- src/openstatspec/sql/inplace_transform.py | 17 +++++++++ tests/test_inplace_transform.py | 45 +++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 1409f0a..63272aa 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -315,6 +315,11 @@ def _delete_variable_metadata( ) -> None: """Delete one variable and every normative catalog row owned by it.""" variable_id = str(variable["variable_id"]) + response_set_ids = list(connection.execute( + select(core.multiple_response_member.c.multiple_response_set_id).where( + core.multiple_response_member.c.variable_id == variable_id + ) + ).scalars()) label_set_id = connection.execute( select(core.variable_value_label_set.c.value_label_set_id).where( core.variable_value_label_set.c.variable_id == variable_id @@ -329,6 +334,18 @@ def _delete_variable_metadata( connection.execute(delete(core.multiple_response_member).where( core.multiple_response_member.c.variable_id == variable_id )) + for response_set_id in response_set_ids: + remaining_member = connection.execute( + select(core.multiple_response_member.c.variable_id).where( + core.multiple_response_member.c.multiple_response_set_id + == response_set_id + ) + ).first() + if remaining_member is None: + connection.execute(delete(core.multiple_response_set).where( + core.multiple_response_set.c.multiple_response_set_id + == response_set_id + )) connection.execute(delete(core.variable_attribute).where( core.variable_attribute.c.variable_id == variable_id )) diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 361cdec..575babf 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -184,6 +184,51 @@ def test_string_declaration_creates_column_and_catalog_variable(catalog) -> None )["valid"] is True +def test_delete_prunes_an_empty_multiple_response_set(catalog) -> None: + url, path, dataset_id, _table_name = catalog + connection = sqlite3.connect(path) + variable_id = connection.execute( + "select variable_id from variable where dataset_id = ?", + (dataset_id,), + ).fetchone()[0] + response_set_id = "00000000-0000-0000-0000-000000000001" + connection.execute( + "insert into multiple_response_set " + "(multiple_response_set_id, dataset_id, source_ordinal, set_name, " + "set_kind, counted_value_kind, counted_numeric_value) " + "values (?, ?, 1, '$scores', 'MD', 'numeric', 1.0)", + (response_set_id, dataset_id), + ) + connection.execute( + "insert into multiple_response_member " + "(multiple_response_set_id, variable_id, source_ordinal) " + "values (?, ?, 1)", + (response_set_id, variable_id), + ) + connection.commit() + connection.close() + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text="COMPUTE other = score. DELETE VARIABLES score.", + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert connection.execute( + "select count(*) from multiple_response_set where dataset_id = ?", + (dataset_id,), + ).fetchone() == (0,) + assert connection.execute( + "select count(*) from multiple_response_member" + ).fetchone() == (0,) + connection.close() + assert validate_wide_dataset( + database_url=url, dataset_id=dataset_id, + )["valid"] is True + + def test_delete_then_recreate_same_name_resolves_operations_in_order(catalog) -> None: url, path, dataset_id, table_name = catalog From b52a0c6d66faabd2f183243ccdfcc6c3b0b05768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 01:22:17 +0300 Subject: [PATCH 20/27] Bind cleanup and preflight schema edits --- src/openstatspec/sql/inplace_transform.py | 19 ++++++++ src/openstatspec/sql/wide.py | 7 ++- tests/test_atomic_import.py | 50 ++++++++++++++++++++ tests/test_conditional_inplace_transform.py | 8 ++-- tests/test_inplace_transform.py | 52 +++++++++++++++++++-- 5 files changed, 127 insertions(+), 9 deletions(-) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 63272aa..8727dbf 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -27,6 +27,7 @@ from .capabilities import effective_profile from .dolt_conformance import DoltConformanceSource from .normative import catalog as core_catalog +from .profiles import SqlProfile, preflight from .wide import physical_name, require_verified_catalog from .workflow import TransformationError @@ -435,6 +436,7 @@ def _apply_plan_on_connection( submission: InPlacePlanSubmission, actor: str, database_profile: str, + target_profile: SqlProfile, allow_schema_change: bool, allow_delete_variable: bool, dolt_branch: str | None, @@ -451,6 +453,22 @@ def _apply_plan_on_connection( table_name = _physical_table_name(dataset) plan = submission.plan bound = bind_transformation_plan(plan, schema) + output_used_physical = {"__case_ordinal"} + output_variables = [ + { + "ordinal": source_ordinal, + "source_name": variable.name, + "physical_name": physical_name( + variable.name, output_used_physical, + ), + "storage_kind": variable.storage_kind, + "string_width": variable.declared_string_width, + } + for source_ordinal, variable in enumerate( + bound.output_schema.variables, start=1, + ) + ] + preflight(target_profile, output_variables) output_by_name = { variable.name.casefold(): variable for variable in bound.output_schema.variables @@ -963,6 +981,7 @@ def _run_in_place_submission( submission=submission, actor=actor, database_profile=profile.name, + target_profile=profile, allow_schema_change=profile.name in {"sqlite", "postgresql"}, allow_delete_variable=allow_delete_variable, dolt_branch=branch, diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 69715ba..606c607 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -949,7 +949,12 @@ def create_wide_dataset( except Exception as error: if namespace_owned: try: - with engine.begin() as cleanup: + with _bound_catalog_transaction( + engine=engine, profile_name=profile.name, + active=active_connection, + audit_relations={*audit_relations, data_table.name}, + phase="import cleanup", + ) as cleanup: create_normative_catalog(cleanup, normative) if cleanup.execute( select(normative.dataset.c.dataset_id).where( diff --git a/tests/test_atomic_import.py b/tests/test_atomic_import.py index dc4fbcc..4af61e3 100755 --- a/tests/test_atomic_import.py +++ b/tests/test_atomic_import.py @@ -1,4 +1,5 @@ import sqlite3 +from contextlib import contextmanager from dataclasses import replace import pytest @@ -194,6 +195,55 @@ def test_declared_string_width_preflight_is_atomic_and_diagnostic(tmp_path, monk assert '"reason": "declared_string_width_limit"' in details assert '"string_width": 4' in details +def test_dolt_import_cleanup_uses_bound_catalog_transaction( + tmp_path, monkeypatch, +) -> None: + database_path = tmp_path / "dolt-cleanup.sqlite" + database = f"sqlite:///{database_path}" + active = { + "working_set_binding": { + "database": "test", + "active_branch": "main", + }, + } + monkeypatch.setattr(wide, "effective_profile", lambda _url: (DOLT, active)) + monkeypatch.setattr( + wide, "_capture_dolt_state", lambda *_args, **_kwargs: None, + ) + phases = [] + real_bound = wide._bound_catalog_transaction + + @contextmanager + def capture_bound(**kwargs): + phases.append(kwargs["phase"]) + with real_bound(**kwargs) as connection: + yield connection + + monkeypatch.setattr(wide, "_bound_catalog_transaction", capture_bound) + real_store = wide.store_normative_dataset + + def fail_after_normative_write(*args, **kwargs): + real_store(*args, **kwargs) + raise RuntimeError("fault after Dolt normative write") + + monkeypatch.setattr(wide, "store_normative_dataset", fail_after_normative_write) + variables = [{ + "ordinal": 1, "source_name": "score", "physical_name": "score", + "storage_kind": "numeric", "string_width": None, "label": "", + "format": "F8.0", "measure": "scale", "alignment": "right", + "display_width": 8, "value_labels": "{}", "missing_ranges": "[]", + }] + + with pytest.raises(RuntimeError, match="fault after Dolt normative write"): + create_wide_dataset( + database_url=database, dataset_id="dolt-cleanup", + source_name="fixture.sav", source_format="SAV", + rows=[{"score": 1.0}], variables=variables, + ) + + assert phases[-2:] == ["import", "import cleanup"] + + def test_nonatomic_failure_after_normative_write_cleans_both_catalogs_and_data( tmp_path, monkeypatch, ) -> None: diff --git a/tests/test_conditional_inplace_transform.py b/tests/test_conditional_inplace_transform.py index 8a5657e..f4c62f3 100644 --- a/tests/test_conditional_inplace_transform.py +++ b/tests/test_conditional_inplace_transform.py @@ -2,11 +2,11 @@ import sqlite3 -from types import SimpleNamespace import pytest import openstatspec import openstatspec.sql.inplace_transform as inplace_transform +from openstatspec.sql.profiles import DOLT from openstatspec.sql.wide import create_wide_dataset @@ -254,7 +254,7 @@ def test_dolt_mock_applies_exact_program_to_preexisting_target_without_schema_dd monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name="dolt"), {}), + lambda _url, **_kwargs: (DOLT, {}), ) states = iter([ ("main", "abc123", 0), @@ -291,7 +291,7 @@ def test_dolt_mock_rejects_create_target_before_schema_mutation( monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name="dolt"), {}), + lambda _url, **_kwargs: (DOLT, {}), ) monkeypatch.setattr( inplace_transform, @@ -334,7 +334,7 @@ def test_dolt_mock_rechecks_clean_state_after_dataset_lock( monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name="dolt"), {}), + lambda _url, **_kwargs: (DOLT, {}), ) states = iter([ ("main", "abc123", 0), diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 575babf..29858d5 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -1,6 +1,7 @@ from __future__ import annotations import sqlite3 +from dataclasses import replace from types import SimpleNamespace import pytest @@ -12,6 +13,9 @@ InPlacePlanSubmission, _apply_plan_on_connection, ) +from openstatspec.sql.profiles import ( + DOLT, SQLITE, TargetCapabilityExceededError, +) from openstatspec.sql.wide import create_wide_dataset, validate_wide_dataset @@ -102,6 +106,7 @@ def test_plan_applies_to_same_dataset_and_physical_table_without_copy( ), actor="test-agent", database_profile="sqlite", + target_profile=inplace_transform.effective_profile(url)[0], allow_schema_change=True, allow_delete_variable=True, dolt_branch=None, @@ -318,6 +323,44 @@ def test_public_generic_plan_apply_accepts_object_and_mapping( ) +def test_generic_string_width_is_rejected_before_ddl( + catalog, monkeypatch, +) -> None: + url, path, dataset_id, table_name = catalog + monkeypatch.setattr( + inplace_transform, + "effective_profile", + lambda _url, **_kwargs: ( + replace(SQLITE, max_text_value_bytes=3), + {"server_version": "3.35.0"}, + ), + ) + plan = openstatspec.TransformationPlan(( + openstatspec.CreateVariableOperation("note", "string", 4), + )) + + with pytest.raises(TargetCapabilityExceededError, match="permits 3"): + openstatspec.apply_transformation_plan_in_place( + database_url=url, + dataset_id=dataset_id, + plan=plan, + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert [ + row[1] for row in connection.execute(f'PRAGMA table_info("{table_name}")') + ] == ["__case_ordinal", "score"] + assert connection.execute( + "select source_name from variable where dataset_id = ?", + (dataset_id,), + ).fetchall() == [("score",)] + assert connection.execute( + "select count(*) from transformation_apply" + ).fetchone() == (0,) + connection.close() + + def test_generic_plan_is_bound_to_live_schema_before_mutation(catalog) -> None: url, path, dataset_id, table_name = catalog plan = openstatspec.TransformationPlan(( @@ -481,7 +524,7 @@ def test_delete_is_rejected_when_drop_column_is_unavailable( inplace_transform, "effective_profile", lambda _url, **_kwargs: ( - SimpleNamespace(name="sqlite"), + SQLITE, {"server_version": "3.34.0"}, ), ) @@ -521,6 +564,7 @@ def test_nontransactional_ddl_profile_rejects_create_before_mutation( ), actor="test-agent", database_profile="mysql", + target_profile=inplace_transform.effective_profile(url)[0], allow_schema_change=False, allow_delete_variable=True, dolt_branch=None, @@ -546,7 +590,7 @@ def test_public_apply_binds_expected_dolt_branch_and_head( monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name="dolt"), {}), + lambda _url, **_kwargs: (DOLT, {}), ) states = iter([ ("feature/recode", "abc123", 0), @@ -577,7 +621,7 @@ def test_public_apply_rejects_dirty_dolt_working_set_before_mutation( monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name="dolt"), {}), + lambda _url, **_kwargs: (DOLT, {}), ) monkeypatch.setattr( inplace_transform, @@ -613,7 +657,7 @@ def test_public_apply_rejects_dolt_context_mismatch_before_mutation( monkeypatch.setattr( inplace_transform, "effective_profile", - lambda _url, **_kwargs: (SimpleNamespace(name="dolt"), {}), + lambda _url, **_kwargs: (DOLT, {}), ) monkeypatch.setattr( inplace_transform, "_dolt_state", lambda _connection: state From b0b4b986205ea97477a0f0f97ccdfd36554e22ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 01:36:59 +0300 Subject: [PATCH 21/27] Preserve canonical schema change contracts --- src/openstatspec/__init__.py | 2 +- src/openstatspec/frontends/spss/binding.py | 12 +++- src/openstatspec/sql/inplace_transform.py | 22 +++++++ src/openstatspec/transform/plan.py | 15 ++++- tests/test_inplace_transform.py | 68 +++++++++++++++++++++- tests/test_transform_frontend.py | 23 ++++++++ 6 files changed, 134 insertions(+), 8 deletions(-) diff --git a/src/openstatspec/__init__.py b/src/openstatspec/__init__.py index f021d16..eb73a94 100644 --- a/src/openstatspec/__init__.py +++ b/src/openstatspec/__init__.py @@ -27,7 +27,7 @@ __all__ = [ "AssignOperation", "BooleanExpression", "ComparisonExpression", "ConditionalAssignOperation", "ExecuteOperation", "Operand", - "ConditionalAssignOperation", "CreateVariableOperation", "DeleteVariableOperation", + "PredicateExpression", "CreateVariableOperation", "DeleteVariableOperation", "CapabilityDeclaration", "DoltConformanceSource", "LossReport", "SpssFrontendCompilation", "TransformationError", "TransformationFrontendError", diff --git a/src/openstatspec/frontends/spss/binding.py b/src/openstatspec/frontends/spss/binding.py index 2bd5d87..cd37d12 100644 --- a/src/openstatspec/frontends/spss/binding.py +++ b/src/openstatspec/frontends/spss/binding.py @@ -13,6 +13,7 @@ PredicateExpression, RecodeMatch, RecodeOperation, RecodeResult, RecodeRule, ReplaceValueLabelsOperation, SetFormatOperation, SetMeasurementLevelOperation, SetVariableLabelOperation, + TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT, TRANSFORMATION_PLAN_V1_CONTRACT, TransformationPlan, TypedValue, ValueLabel, ) @@ -491,10 +492,15 @@ def bind_spss_syntax( v01_types = ( RecodeOperation, SetVariableLabelOperation, ReplaceValueLabelsOperation, ) + schema_change_types = (CreateVariableOperation, DeleteVariableOperation) contract = ( - TRANSFORMATION_PLAN_V1_CONTRACT - if all(isinstance(operation, v01_types) for operation in operations) - else "openstatspec-transformation-plan-v0.2" + TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT + if any(isinstance(operation, schema_change_types) for operation in operations) + else ( + TRANSFORMATION_PLAN_V1_CONTRACT + if all(isinstance(operation, v01_types) for operation in operations) + else "openstatspec-transformation-plan-v0.2" + ) ) plan = TransformationPlan( tuple(operations), contract=contract, input_alias=input_alias, diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 8727dbf..04b0b35 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -709,6 +709,28 @@ def _apply_plan_on_connection( _compact_variable_ordinals( connection, core=core, variables=variables, ) + canonical_physical = {"__case_ordinal"} + for remaining_variable in variables: + expected_physical = physical_name( + str(remaining_variable["source_name"]), canonical_physical, + ) + current_physical = str(remaining_variable["physical_name"]) + if current_physical == expected_physical: + continue + connection.exec_driver_sql( + f"ALTER TABLE {qualified_table} RENAME COLUMN " + f"{quote(current_physical)} TO {quote(expected_physical)}" + ) + connection.execute( + update(core.variable) + .where( + core.variable.c.variable_id + == remaining_variable["variable_id"] + ) + .values(physical_name=expected_physical) + ) + remaining_variable["physical_name"] = expected_physical + used_physical = canonical_physical relation = Table( table_name, MetaData(), schema=dataset.get("physical_table_schema"), autoload_with=connection, diff --git a/src/openstatspec/transform/plan.py b/src/openstatspec/transform/plan.py index 7e6b842..74bef7c 100644 --- a/src/openstatspec/transform/plan.py +++ b/src/openstatspec/transform/plan.py @@ -588,6 +588,19 @@ class TransformationPlan: def __post_init__(self) -> None: if not isinstance(self.contract, str) or self.contract not in _TRANSFORMATION_PLAN_CONTRACTS: _invalid("Plan contract is not a supported transformation-plan contract.") + if ( + self.contract != TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT + and any( + isinstance(operation, ( + CreateVariableOperation, DeleteVariableOperation, + )) + for operation in self.operations + ) + ): + _invalid( + "Create/delete schema operations require " + "openstatspec-transformation-plan-v0.3." + ) if self.contract == TRANSFORMATION_PLAN_V1_CONTRACT and any( isinstance(operation, ( AssignOperation, ConditionalAssignOperation, SetFormatOperation, @@ -710,7 +723,7 @@ def _match(raw: Any) -> RecodeMatch: _invalid("Unknown recode match kind.") def transformation_plan_from_dict(raw: Mapping[str, Any]) -> TransformationPlan: - """Strictly validate canonical v0.1 or additive v0.2 plan documents.""" + """Strictly validate canonical v0.1, v0.2, or schema-change v0.3 plans.""" if not isinstance(raw, Mapping): _invalid("Transformation plan must be an object.") _exact(raw, {"contract", "input_alias", "operations"}, "Transformation plan") diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 29858d5..f649b78 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -189,6 +189,67 @@ def test_string_declaration_creates_column_and_catalog_variable(catalog) -> None )["valid"] is True +def test_delete_recanonicalizes_surviving_collision_columns(tmp_path) -> None: + path = tmp_path / "collision-delete.sqlite" + url = f"sqlite:///{path}" + openstatspec.initialize_catalog(database_url=url) + base = _variables()[0] + variables = [ + { + **base, + "ordinal": ordinal, + "source_name": source_name, + "physical_name": physical, + } + for ordinal, (source_name, physical) in enumerate(( + ("a-b", "a_b"), ("a_b", "a_b_2"), ("keep", "keep"), + ), start=1) + ] + create_wide_dataset( + database_url=url, + dataset_id="collision_source", + source_name="collision.sav", + source_format="SAV", + source_sha256="c" * 64, + rows=[ + {"a_b": 1.0, "a_b_2": 2.0, "keep": 3.0}, + {"a_b": 4.0, "a_b_2": 5.0, "keep": 6.0}, + ], + variables=variables, + ) + openstatspec.install_in_place_transformation_schema(database_url=url) + connection = sqlite3.connect(path) + dataset_id = connection.execute( + "select dataset_id from dataset where dataset_name = 'collision_source'" + ).fetchone()[0] + connection.close() + plan = openstatspec.TransformationPlan( + (openstatspec.DeleteVariableOperation("a-b"),), + contract="openstatspec-transformation-plan-v0.3", + ) + + openstatspec.apply_transformation_plan_in_place( + database_url=url, + dataset_id=dataset_id, + plan=plan, + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert connection.execute( + "select source_name, physical_name, source_ordinal from variable " + "where dataset_id = ? order by source_ordinal", + (dataset_id,), + ).fetchall() == [("a_b", "a_b", 1), ("keep", "keep", 2)] + assert connection.execute( + "select a_b, keep from data_collision_source order by __case_ordinal" + ).fetchall() == [(2.0, 3.0), (5.0, 6.0)] + connection.close() + assert validate_wide_dataset( + database_url=url, dataset_id=dataset_id, + )["valid"] is True + + def test_delete_prunes_an_empty_multiple_response_set(catalog) -> None: url, path, dataset_id, _table_name = catalog connection = sqlite3.connect(path) @@ -335,9 +396,10 @@ def test_generic_string_width_is_rejected_before_ddl( {"server_version": "3.35.0"}, ), ) - plan = openstatspec.TransformationPlan(( - openstatspec.CreateVariableOperation("note", "string", 4), - )) + plan = openstatspec.TransformationPlan( + (openstatspec.CreateVariableOperation("note", "string", 4),), + contract="openstatspec-transformation-plan-v0.3", + ) with pytest.raises(TargetCapabilityExceededError, match="permits 3"): openstatspec.apply_transformation_plan_in_place( diff --git a/tests/test_transform_frontend.py b/tests/test_transform_frontend.py index 0968e72..76e6b5d 100644 --- a/tests/test_transform_frontend.py +++ b/tests/test_transform_frontend.py @@ -13,6 +13,7 @@ spss_source_hash, ) from openstatspec.transform import ( + CreateVariableOperation, RecodeMatch, RecodeOperation, RecodeResult, @@ -559,6 +560,28 @@ def test_v02_plan_and_schema_reject_decimal_format_that_cannot_fit() -> None: ) +def test_schema_operations_require_v03_contract() -> None: + operation = CreateVariableOperation("note", "string", 8) + with pytest.raises(TransformationFrontendError) as caught: + TransformationPlan((operation,)) + assert caught.value.code == "invalid_transformation_plan" + + plan = TransformationPlan( + (operation,), contract="openstatspec-transformation-plan-v0.3", + ) + assert plan.contract == "openstatspec-transformation-plan-v0.3" + + +def test_spss_schema_commands_emit_v03_contract() -> None: + schema = _schema(VariableDefinition("q1", "numeric")) + assert bind_spss_syntax( + parse_spss_syntax("STRING note (A8)."), schema, + ).plan.contract == "openstatspec-transformation-plan-v0.3" + assert bind_spss_syntax( + parse_spss_syntax("COMPUTE other = q1. DELETE VARIABLES q1."), schema, + ).plan.contract == "openstatspec-transformation-plan-v0.3" + + def test_custom_nonempty_input_alias_is_canonical() -> None: plan = bind_spss_syntax( parse_spss_syntax("VARIABLE LABELS q1 'One'."), From ecebe50ba8d7e7644a8506b5bc545d837717506a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 01:55:42 +0300 Subject: [PATCH 22/27] Reject foreign views and prune variable sets --- src/openstatspec/sql/inplace_transform.py | 15 +++++++ src/openstatspec/sql/wide.py | 21 ++++++---- tests/test_atomic_import.py | 38 +++++++++++++++++- tests/test_inplace_transform.py | 48 ++++++++++++++++++++++- 4 files changed, 112 insertions(+), 10 deletions(-) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index 04b0b35..b9b7a84 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -316,6 +316,11 @@ def _delete_variable_metadata( ) -> None: """Delete one variable and every normative catalog row owned by it.""" variable_id = str(variable["variable_id"]) + variable_set_ids = list(connection.execute( + select(core.variable_set_member.c.variable_set_id).where( + core.variable_set_member.c.variable_id == variable_id + ) + ).scalars()) response_set_ids = list(connection.execute( select(core.multiple_response_member.c.multiple_response_set_id).where( core.multiple_response_member.c.variable_id == variable_id @@ -332,6 +337,16 @@ def _delete_variable_metadata( connection.execute(delete(core.variable_set_member).where( core.variable_set_member.c.variable_id == variable_id )) + for variable_set_id in variable_set_ids: + remaining_member = connection.execute( + select(core.variable_set_member.c.variable_id).where( + core.variable_set_member.c.variable_set_id == variable_set_id + ) + ).first() + if remaining_member is None: + connection.execute(delete(core.variable_set).where( + core.variable_set.c.variable_set_id == variable_set_id + )) connection.execute(delete(core.multiple_response_member).where( core.multiple_response_member.c.variable_id == variable_id )) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index 606c607..cfa75d8 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -813,14 +813,18 @@ def create_wide_dataset( engine=engine, profile_name=profile.name, active=active_connection, audit_relations=audit_relations, phase="catalog initialization", ) as catalog_connection: - catalog_existed = inspect(catalog_connection).has_table( - normative.catalog_identity.name - ) - create_normative_catalog(catalog_connection, normative) - if catalog_existed: - require_verified_catalog(catalog_connection) + inspector = inspect(catalog_connection) + existing_tables = set(inspector.get_table_names()) + existing_views = set(inspector.get_view_names()) + if existing_tables or existing_views: + if normative.catalog_identity.name not in existing_tables: + raise UnsupportedOperationError( + "The selected database catalog is foreign; " + "import is not permitted." + ) else: - _verify_normative_catalog(catalog_connection, normative) + create_normative_catalog(catalog_connection, normative) + require_verified_catalog(catalog_connection) operation_id = str(uuid4()) normative_dataset_id = str(uuid4()) fidelity_events = tuple(fidelity_events) @@ -873,12 +877,13 @@ def create_wide_dataset( data_table_created = False try: with engine.begin() as setup: - _verify_normative_catalog(setup, normative) + require_verified_catalog(setup) namespace_owned = True with _bound_catalog_transaction( engine=engine, profile_name=profile.name, active=active_connection, audit_relations={*audit_relations, data_table.name}, phase="import", ) as connection: + require_verified_catalog(connection) if connection.execute( select(normative.dataset.c.dataset_id).where(or_( normative.dataset.c.dataset_name == dataset_id, diff --git a/tests/test_atomic_import.py b/tests/test_atomic_import.py index 4af61e3..95bdb5a 100755 --- a/tests/test_atomic_import.py +++ b/tests/test_atomic_import.py @@ -50,6 +50,9 @@ def inspect_with_concurrent_table(bind): actual = real_inspect(bind) class Inspector: + def __getattr__(self, name): + return getattr(actual, name) + def has_table(self, table_name): nonlocal inspected_data_table if table_name == "data_race": @@ -298,6 +301,39 @@ def fail_after_normative_write(*args, **kwargs): "select direction, severity, event_code, dataset_id from fidelity_event" ).fetchall() == [("import", "error", "import_failed", None)] +def test_occupied_foreign_view_namespace_fails_without_modification( + tmp_path, +) -> None: + database_path = tmp_path / "foreign-view.sqlite" + database = f"sqlite:///{database_path}" + connection = sqlite3.connect(database_path) + connection.execute("create view foreign_view as select 1 as value") + before_schema = connection.execute( + "select name, type, sql from sqlite_master order by name" + ).fetchall() + connection.close() + variables = [{ + "ordinal": 1, "source_name": "score", "physical_name": "score", + "storage_kind": "numeric", "string_width": None, "label": "", + "format": "F8.0", "measure": "scale", "alignment": "right", + "display_width": 8, "value_labels": "{}", "missing_ranges": "[]", + }] + + with pytest.raises(wide.UnsupportedOperationError, match="foreign"): + create_wide_dataset( + database_url=database, dataset_id="foreign-view", + source_name="fixture.sav", source_format="SAV", + rows=[{"score": 1.0}], variables=variables, + ) + + connection = sqlite3.connect(database_path) + assert connection.execute( + "select name, type, sql from sqlite_master order by name" + ).fetchall() == before_schema + assert connection.execute("select value from foreign_view").fetchall() == [(1,)] + connection.close() + + def test_occupied_foreign_namespace_fails_without_modification(tmp_path) -> None: database_path = tmp_path / "foreign.sqlite" database = f"sqlite:///{database_path}" @@ -315,7 +351,7 @@ def test_occupied_foreign_namespace_fails_without_modification(tmp_path) -> None "format": "A8", "measure": "nominal", "alignment": "left", "display_width": 8, "value_labels": "{}", "missing_ranges": "[]", }] - with pytest.raises(RuntimeError, match="occupied"): + with pytest.raises(RuntimeError, match="foreign"): create_wide_dataset( database_url=database, dataset_id="foreign", source_name="fixture.sav", source_format="SAV", rows=[{"name": "ok"}], variables=variables, diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index f649b78..7153f7b 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -16,7 +16,9 @@ from openstatspec.sql.profiles import ( DOLT, SQLITE, TargetCapabilityExceededError, ) -from openstatspec.sql.wide import create_wide_dataset, validate_wide_dataset +from openstatspec.sql.wide import ( + create_wide_dataset, read_wide_dataset, validate_wide_dataset, +) def _variables() -> list[dict[str, object]]: @@ -250,6 +252,50 @@ def test_delete_recanonicalizes_surviving_collision_columns(tmp_path) -> None: )["valid"] is True +def test_delete_prunes_an_empty_spss_variable_set(catalog) -> None: + url, path, dataset_id, _table_name = catalog + connection = sqlite3.connect(path) + variable_id = connection.execute( + "select variable_id from variable where dataset_id = ?", + (dataset_id,), + ).fetchone()[0] + variable_set_id = "00000000-0000-0000-0000-000000000002" + connection.execute( + "insert into variable_set " + "(variable_set_id, dataset_id, source_ordinal, set_name) " + "values (?, ?, 1, 'scores')", + (variable_set_id, dataset_id), + ) + connection.execute( + "insert into variable_set_member " + "(variable_set_id, variable_id, source_ordinal) values (?, ?, 1)", + (variable_set_id, variable_id), + ) + connection.commit() + connection.close() + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text="COMPUTE other = score. DELETE VARIABLES score.", + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert connection.execute( + "select count(*) from variable_set where dataset_id = ?", + (dataset_id,), + ).fetchone() == (0,) + assert connection.execute( + "select count(*) from variable_set_member" + ).fetchone() == (0,) + connection.close() + dataset, _variables, _rows = read_wide_dataset( + database_url=url, dataset_id=dataset_id, + ) + assert dataset["source_extensions"] == {} + + def test_delete_prunes_an_empty_multiple_response_set(catalog) -> None: url, path, dataset_id, _table_name = catalog connection = sqlite3.connect(path) From dd7b75c6c52be438f094333ec4795de3459ffa7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 02:11:44 +0300 Subject: [PATCH 23/27] Handle transient schema create targets --- src/openstatspec/sql/inplace_transform.py | 27 ++++++++----- tests/test_inplace_transform.py | 49 +++++++++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/openstatspec/sql/inplace_transform.py b/src/openstatspec/sql/inplace_transform.py index b9b7a84..06ffecc 100644 --- a/src/openstatspec/sql/inplace_transform.py +++ b/src/openstatspec/sql/inplace_transform.py @@ -484,10 +484,6 @@ def _apply_plan_on_connection( ) ] preflight(target_profile, output_variables) - output_by_name = { - variable.name.casefold(): variable - for variable in bound.output_schema.variables - } audit = apply_audit_catalog(MetaData()) if not inspect(connection).has_table("transformation_apply"): raise TransformationError( @@ -528,11 +524,24 @@ def _apply_plan_on_connection( "delete_variable_not_supported", "This SQLite runtime does not support ALTER TABLE DROP COLUMN.", ) - unsupported_targets = [ - operation.target for operation in create_operations - if not isinstance(operation, CreateVariableOperation) - and output_by_name[operation.target.casefold()].storage_kind != "numeric" - ] + unsupported_targets: list[str] = [] + for operation_index, operation in enumerate(plan.operations): + if not isinstance(operation, (RecodeOperation, AssignOperation)): + continue + if operation.target_mode != "create": + continue + prefix = TransformationPlan( + plan.operations[:operation_index + 1], + contract=plan.contract, + input_alias=plan.input_alias, + ) + prefix_output = bind_transformation_plan(prefix, schema).output_schema + created = next( + variable for variable in prefix_output.variables + if variable.name.casefold() == operation.target.casefold() + ) + if created.storage_kind != "numeric": + unsupported_targets.append(operation.target) if unsupported_targets: raise TransformationError( "in_place_target_type_unsupported", diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 7153f7b..31d1bdb 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -374,6 +374,55 @@ def test_delete_then_recreate_same_name_resolves_operations_in_order(catalog) -> )["valid"] is True +def test_temporary_created_target_can_be_deleted_before_final_schema(catalog) -> None: + url, path, dataset_id, table_name = catalog + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text="COMPUTE tmp = score. DELETE VARIABLES tmp.", + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert connection.execute( + "select source_name, source_ordinal from variable " + "where dataset_id = ? order by source_ordinal", + (dataset_id,), + ).fetchall() == [("score", 1)] + assert [ + row[1] for row in connection.execute(f'PRAGMA table_info("{table_name}")') + ] == ["__case_ordinal", "score"] + connection.close() + + +def test_temporary_target_type_is_not_taken_from_same_name_recreation(catalog) -> None: + url, path, dataset_id, table_name = catalog + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text=( + "COMPUTE tmp = score. DELETE VARIABLES tmp. STRING tmp (A4)." + ), + actor="test-agent", + ) + + connection = sqlite3.connect(path) + assert connection.execute( + "select source_name, storage_kind, declared_string_width " + "from variable where dataset_id = ? order by source_ordinal", + (dataset_id,), + ).fetchall() == [("score", "numeric", None), ("tmp", "string", 4)] + assert connection.execute( + f'SELECT score, tmp FROM "{table_name}" ORDER BY __case_ordinal' + ).fetchall() == [(1.0, ""), (2.0, ""), (3.0, "")] + connection.close() + assert validate_wide_dataset( + database_url=url, dataset_id=dataset_id, + )["valid"] is True + + def test_public_apply_supports_non_dolt_without_building_undo(catalog) -> None: url, path, dataset_id, table_name = catalog plan = _plan("RECODE score (1 = 0).") From be1ed43fccc862ad00c25401b0c76786b2193247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 02:22:06 +0300 Subject: [PATCH 24/27] Restore transform star exports --- src/openstatspec/transform/__init__.py | 5 +++-- tests/test_transform_frontend.py | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/openstatspec/transform/__init__.py b/src/openstatspec/transform/__init__.py index d2df0ea..9cd2b62 100644 --- a/src/openstatspec/transform/__init__.py +++ b/src/openstatspec/transform/__init__.py @@ -20,14 +20,15 @@ __all__ = [ "AssignOperation", "BooleanExpression", "ComparisonExpression", "ConditionalAssignOperation", "CreateVariableOperation", "DeleteVariableOperation", - "PredicateExpression", + "ExecuteOperation", "Operand", "PredicateExpression", "BoundTransformation", "RecodeMatch", "RecodeOperation", "RecodeResult", "RecodeRule", "ReplaceValueLabelsOperation", "SPSS_FRONTEND_CONTRACT", "SetFormatOperation", "SetMeasurementLevelOperation", "SetVariableLabelOperation", "SourcePosition", "SourceSpan", "StorageKind", "SpssFrontendCompilation", - "SpssSyntaxProgram", "TRANSFORMATION_PLAN_CONTRACT", "TransformationFrontendError", + "SpssSyntaxProgram", "TRANSFORMATION_PLAN_CONTRACT", + "TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT", "TransformationFrontendError", "TransformationPlan", "TypedValue", "ValueLabel", "VariableDefinition", "VariableSchema", "bind_spss_syntax", "bind_transformation_plan", diff --git a/tests/test_transform_frontend.py b/tests/test_transform_frontend.py index 76e6b5d..b9d511b 100644 --- a/tests/test_transform_frontend.py +++ b/tests/test_transform_frontend.py @@ -5,6 +5,7 @@ from pathlib import Path import pytest +import openstatspec.transform as transform_module from openstatspec.frontends.spss import ( bind_spss_syntax, compile_spss_syntax, @@ -32,6 +33,14 @@ ) +def test_transform_star_exports_keep_public_plan_nodes() -> None: + assert { + "ExecuteOperation", + "Operand", + "TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT", + }.issubset(transform_module.__all__) + + def _schema(*variables: VariableDefinition) -> VariableSchema: return VariableSchema(tuple(variables)) From d8716585fcbdfe46fc125944c5c55aa4ec2feebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 02:35:20 +0300 Subject: [PATCH 25/27] Version schema-changing SPSS frontend submissions --- src/openstatspec/frontends/spss/__init__.py | 10 +++++++--- src/openstatspec/frontends/spss/compiler.py | 15 ++++++++++++++- src/openstatspec/frontends/spss/execution.py | 3 +-- src/openstatspec/transform/__init__.py | 2 ++ tests/test_inplace_transform.py | 19 +++++++++++++++++++ tests/test_transform_frontend.py | 11 +++++++++++ 6 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/openstatspec/frontends/spss/__init__.py b/src/openstatspec/frontends/spss/__init__.py index 18a7891..58ed87e 100644 --- a/src/openstatspec/frontends/spss/__init__.py +++ b/src/openstatspec/frontends/spss/__init__.py @@ -1,7 +1,12 @@ """SPSS syntax frontend for canonical OpenStatSpec transformation plans.""" from .binding import bind_spss_syntax -from .compiler import SpssFrontendCompilation, compile_spss_syntax +from .compiler import ( + SPSS_FRONTEND_CONTRACT, + SPSS_FRONTEND_SCHEMA_CHANGE_CONTRACT, + SpssFrontendCompilation, + compile_spss_syntax, +) from .syntax import ( SpssSyntaxProgram, normalize_spss_source, @@ -11,10 +16,9 @@ ) -SPSS_FRONTEND_CONTRACT = "openstatspec-spss-syntax-frontend-v0.2" - __all__ = [ "SPSS_FRONTEND_CONTRACT", + "SPSS_FRONTEND_SCHEMA_CHANGE_CONTRACT", "SpssFrontendCompilation", "SpssSyntaxProgram", "bind_spss_syntax", diff --git a/src/openstatspec/frontends/spss/compiler.py b/src/openstatspec/frontends/spss/compiler.py index 4e003c0..ecd05f2 100644 --- a/src/openstatspec/frontends/spss/compiler.py +++ b/src/openstatspec/frontends/spss/compiler.py @@ -4,7 +4,10 @@ from dataclasses import dataclass -from ...transform.plan import TransformationPlan +from ...transform.plan import ( + TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT, + TransformationPlan, +) from ...transform.schema import BoundTransformation, VariableSchema from .binding import bind_spss_syntax from .syntax import ( @@ -14,6 +17,10 @@ ) +SPSS_FRONTEND_CONTRACT = "openstatspec-spss-syntax-frontend-v0.2" +SPSS_FRONTEND_SCHEMA_CHANGE_CONTRACT = "openstatspec-spss-syntax-frontend-v0.3" + + @dataclass(frozen=True) class SpssFrontendCompilation: """One source artifact and its fully bound canonical plan.""" @@ -30,6 +37,12 @@ def plan(self) -> TransformationPlan: def plan_hash(self) -> str: return self.plan.sha256() + @property + def frontend_contract(self) -> str: + if self.plan.contract == TRANSFORMATION_PLAN_SCHEMA_CHANGE_CONTRACT: + return SPSS_FRONTEND_SCHEMA_CHANGE_CONTRACT + return SPSS_FRONTEND_CONTRACT + def compile_spss_syntax( source: str, diff --git a/src/openstatspec/frontends/spss/execution.py b/src/openstatspec/frontends/spss/execution.py index 1d947da..108bed0 100644 --- a/src/openstatspec/frontends/spss/execution.py +++ b/src/openstatspec/frontends/spss/execution.py @@ -10,7 +10,6 @@ _run_in_place_submission, load_transformation_schema, ) -from . import SPSS_FRONTEND_CONTRACT from .compiler import compile_spss_syntax @@ -37,7 +36,7 @@ def prepare(connection: Any, live_dataset_id: str) -> InPlacePlanSubmission: plan=compilation.plan, source_kind="spss_syntax", source_hash=compilation.source_hash, - frontend_contract=SPSS_FRONTEND_CONTRACT, + frontend_contract=compilation.frontend_contract, ) return _run_in_place_submission( diff --git a/src/openstatspec/transform/__init__.py b/src/openstatspec/transform/__init__.py index 9cd2b62..bf220bd 100644 --- a/src/openstatspec/transform/__init__.py +++ b/src/openstatspec/transform/__init__.py @@ -24,6 +24,7 @@ "BoundTransformation", "RecodeMatch", "RecodeOperation", "RecodeResult", "RecodeRule", "ReplaceValueLabelsOperation", "SPSS_FRONTEND_CONTRACT", + "SPSS_FRONTEND_SCHEMA_CHANGE_CONTRACT", "SetFormatOperation", "SetMeasurementLevelOperation", "SetVariableLabelOperation", "SourcePosition", "SourceSpan", "StorageKind", "SpssFrontendCompilation", @@ -42,6 +43,7 @@ _SPSS_COMPAT_EXPORTS = { "SPSS_FRONTEND_CONTRACT", + "SPSS_FRONTEND_SCHEMA_CHANGE_CONTRACT", "SpssFrontendCompilation", "SpssSyntaxProgram", "bind_spss_syntax", diff --git a/tests/test_inplace_transform.py b/tests/test_inplace_transform.py index 31d1bdb..cd97771 100644 --- a/tests/test_inplace_transform.py +++ b/tests/test_inplace_transform.py @@ -447,6 +447,25 @@ def test_public_apply_supports_non_dolt_without_building_undo(catalog) -> None: ) +def test_schema_commands_record_the_v03_frontend_contract(catalog) -> None: + url, path, dataset_id, _table_name = catalog + + openstatspec.apply_spss_in_place( + database_url=url, + dataset_id=dataset_id, + source_text="STRING note (A4).", + actor="test-agent", + ) + + audit = sqlite3.connect(path).execute( + "SELECT source_kind, frontend_contract FROM transformation_apply" + ).fetchone() + assert audit == ( + "spss_syntax", + "openstatspec-spss-syntax-frontend-v0.3", + ) + + @pytest.mark.parametrize("as_mapping", [False, True]) def test_public_generic_plan_apply_accepts_object_and_mapping( catalog, as_mapping, diff --git a/tests/test_transform_frontend.py b/tests/test_transform_frontend.py index b9d511b..0caeee0 100644 --- a/tests/test_transform_frontend.py +++ b/tests/test_transform_frontend.py @@ -368,6 +368,17 @@ def test_source_normalization_hash_and_positions_are_stable() -> None: assert compilation.source_text_lf == lf assert compilation.source_hash == spss_source_hash(lf) assert compilation.plan_hash == compilation.plan.sha256() + assert compilation.frontend_contract == "openstatspec-spss-syntax-frontend-v0.2" + + +def test_schema_commands_use_the_v03_frontend_contract() -> None: + compilation = compile_spss_syntax( + "STRING note (A4).", + _schema(VariableDefinition("q1", "numeric")), + ) + + assert compilation.plan.contract == "openstatspec-transformation-plan-v0.3" + assert compilation.frontend_contract == "openstatspec-spss-syntax-frontend-v0.3" def test_string_comparison_fails_closed_until_exact_collation_is_supported() -> None: error = _error( From d4f2ec0120330bee3e5b6f47ca6aa8f25bffb5dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 02:49:24 +0300 Subject: [PATCH 26/27] Verify catalog before failed import audits --- src/openstatspec/sql/wide.py | 2 +- tests/test_catalog_persistence_review.py | 40 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index cfa75d8..d76971e 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -97,7 +97,7 @@ def _record_failed_preflight( audit_relations={normative.operation.name, normative.fidelity_event.name}, phase="import preflight failure", ) as connection: - _verify_normative_catalog(connection, normative) + require_verified_catalog(connection) failed_at = datetime.now(UTC).replace(tzinfo=None) record_normative_operation( connection, normative, operation_id=operation_id, diff --git a/tests/test_catalog_persistence_review.py b/tests/test_catalog_persistence_review.py index 2afdcc4..c1d50f5 100644 --- a/tests/test_catalog_persistence_review.py +++ b/tests/test_catalog_persistence_review.py @@ -124,6 +124,46 @@ def observe(**kwargs): assert phases == ["catalog initialization", "import preflight failure"] +def test_failed_import_preflight_rejects_catalog_drift_before_audit( + tmp_path, monkeypatch, +): + path = tmp_path / "failed-preflight-drift.sqlite" + database_url = f"sqlite:///{path}" + real_bound_transaction = wide._bound_catalog_transaction + + @contextmanager + def inject_drift(**kwargs): + if kwargs["phase"] == "import preflight failure": + connection = sqlite3.connect(path) + connection.execute("create view foreign_view as select 1 as value") + connection.commit() + connection.close() + with real_bound_transaction(**kwargs) as connection: + yield connection + + monkeypatch.setattr(wide, "_bound_catalog_transaction", inject_drift) + variables = _variables() + variables[0].update({ + "storage_kind": "numeric", "string_width": None, + "format": "F8.2", "alignment": "right", + }) + + with pytest.raises(UnsupportedOperationError, match="foreign"): + wide.create_wide_dataset( + database_url=database_url, + dataset_id="invalid", + source_name="invalid.sav", + source_format="SAV", + rows=[{"name": "not-a-number"}], + variables=variables, + ) + + connection = sqlite3.connect(path) + assert connection.execute("select count(*) from operation").fetchone() == (0,) + assert connection.execute("select count(*) from fidelity_event").fetchone() == (0,) + connection.close() + + def test_export_audit_mutations_route_through_binding_guard(tmp_path, monkeypatch): database_url = f"sqlite:///{tmp_path / 'bound-export.sqlite'}" _create(database_url) From 3e55e5179c102875f0e75f845a3f10476628a08f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B5nis=20Ormisson?= Date: Wed, 5 Aug 2026 03:02:10 +0300 Subject: [PATCH 27/27] Reject Dolt audit relation schema diffs --- src/openstatspec/sql/wide.py | 22 +++++-- tests/test_review_transaction_boundary.py | 75 +++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/src/openstatspec/sql/wide.py b/src/openstatspec/sql/wide.py index d76971e..bd8488b 100644 --- a/src/openstatspec/sql/wide.py +++ b/src/openstatspec/sql/wide.py @@ -509,6 +509,21 @@ def _capture_dolt_state( ), "diff_summaries": {}, } + + def allowed_audit_data_diff(row: Mapping[str, Any]) -> bool: + values = dict(row) + from_name = str(values.get("from_table_name") or "") + to_name = str(values.get("to_table_name") or "") + data_change = str(values.get("data_change")).strip().lower() + schema_change = str(values.get("schema_change")).strip().lower() + return ( + from_name == to_name + and from_name in audit_relations + and str(values.get("diff_type") or "").strip().lower() == "modified" + and data_change in {"1", "true"} + and schema_change in {"0", "false"} + ) + for label, left, right in ( ("head_to_working", "HEAD", "WORKING"), ("head_to_staged", "HEAD", "STAGED"), @@ -530,12 +545,7 @@ def _capture_dolt_state( unrelated["diff_summaries"][label] = _dolt_evidence_block( ( row for row in rows - if not { - str(value) for value in ( - dict(row).get("from_table_name"), - dict(row).get("to_table_name"), - ) if value - } <= audit_relations + if not allowed_audit_data_diff(row) ), expected_keys=expected_keys, ) diff --git a/tests/test_review_transaction_boundary.py b/tests/test_review_transaction_boundary.py index 413bfc1..2fe5165 100644 --- a/tests/test_review_transaction_boundary.py +++ b/tests/test_review_transaction_boundary.py @@ -57,6 +57,81 @@ def capture(connection, **_kwargs): assert transaction_states == [False, True] +class _DoltResult: + def __init__(self, rows): + self.rows = rows + + def mappings(self): + return self + + def one(self): + return self.rows[0] + + def all(self): + return self.rows + + +class _DoltDiffConnection: + def __init__(self, summary): + self.summary = summary + + def exec_driver_sql(self, statement): + if "ACTIVE_BRANCH" in statement: + return _DoltResult([{ + "database_name": "catalog", + "active_branch": "main", + "head_hash": "abc123", + }]) + if "dolt_status" in statement: + return _DoltResult([]) + return _DoltResult([self.summary]) + + +def _dolt_summary(**changes): + result = { + "from_table_name": "operation", + "to_table_name": "operation", + "diff_type": "modified", + "data_change": 1, + "schema_change": 0, + } + result.update(changes) + return result + + +def test_dolt_classifier_allows_only_same_table_audit_data_changes(): + allowed = wide._capture_dolt_state( + _DoltDiffConnection(_dolt_summary()), + profile_name="dolt", + audit_relations={"operation", "fidelity_event"}, + ) + assert allowed is not None + assert all( + not evidence["rows"] + for evidence in allowed["unrelated_working_set"]["diff_summaries"].values() + ) + + for unsafe in ( + _dolt_summary(schema_change=1), + _dolt_summary(diff_type="dropped", to_table_name=None, schema_change=1), + _dolt_summary( + diff_type="renamed", to_table_name="fidelity_event", schema_change=1, + ), + ): + classified = wide._capture_dolt_state( + _DoltDiffConnection(unsafe), + profile_name="dolt", + audit_relations={"operation", "fidelity_event"}, + ) + assert classified is not None + assert all( + evidence["rows"] == [unsafe] + for evidence in classified[ + "unrelated_working_set" + ]["diff_summaries"].values() + ) + + def test_dolt_completion_rejects_unrelated_working_set_changes(): before = { "database": "catalog", "active_branch": "main", "head": "abc123",