diff --git a/changelog.d/24.added.md b/changelog.d/24.added.md new file mode 100644 index 0000000..025dbb9 --- /dev/null +++ b/changelog.d/24.added.md @@ -0,0 +1 @@ +Contract the rebuilt column layer. The column-expression and protocol contracts are validated against each backend's own `ColumnTypeSupport` tables: an explicit declaration is backend-independent, inference is compared with the backend's `suggested_column_types()` answer, and a backend refusal (`None`) skips rather than fails. New guarantees pin the completeness of the common entries (a canonical `COMMON_TYPES` list now lives in `column_helpers.py`), class-or-`None` answers, constructibility, the absence of a value-family label, and the `UseColumnType` escape hatch past a refusal. Core-level `Model.c` operation gates exercise the surface through the field proxy: each value family's operations are reachable and produce expressions, and the ones a family does not offer refuse — `AttributeError` for a missing attribute, `TypeError` where the interpreter itself refuses a family that never declared arithmetic. diff --git a/changelog.d/24.changed.md b/changelog.d/24.changed.md new file mode 100644 index 0000000..37d90c7 --- /dev/null +++ b/changelog.d/24.changed.md @@ -0,0 +1 @@ +Sync the contracts with the core's DataType accessors — `column_type()` becomes `column_data_type()` and `columns_type()` becomes `columns_data_type()`, and a new declaration-accessor contract pins the `UseColumnType` marker's identity and `.column_class`, the `None` answer for a field that declares nothing, and the `TypeError` for two declarations on one field. Unify the unknown-field ruling across the accessor family: a field the model does not have is a mistake in the call rather than an answer, so every field accessor raises `KeyError` for it and an explicitly named unknown field fails the batch accessor the same way, while `None` (or an empty collection) remains the answer for a real field that declares nothing. Follow the core's `DateTimeColumn` → `TimestampColumn` rename, migrate the shared fixtures off the bare-`Column` doctrine (expressions are built from objects — a `FunctionCall` where a parameter wraps one, a `TableExpression` for parameters annotated as one, `ArrayType`'s required `element_type` supplied), and update the `typed_column` README. diff --git a/changelog.d/24.fixed.md b/changelog.d/24.fixed.md new file mode 100644 index 0000000..af33cd5 --- /dev/null +++ b/changelog.d/24.fixed.md @@ -0,0 +1 @@ +Keep the pool's per-test slot release off the POSIX locking module. `release_all` imported `fcntl` at function entry, so a serial run on Windows failed every test with `ModuleNotFoundError` even though no slot lock had ever been taken — slot locks are only ever taken by `_acquire_slot`, reachable exclusively while pooling is active. The teardown hook now returns before the import when the worker holds no slot, with regression tests covering both the serial-run path and the POSIX-only locked path. diff --git a/src/rhosocial/activerecord/testsuite/core/pool.py b/src/rhosocial/activerecord/testsuite/core/pool.py index 1efd9c9..a2f4187 100644 --- a/src/rhosocial/activerecord/testsuite/core/pool.py +++ b/src/rhosocial/activerecord/testsuite/core/pool.py @@ -276,12 +276,24 @@ def release_all() -> None: Called when a test finishes so the freed slots are immediately available to the next test on any worker. - """ - import fcntl + Returns immediately when the worker holds no slot. Slot locks are only ever + taken by :func:`_acquire_slot`, which is reachable exclusively through the + ``pooled_database_*`` helpers and therefore only while :func:`pooling_active` + holds. A serial run (no ``-n``) never opens a lock file, so it must not touch + the platform-specific locking module either: ``fcntl`` exists only on POSIX, + and importing it unconditionally made ``pytest_sessionfinish`` fail on Windows + with ``ModuleNotFoundError: No module named 'fcntl'`` even though no lock was + ever held. + """ with _GUARD: held = list(_HELD.items()) _HELD.clear() + if not held: + return + + import fcntl + for scenario, index in held: fh = _OPEN_LOCKS.pop((scenario, index), None) if fh is None: diff --git a/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field.py b/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field.py index dacfd12..f4684a3 100644 --- a/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field.py +++ b/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field.py @@ -6,7 +6,7 @@ from rhosocial.activerecord.base import DerivedField, UseColumn from rhosocial.activerecord.model import ActiveRecord -from rhosocial.activerecord.backend.expression import Column, Literal +from rhosocial.activerecord.backend.expression import Literal, NumericColumn class TestDerivedFieldDeclaration: @@ -27,7 +27,7 @@ def test_descriptor_class_access(self, product_class): def test_derived_field_with_static_expression(self, product_class): """Test DerivedField constructed with a pre-built expression object (non-callable path).""" dialect = product_class.backend().dialect - static_expr = Column(dialect, "price") * Literal(dialect, 2) + static_expr = NumericColumn(dialect, "price") * Literal(dialect, 2) df = DerivedField(static_expr) resolved = df.resolve(dialect) assert resolved is static_expr, \ @@ -182,7 +182,7 @@ def test_derived_dict_with_lambda(self, product_class): """A lambda-based derived dict should evaluate against the dialect.""" self._insert(product_class, "D1", 100.0, 4) results = product_class.find_all( - derived={"my_discount": lambda d: Column(d, "price") * Literal(d, 0.8)} + derived={"my_discount": lambda d: NumericColumn(d, "price") * Literal(d, 0.8)} ) assert results[0].__dict__["my_discount"] == pytest.approx(80.0), \ "Expected my_discount to be 80.0" @@ -202,7 +202,7 @@ def test_derived_dict_with_expression_object(self, product_class): """A derived dict should accept a pre-built expression object directly.""" self._insert(product_class, "D3", 80.0, 5) dialect = product_class.backend().dialect - expr = Column(dialect, "price") * Literal(dialect, 0.5) + expr = NumericColumn(dialect, "price") * Literal(dialect, 0.5) results = product_class.find_all( derived={"half_price": expr} ) @@ -239,7 +239,7 @@ def test_extra_derived_basic(self, product_class): """extra_derived should add new derived fields without disturbing declared ones.""" self._insert(product_class, "F", 100.0, 10) results = product_class.find_all( - extra_derived={"triple_price": lambda d: Column(d, "price") * Literal(d, 3)} + extra_derived={"triple_price": lambda d: NumericColumn(d, "price") * Literal(d, 3)} ) assert len(results) == 1, "Expected 1 record to be returned" assert results[0].__dict__["triple_price"] == pytest.approx(300.0), \ @@ -250,7 +250,7 @@ def test_extra_derived_conflict_raises(self, product_class): self._insert(product_class, "G", 50.0, 2) with pytest.raises(ValueError, match="conflicts with a declared derived field"): product_class.find_all( - extra_derived={"discounted_price": lambda d: Column(d, "price") * Literal(d, 0.5)} + extra_derived={"discounted_price": lambda d: NumericColumn(d, "price") * Literal(d, 0.5)} ) def test_derived_and_extra_derived_together(self, product_class): @@ -258,7 +258,7 @@ def test_derived_and_extra_derived_together(self, product_class): self._insert(product_class, "H", 40.0, 5) results = product_class.find_all( derived=True, - extra_derived={"double_qty": lambda d: Column(d, "quantity") * Literal(d, 2)} + extra_derived={"double_qty": lambda d: NumericColumn(d, "quantity") * Literal(d, 2)} ) assert results[0].discounted_price == pytest.approx(36.0), \ "Expected discounted_price to be 36.0" @@ -397,7 +397,7 @@ class ConflictModel(ActiveRecord): id: Optional[int] = None discount_rate: Annotated[float, UseColumn("disc")] discounted: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ), UseColumn("disc")]] def test_use_column_no_conflict_different_names(self): @@ -407,7 +407,7 @@ class NoConflict(ActiveRecord): id: Optional[int] = None discount_rate: Annotated[float, UseColumn("rate")] discounted: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ), UseColumn("disc")]] assert NoConflict.__derived_fields__["discounted"].column_name == "disc", \ diff --git a/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field_async.py b/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field_async.py index 00ebfee..0ff9181 100644 --- a/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field_async.py +++ b/src/rhosocial/activerecord/testsuite/feature/basic/fields/test_derived_field_async.py @@ -6,7 +6,7 @@ from rhosocial.activerecord.base import DerivedField, UseColumn from rhosocial.activerecord.model import ActiveRecord -from rhosocial.activerecord.backend.expression import Column, Literal +from rhosocial.activerecord.backend.expression import Literal, NumericColumn class TestAsyncDerivedFieldDeclaration: @@ -27,7 +27,7 @@ async def test_descriptor_class_access(self, async_product_class): async def test_derived_field_with_static_expression(self, async_product_class): """Test DerivedField constructed with a pre-built expression object (non-callable path).""" dialect = async_product_class.backend().dialect - static_expr = Column(dialect, "price") * Literal(dialect, 2) + static_expr = NumericColumn(dialect, "price") * Literal(dialect, 2) df = DerivedField(static_expr) resolved = df.resolve(dialect) assert resolved is static_expr, \ @@ -182,7 +182,7 @@ async def test_derived_dict_with_lambda(self, async_product_class): """A lambda-based derived dict should evaluate against the dialect.""" await self._insert(async_product_class, "D1", 100.0, 4) results = await async_product_class.find_all( - derived={"my_discount": lambda d: Column(d, "price") * Literal(d, 0.8)} + derived={"my_discount": lambda d: NumericColumn(d, "price") * Literal(d, 0.8)} ) assert results[0].__dict__["my_discount"] == pytest.approx(80.0), \ "Expected my_discount to be 80.0" @@ -202,7 +202,7 @@ async def test_derived_dict_with_expression_object(self, async_product_class): """A derived dict should accept a pre-built expression object directly.""" await self._insert(async_product_class, "D3", 80.0, 5) dialect = async_product_class.backend().dialect - expr = Column(dialect, "price") * Literal(dialect, 0.5) + expr = NumericColumn(dialect, "price") * Literal(dialect, 0.5) results = await async_product_class.find_all( derived={"half_price": expr} ) @@ -239,7 +239,7 @@ async def test_extra_derived_basic(self, async_product_class): """extra_derived should add new derived fields without disturbing declared ones.""" await self._insert(async_product_class, "F", 100.0, 10) results = await async_product_class.find_all( - extra_derived={"triple_price": lambda d: Column(d, "price") * Literal(d, 3)} + extra_derived={"triple_price": lambda d: NumericColumn(d, "price") * Literal(d, 3)} ) assert len(results) == 1, "Expected 1 record to be returned" assert results[0].__dict__["triple_price"] == pytest.approx(300.0), \ @@ -250,7 +250,7 @@ async def test_extra_derived_conflict_raises(self, async_product_class): await self._insert(async_product_class, "G", 50.0, 2) with pytest.raises(ValueError, match="conflicts with a declared derived field"): await async_product_class.find_all( - extra_derived={"discounted_price": lambda d: Column(d, "price") * Literal(d, 0.5)} + extra_derived={"discounted_price": lambda d: NumericColumn(d, "price") * Literal(d, 0.5)} ) async def test_derived_and_extra_derived_together(self, async_product_class): @@ -258,7 +258,7 @@ async def test_derived_and_extra_derived_together(self, async_product_class): await self._insert(async_product_class, "H", 40.0, 5) results = await async_product_class.find_all( derived=True, - extra_derived={"double_qty": lambda d: Column(d, "quantity") * Literal(d, 2)} + extra_derived={"double_qty": lambda d: NumericColumn(d, "quantity") * Literal(d, 2)} ) assert results[0].discounted_price == pytest.approx(36.0), \ "Expected discounted_price to be 36.0" @@ -397,7 +397,7 @@ class ConflictModel(ActiveRecord): id: Optional[int] = None discount_rate: Annotated[float, UseColumn("disc")] discounted: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ), UseColumn("disc")]] async def test_use_column_no_conflict_different_names(self): @@ -407,7 +407,7 @@ class NoConflict(ActiveRecord): id: Optional[int] = None discount_rate: Annotated[float, UseColumn("rate")] discounted: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ), UseColumn("disc")]] assert NoConflict.__derived_fields__["discounted"].column_name == "disc", \ diff --git a/src/rhosocial/activerecord/testsuite/feature/basic/fixtures/models.py b/src/rhosocial/activerecord/testsuite/feature/basic/fixtures/models.py index 5326db0..6258da8 100644 --- a/src/rhosocial/activerecord/testsuite/feature/basic/fixtures/models.py +++ b/src/rhosocial/activerecord/testsuite/feature/basic/fixtures/models.py @@ -767,7 +767,7 @@ class AsyncMappedOrderItem(CompositePKMixin, AsyncActiveRecord): # ============================================================================= from rhosocial.activerecord.base import DerivedField -from rhosocial.activerecord.backend.expression import Column, Literal +from rhosocial.activerecord.backend.expression import NumericColumn, Literal class Product(ActiveRecord): @@ -779,11 +779,11 @@ class Product(ActiveRecord): quantity: int discounted_price: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), )]] total_value: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Column(d, "quantity"), + lambda d: NumericColumn(d, "price") * NumericColumn(d, "quantity"), )]] @@ -796,11 +796,11 @@ class AsyncProduct(AsyncActiveRecord): quantity: int discounted_price: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), )]] total_value: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Column(d, "quantity"), + lambda d: NumericColumn(d, "price") * NumericColumn(d, "quantity"), )]] @@ -814,11 +814,11 @@ class ProductFormA(ActiveRecord): quantity: int discounted_price: ClassVar[DerivedField] = DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ) total_value: ClassVar[DerivedField] = DerivedField( - lambda d: Column(d, "price") * Column(d, "quantity"), + lambda d: NumericColumn(d, "price") * NumericColumn(d, "quantity"), ) @@ -831,11 +831,11 @@ class AsyncProductFormA(AsyncActiveRecord): quantity: int discounted_price: ClassVar[DerivedField] = DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ) total_value: ClassVar[DerivedField] = DerivedField( - lambda d: Column(d, "price") * Column(d, "quantity"), + lambda d: NumericColumn(d, "price") * NumericColumn(d, "quantity"), ) @@ -900,11 +900,11 @@ class ProductWithColumnAndAdapter(ActiveRecord): quantity: int discounted_price: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ), UseColumn("disc")]] total_int: ClassVar[Annotated[int, DerivedField( - lambda d: Column(d, "price") * Column(d, "quantity"), + lambda d: NumericColumn(d, "price") * NumericColumn(d, "quantity"), ), UseAdapter(PriceToIntAdapter(), int)]] @@ -917,10 +917,10 @@ class AsyncProductWithColumnAndAdapter(AsyncActiveRecord): quantity: int discounted_price: ClassVar[Annotated[float, DerivedField( - lambda d: Column(d, "price") * Literal(d, 0.9), + lambda d: NumericColumn(d, "price") * Literal(d, 0.9), ), UseColumn("disc")]] total_int: ClassVar[Annotated[int, DerivedField( - lambda d: Column(d, "price") * Column(d, "quantity"), + lambda d: NumericColumn(d, "price") * NumericColumn(d, "quantity"), ), UseAdapter(PriceToIntAdapter(), int)]] diff --git a/src/rhosocial/activerecord/testsuite/feature/ddl/fixtures/models.py b/src/rhosocial/activerecord/testsuite/feature/ddl/fixtures/models.py index 6454082..d66c068 100644 --- a/src/rhosocial/activerecord/testsuite/feature/ddl/fixtures/models.py +++ b/src/rhosocial/activerecord/testsuite/feature/ddl/fixtures/models.py @@ -11,7 +11,10 @@ from typing import ClassVar, Optional from uuid import UUID -from rhosocial.activerecord.backend.expression.core import Column +from rhosocial.activerecord.backend.expression.column_types import ( + NumericColumn, + StringColumn, +) from rhosocial.activerecord.backend.expression.statements.ddl_table import ( ColumnConstraintType, ForeignKeyConstraint, @@ -65,7 +68,7 @@ class SpecOrder(UUIDMixin, ActiveRecord): str, UseConstraint( ColumnConstraintType.CHECK, - check_condition=lambda dialect: Column(dialect, "status").in_( + check_condition=lambda dialect: StringColumn(dialect, "status").in_( ["open", "paid", "shipped"] ), name="ck_spec_orders_status", @@ -96,7 +99,7 @@ class AsyncSpecOrder(UUIDMixin, AsyncActiveRecord): str, UseConstraint( ColumnConstraintType.CHECK, - check_condition=lambda dialect: Column(dialect, "status").in_( + check_condition=lambda dialect: StringColumn(dialect, "status").in_( ["open", "paid", "shipped"] ), name="ck_spec_orders_status", @@ -132,7 +135,7 @@ class CapabilityPost(ActiveRecord): UseGeneratedColumn( lambda dialect: GeneratedColumnExpression( dialect, - expression=Column(dialect, "views") * 2, + expression=NumericColumn(dialect, "views") * 2, storage_type=GeneratedColumnType.STORED, ) ), @@ -161,7 +164,7 @@ class AsyncCapabilityPost(AsyncActiveRecord): UseGeneratedColumn( lambda dialect: GeneratedColumnExpression( dialect, - expression=Column(dialect, "views") * 2, + expression=NumericColumn(dialect, "views") * 2, storage_type=GeneratedColumnType.STORED, ) ), diff --git a/src/rhosocial/activerecord/testsuite/feature/ddl/test_ddl_source_contract.py b/src/rhosocial/activerecord/testsuite/feature/ddl/test_ddl_source_contract.py index b85c325..1281e46 100644 --- a/src/rhosocial/activerecord/testsuite/feature/ddl/test_ddl_source_contract.py +++ b/src/rhosocial/activerecord/testsuite/feature/ddl/test_ddl_source_contract.py @@ -9,6 +9,12 @@ import pytest +from rhosocial.activerecord.backend.expression import ( + ArrayColumn, + JSONColumn, + NumericColumn, + StringColumn, +) from rhosocial.activerecord.backend.expression.core import Column from rhosocial.activerecord.backend.expression.statements import ( ColumnConstraintType, @@ -35,6 +41,7 @@ DerivedField, UseColumn, UseColumnAttributes, + UseColumnType, UseComment, UseConstraint, UseGeneratedColumn, @@ -79,6 +86,10 @@ class BackendOption(ColumnOptions): SECOND_INDEX = UseIndex("idx_payload_secondary", type="BTREE") FIRST_COMMENT = UseComment("first comment") SECOND_COMMENT = UseComment("second comment") +COLUMN_TYPE_PAYLOAD = UseColumnType(JSONColumn) +COLUMN_TYPE_LABEL = UseColumnType(StringColumn) +COLUMN_TYPE_STORED = UseColumnType(NumericColumn) +SQL_TYPE_STORED = UseSqlType(TextType()) def first_generated(dialect): @@ -289,6 +300,24 @@ class AsyncPlainModel(AsyncActiveRecord): note: Optional[str] = None +class ColumnTypeDeclarations: + __table_name__ = "ddl_contract_column_types" + + record_id: Annotated[int, UseColumn("record_id")] + payload: Annotated[dict, COLUMN_TYPE_PAYLOAD] + label: Annotated[str, COLUMN_TYPE_LABEL] + stored: Annotated[int, SQL_TYPE_STORED, COLUMN_TYPE_STORED] + note: Optional[str] = None + + +class SyncColumnTypeModel(ColumnTypeDeclarations, ActiveRecord): + pass + + +class AsyncColumnTypeModel(ColumnTypeDeclarations, AsyncActiveRecord): + pass + + class UnhandledAnnotation(DDLAnnotation): pass @@ -335,7 +364,7 @@ def test_contract_source_collects_table_and_field_declarations(model): def test_contract_field_collection_preserves_order_and_first_singular_marker(): model = SyncContractModel - sql_type = model.column_type("status") + sql_type = model.column_data_type("status") assert sql_type is SQL_TYPE assert len(sql_type.data_types) == 2 assert isinstance(sql_type.data_types[0], JsonType) @@ -362,13 +391,88 @@ def test_contract_field_collection_preserves_order_and_first_singular_marker(): assert indexes[0].if_exists is True assert indexes[0].concurrent is True assert model.column_options("status") is BACKEND_OPTION - assert model.column_type("payload") is PAYLOAD_TYPE - assert len(model.column_type("payload").data_types) == 2 - assert isinstance(model.column_type("payload").data_types[0], JsonType) - assert isinstance(model.column_type("payload").data_types[1], TextType) + assert model.column_data_type("payload") is PAYLOAD_TYPE + assert len(model.column_data_type("payload").data_types) == 2 + assert isinstance(model.column_data_type("payload").data_types[0], JsonType) + assert isinstance(model.column_data_type("payload").data_types[1], TextType) + assert model.column_data_type("note") is None + + +@pytest.mark.parametrize( + "model", [SyncColumnTypeModel, AsyncColumnTypeModel], ids=("sync", "async") +) +def test_contract_column_type_presents_the_declaration_as_written(model): + """``column_type()`` hands the declaration back; it chooses nothing. + + Identity is preserved: which class the declaration *means* on a given + backend is the field accessor's question (answered in the typed-column + contracts), and this accessor must not pre-empt it. A field that declares + nothing answers ``None``, exactly as ``column_data_type()`` does when + storage is left to inference. + """ + assert model.column_type("payload") is COLUMN_TYPE_PAYLOAD + assert model.column_type("payload").column_class is JSONColumn + assert model.column_type("label") is COLUMN_TYPE_LABEL assert model.column_type("note") is None +@pytest.mark.parametrize( + "model", [SyncColumnTypeModel, AsyncColumnTypeModel], ids=("sync", "async") +) +def test_contract_declaration_accessors_are_independent(model): + """Each accessor answers only its own side of a field's declarations. + + ``UseColumnType`` and ``UseSqlType`` are documented as strictly + independent -- a field may declare either, both, or neither -- so a + storage declaration must not leak into the column-class accessor, a + column-class declaration must not answer for storage, and a field that + declares both answers both, identity preserved on each side. This is what + makes the two checkable independently instead of one being derived from + the other. + """ + # Declares the column class only: storage is left unanswered. + assert model.column_type("payload") is COLUMN_TYPE_PAYLOAD + assert model.column_data_type("payload") is None + # Declares both: each side presents its own declaration object. + assert model.column_type("stored") is COLUMN_TYPE_STORED + assert model.column_data_type("stored") is SQL_TYPE_STORED + # Declares neither. + assert model.column_type("note") is None + assert model.column_data_type("note") is None + + +def test_contract_a_storage_only_field_answers_no_column_class(): + """``SyncContractModel.status`` declares storage with ``UseSqlType`` only. + + The column-class accessor answers ``None`` for it: inference is the field + accessor's route, and the declaration accessor must not invent one. + """ + assert SyncContractModel.column_type("status") is None + assert SyncContractModel.column_data_type("status") is SQL_TYPE + + +def test_contract_repeated_column_type_declarations_fail_explicitly(): + """One field declares one column class; a second declaration is a mistake. + + Keeping the first silently would make the model behave as whichever the + annotation order happened to put first -- a coin flip its author never + wrote down. The failure names the declaration mechanism, so the fix is + obvious. + """ + + class DoubleDeclared: + __table_name__ = "ddl_contract_double_column_types" + + record_id: Annotated[int, UseColumn("record_id")] + payload: Annotated[dict, UseColumnType(JSONColumn), UseColumnType(ArrayColumn)] + + class SyncDoubleDeclared(DoubleDeclared, ActiveRecord): + pass + + with pytest.raises(TypeError, match="UseColumnType"): + SyncDoubleDeclared.column_type("payload") + + def test_contract_table_collection_preserves_overrides_and_copies_sequences(): model = SyncContractModel assert model.table_options() is TABLE_OPTIONS @@ -391,7 +495,7 @@ def test_contract_table_collection_preserves_overrides_and_copies_sequences(): def test_contract_batch_interfaces_cover_all_model_fields(): model = SyncContractModel assert list(model.columns_name()) == ["record_id", "status", "payload", "note"] - assert list(model.columns_type()) == ["record_id", "status", "payload", "note"] + assert list(model.columns_data_type()) == ["record_id", "status", "payload", "note"] assert list(model.columns_constraints()) == ["record_id", "status", "payload", "note"] assert list(model.columns_attributes()) == ["record_id", "status", "payload", "note"] assert list(model.columns_indexes()) == ["record_id", "status", "payload", "note"] @@ -401,7 +505,7 @@ def test_contract_batch_interfaces_cover_all_model_fields(): fields = ["status", "record_id", "note"] for method_name in ( "columns_name", - "columns_type", + "columns_data_type", "columns_constraints", "columns_attributes", "columns_indexes", @@ -467,7 +571,7 @@ def test_contract_primary_key_nullability_overrides_follow_source_rules(): @pytest.mark.parametrize("model", [PlainModel, AsyncPlainModel], ids=("sync", "async")) def test_contract_defaults_are_empty_or_none(model): assert model.ddl_field_names() == ("id", "note") - assert model.column_type("id") is None + assert model.column_data_type("id") is None assert model.column_constraints("note") == [] assert model.column_attributes("note") == [] assert model.column_indexes("note") == [] @@ -488,14 +592,30 @@ def test_contract_defaults_are_empty_or_none(model): def test_contract_unknown_fields_and_unhandled_annotations_fail_explicitly(): - assert SyncContractModel.column_type("missing") is None - assert SyncContractModel.column_constraints("missing") == [] - assert SyncContractModel.column_attributes("missing") == [] - assert SyncContractModel.column_indexes("missing") == [] - assert SyncContractModel.column_comment("missing") is None - assert SyncContractModel.generated_column("missing") is None - assert SyncContractModel.column_options("missing") is None + """A field the model does not have is a mistake in the call, not an answer. + + ``None`` (or an empty collection) is the answer for a *real* field that + declares nothing. A name the model does not carry must not produce the + same answer, or a misspelled field would be ignored in silence -- so every + accessor refuses it with ``KeyError``, the failure ``field_python_type`` + has always raised. + """ + for accessor in ( + "column_type", + "column_data_type", + "column_constraints", + "column_attributes", + "column_indexes", + "column_comment", + "generated_column", + "column_options", + ): + with pytest.raises(KeyError): + getattr(SyncContractModel, accessor)("missing") with pytest.raises(KeyError): SyncContractModel.field_python_type("missing") + # An explicitly named unknown field fails the batch accessor the same way. + with pytest.raises(KeyError): + SyncContractModel.columns_data_type(["missing"]) with pytest.raises(TypeError, match="explicit handler"): UnhandledModel.ddl_field_names() diff --git a/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation.py b/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation.py index fab431d..95a985d 100644 --- a/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation.py +++ b/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation.py @@ -38,7 +38,7 @@ def test_primary_key_and_nullability_declarations(self, bare_class): class TestGenericDeclarationDerivation: def test_type_and_constraint_declarations(self, spec_order_class): - code_type = spec_order_class.column_type("code") + code_type = spec_order_class.column_data_type("code") assert isinstance(code_type, UseSqlType) assert code_type.data_type.name == "varchar" assert code_type.data_type.length == 32 diff --git a/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation_async.py b/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation_async.py index 3630a9c..aa9ac1f 100644 --- a/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation_async.py +++ b/src/rhosocial/activerecord/testsuite/feature/ddl/test_derivation_async.py @@ -40,7 +40,7 @@ async def test_primary_key_and_nullability_declarations(self, async_bare_class): class TestGenericDeclarationDerivationAsync: async def test_type_and_constraint_declarations(self, async_spec_order_class): - code_type = async_spec_order_class.column_type("code") + code_type = async_spec_order_class.column_data_type("code") assert isinstance(code_type, UseSqlType) assert code_type.data_type.name == "varchar" assert code_type.data_type.length == 32 diff --git a/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query.py b/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query.py index 121c3e6..684d24b 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query.py @@ -29,19 +29,19 @@ def test_where_pk_predicate(self, seeded, order_item_class): def test_where_single_column(self, seeded, order_item_class): """Filtering by a single composite key column should return matching rows.""" - backend = order_item_class.backend() + dialect = order_item_class.backend().dialect results = order_item_class.query().where( - Column(backend.dialect, "order_id") == 1 + ComparisonPredicate(dialect, "=", Column(dialect, "order_id"), Literal(dialect, 1)) ).all() assert len(results) == 2, "Expected 2 rows with order_id == 1" def test_where_and_condition(self, seeded, order_item_class): """Combining multiple where clauses should apply both filters.""" - backend = order_item_class.backend() + dialect = order_item_class.backend().dialect results = order_item_class.query().where( - Column(backend.dialect, "order_id") == 1 + ComparisonPredicate(dialect, "=", Column(dialect, "order_id"), Literal(dialect, 1)) ).where( - Column(backend.dialect, "quantity") > 1 + ComparisonPredicate(dialect, ">", Column(dialect, "quantity"), Literal(dialect, 1)) ).all() assert len(results) == 1, "Expected 1 row matching both conditions" assert results[0].product_id == 101, "Expected product_id to be 101" diff --git a/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query_async.py b/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query_async.py index 4ffc3d5..b6cb36c 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query_async.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/basic/test_composite_pk_query_async.py @@ -29,9 +29,9 @@ async def test_where_single_column(self, async_order_item_class): async_order_item_class(order_id=2, product_id=101, quantity=5, unit_price=Decimal("15.00")), ] await async_order_item_class.bulk_create(items) - backend = async_order_item_class.backend() + dialect = async_order_item_class.backend().dialect results = await async_order_item_class.query().where( - Column(backend.dialect, "order_id") == 1 + ComparisonPredicate(dialect, "=", Column(dialect, "order_id"), Literal(dialect, 1)) ).all() assert len(results) == 2, "Expected 2 rows with order_id == 1" @@ -43,11 +43,11 @@ async def test_where_and_condition(self, async_order_item_class): async_order_item_class(order_id=2, product_id=101, quantity=5, unit_price=Decimal("15.00")), ] await async_order_item_class.bulk_create(items) - backend = async_order_item_class.backend() + dialect = async_order_item_class.backend().dialect results = await async_order_item_class.query().where( - Column(backend.dialect, "order_id") == 1 + ComparisonPredicate(dialect, "=", Column(dialect, "order_id"), Literal(dialect, 1)) ).where( - Column(backend.dialect, "quantity") > 1 + ComparisonPredicate(dialect, ">", Column(dialect, "quantity"), Literal(dialect, 1)) ).all() assert len(results) == 1, "Expected 1 row matching both conditions" assert results[0].product_id == 101, "Expected product_id to be 101" diff --git a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte.py b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte.py index 738700f..7ae022b 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte.py @@ -25,14 +25,14 @@ def test_cte_aggregate(self, seeded, order_item_class): pytest.skip("Backend does not support CTE") from rhosocial.activerecord.query import CTEQuery - from rhosocial.activerecord.backend.expression import Column, Literal + from rhosocial.activerecord.backend.expression import Column, ComparisonPredicate, Literal base = order_item_class.query() cte = CTEQuery(backend) cte.with_cte("order_summary", base) result = cte.from_cte("order_summary").select( Column(dialect, "order_id"), Column(dialect, "quantity") - ).where(Column(dialect, "order_id") == 1).aggregate() + ).where(ComparisonPredicate(dialect, "=", Column(dialect, "order_id"), Literal(dialect, 1))).aggregate() assert len(result) == 2, "Expected 2 aggregated rows for order_id == 1" def test_cte_pk_filter(self, seeded, order_item_class): diff --git a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte_async.py b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte_async.py index c80f8e8..e4b8394 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte_async.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_composite_pk_cte_async.py @@ -21,14 +21,14 @@ async def test_cte_aggregate(self, async_order_item_class): await async_order_item_class.bulk_create(items) from rhosocial.activerecord.query import AsyncCTEQuery - from rhosocial.activerecord.backend.expression import Column + from rhosocial.activerecord.backend.expression import Column, ComparisonPredicate, Literal base = async_order_item_class.query() cte = AsyncCTEQuery(backend) cte.with_cte("order_summary", base) result = await cte.from_cte("order_summary").select( Column(dialect, "order_id") - ).where(Column(dialect, "order_id") == 1).aggregate() + ).where(ComparisonPredicate(dialect, "=", Column(dialect, "order_id"), Literal(dialect, 1))).aggregate() assert len(result) == 2, "Expected 2 aggregated rows for order_id == 1" async def test_cte_pk_filter(self, async_order_item_class): diff --git a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation.py b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation.py index c61fd3d..7ff87b6 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation.py @@ -16,6 +16,7 @@ from rhosocial.activerecord.query import CTEQuery, AsyncCTEQuery from rhosocial.activerecord.backend.expression import statements, core, query_parts +from rhosocial.activerecord.backend.expression.predicates import ComparisonPredicate from rhosocial.activerecord.backend.expression.objects import Table from rhosocial.activerecord.backend.expression.sources import NamedRelationRef from rhosocial.activerecord.testsuite.utils import requires_cte @@ -188,7 +189,12 @@ def test_cte_with_query_expression_as_subquery(self, order_fixtures): dialect, select=[core.Column(dialect, "id"), core.Column(dialect, "status"), core.Column(dialect, "total_amount")], from_=NamedRelationRef(dialect, Table(dialect, Order.table_name())), - where=query_parts.WhereClause(dialect, condition=core.Column(dialect, "status") == core.Literal(dialect, 'active')) + where=query_parts.WhereClause( + dialect, + condition=ComparisonPredicate( + dialect, "=", core.Column(dialect, "status"), core.Literal(dialect, "active") + ), + ) ) # Create a CTE that uses the QueryExpression as its source diff --git a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation_async.py b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation_async.py index c6074a6..63097ba 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation_async.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/cte/test_cte_query_set_operation_async.py @@ -16,6 +16,7 @@ from rhosocial.activerecord.query import CTEQuery, AsyncCTEQuery from rhosocial.activerecord.backend.expression import statements, core, query_parts +from rhosocial.activerecord.backend.expression.predicates import ComparisonPredicate from rhosocial.activerecord.backend.expression.objects import Table from rhosocial.activerecord.backend.expression.sources import NamedRelationRef from rhosocial.activerecord.testsuite.utils import requires_cte @@ -191,7 +192,12 @@ async def test_cte_with_query_expression_as_subquery(self, async_order_fixtures) dialect, select=[core.Column(dialect, "id"), core.Column(dialect, "status"), core.Column(dialect, "total_amount")], from_=NamedRelationRef(dialect, Table(dialect, AsyncOrder.table_name())), - where=query_parts.WhereClause(dialect, condition=core.Column(dialect, "status") == core.Literal(dialect, 'active')) + where=query_parts.WhereClause( + dialect, + condition=ComparisonPredicate( + dialect, "=", core.Column(dialect, "status"), core.Literal(dialect, "active") + ), + ) ) # Create a CTE that uses the QueryExpression as its source diff --git a/src/rhosocial/activerecord/testsuite/feature/query/json/test_json_probe_honesty.py b/src/rhosocial/activerecord/testsuite/feature/query/json/test_json_probe_honesty.py new file mode 100644 index 0000000..c5db4b6 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/json/test_json_probe_honesty.py @@ -0,0 +1,329 @@ +# src/rhosocial/activerecord/testsuite/feature/query/json/test_json_probe_honesty.py +"""A capability probe that returns True must correspond to working SQL. + +This is the invariant that catches "the probe says yes, the implementation +emits another dialect's syntax". It is not hypothetical: on PostgreSQL +``supports_json_table()`` claimed 12.0+ while the formatter raised +unconditionally, and on SQL Server ``supports_json_table()`` claimed 2016+ +for the same reason. Both handed callers a promise the backend could not +keep. + +Two layers are needed, because either alone passes while the defect is live: + +1. *Renderability* — the probe is True, so the expression must render. This + catches a formatter that raises behind a True probe. +2. *Own syntax* — the SQL must contain a function name the backend itself + declared, and must not contain a function name belonging to some other + backend. Renderability alone is not enough: the core's default + function-based fallback emits MySQL's ``JSON_EXTRACT``, which renders + happily on any dialect and is wrong on most of them. +""" + +# src/rhosocial/activerecord/testsuite/feature/query/json/test_json_probe_honesty.py +import pytest + +from rhosocial.activerecord.backend.dialect.exceptions import UnsupportedFeatureError +from rhosocial.activerecord.backend.expression import ( + Column, + JSONDocumentExpression, + JSONTextExpression, +) + + +@pytest.fixture +def dialect(json_user_fixture): + """The dialect under test, taken from a provider-configured model. + + Read off ``__backend__`` rather than calling ``Model.backend()``: that + resolves the *currently active* backend and raises "No backend configured" + on a shard with no live connection, which is the situation here — every + assertion is on an expression tree or on SQL from a dialect that was never + connected. ``__backend__`` is the instance the provider configured and + needs no connection. + + The model arrives as a direct argument rather than through + ``request.getfixturevalue``, because that cannot be combined with + ``@pytest.mark.parametrize``: a parametrised test has no fixture parameter + to resolve, and pytest rejects the request. + """ + return json_user_fixture.__backend__.dialect + + +#: Function names that belong to one specific backend. A dialect rendering any +#: of these is emitting foreign syntax. +#: Names that identify one dialect's spelling rather than another's. Kept to +#: functions only a single backend has: JSON_VALUE and JSON_QUERY are the +#: standard spellings and Oracle, SQL Server and BigQuery all use them, so +#: listing them as foreign failed the dialect that legitimately renders them. +#: A dialect's own declared set is what decides, not this table — see +#: _own_function_names. +_FOREIGN_FUNCTION_NAMES = ( + "JSON_EXTRACT", # MySQL / MariaDB + "JSON_UNQUOTE", # MySQL / MariaDB + "JSON_CONTAINS", # MySQL + "JSON_TYPE", # MySQL + "JSON_SEARCH", # MySQL + "GET_PATH", # Snowflake + "JSONExtractRaw", # ClickHouse + "JSONExtractString", # ClickHouse + "jsonb_path_query_first", # PostgreSQL + "jsonb_path_query_array", # PostgreSQL + "OPENJSON", # SQL Server +) + + +#: Core used to have one class for both JSON access operators and has since +#: split it in two, one per operator, because the two yield different kinds of +#: value: ``->`` hands back a document that can be indexed into again, ``->>`` +#: hands back text that cannot. The helpers below are split to match, so which +#: operator a caller means is read off the class it builds rather than off a +#: string it passes. The rendered SQL is unchanged by the split -- the +#: formatters branch on ``expr.operation``, not on the class, and dispatch is +#: on ``format_method``, which both classes share -- so every assertion below +#: says the same thing it said before, only through the class that now means it. +def _document_sql(dialect, mode, path="a"): + """Render the document access ``->`` under ``mode``.""" + col = Column(dialect, "j", table="t") + return JSONDocumentExpression(dialect, col, path, mode=mode).to_sql() + + +def _text_sql(dialect, mode, path="a"): + """Render the text access ``->>`` under ``mode``.""" + col = Column(dialect, "j", table="t") + return JSONTextExpression(dialect, col, path, operation="->>", mode=mode).to_sql() + + +# --------------------------------------------------------------------------- +# Layer 1: a True probe must render +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "probe, render", + [ + ("supports_json_type", _text_sql), + ("supports_json_arrow_operators", _document_sql), + ], + ids=["supports_json_type->>", "supports_json_arrow_operators->"], +) +def test_true_json_probe_renders(dialect, probe, render): + """If the probe says the dialect handles JSON, it must not then refuse. + + The arrow mode is only required of the arrow probe. Having JSON and having + ``->`` are separate capabilities — Oracle, Snowflake, BigQuery and Firebird + all have the first without the second — so demanding arrows of the type + probe asks the dialect for something it never claimed. The modes each probe + does cover must render. + + Each cell names the operator it renders through the helper that builds it: + the type probe is checked with ``->>`` and the arrow probe with ``->``, + which is the split the helpers encode. + """ + if not getattr(dialect, probe)(): + pytest.skip(f"{dialect.name} does not advertise {probe}") + + modes = ("auto", "arrow", "function") if probe == "supports_json_arrow_operators" else ("auto", "function") + for mode in modes: + try: + sql, params = render(dialect, mode) + except Exception as exc: # noqa: BLE001 - any refusal is a defect + pytest.fail( + f"{dialect.name} advertises {probe}() but rendering mode=" + f"{mode!r} raised {type(exc).__name__}: {exc}" + ) + assert sql, f"{dialect.name} rendered an empty statement for mode={mode!r}" + + +def test_arrow_mode_is_refused_without_arrow_support(dialect): + """Asking for a spelling the server lacks should be a clear refusal. + + AUTO covers the honest answer — it falls back to the function form — so a + dialect that says it has JSON but not arrows is not a defect, and neither + is refusing the mode that would need them. + """ + if dialect.supports_json_arrow_operators(): + pytest.skip(f"{dialect.name} has arrow operators") + if not dialect.supports_json_type(): + pytest.skip(f"{dialect.name} has no JSON support at all") + + with pytest.raises(UnsupportedFeatureError): + _document_sql(dialect, "arrow") + + +def test_forced_arrow_mode_is_honoured(dialect): + """ARROW must mean arrows, not "whatever this dialect defaults to". + + This is the assertion that catches ``format_json_expression`` discarding + ``expr.mode``. It deliberately does not require AUTO to produce arrows: the + two spellings take different path arguments — ``->`` takes a key name, + while a jsonpath function takes ``$.a[0]`` — so a dialect whose callers + pass jsonpaths must keep the path form in AUTO. What AUTO may not do is + silently return the same SQL for every mode. + """ + if not dialect.supports_json_arrow_operators(): + pytest.skip(f"{dialect.name} has no arrow operators") + + sql, _ = _document_sql(dialect, "arrow") + assert "->" in sql, f"ARROW mode produced {sql!r} on {dialect.name}" + + +def test_arrow_probe_is_not_claimed_without_the_operators(dialect): + """A dialect with no arrow support must not be able to render them. + + The inverse of the above, and the one that matters for honesty: claiming + arrows and then refusing is a broken promise to the caller. + """ + if dialect.supports_json_arrow_operators(): + pytest.skip(f"{dialect.name} has arrow operators") + + with pytest.raises(UnsupportedFeatureError): + _document_sql(dialect, "arrow") + + +def test_forced_function_mode_differs_from_arrow(dialect): + """FUNCTION and ARROW must be distinguishable, else the mode is ignored. + + This is the assertion that would have caught ``format_json_expression`` + discarding ``expr.mode``: all four mode values rendered the same SQL. + + Both sides build the *same* operator — ``->``, the document access — and + differ only in ``mode``. That is the point: it is ``mode`` reaching the + formatter that is under test, not the choice of operator, so holding the + operator fixed is what makes the comparison mean anything. + """ + if not dialect.supports_json_arrow_operators(): + pytest.skip(f"{dialect.name} has no arrow operators") + + arrow, _ = _document_sql(dialect, "arrow") + function, _ = _document_sql(dialect, "function") + assert arrow != function, ( + f"{dialect.name} renders ARROW and FUNCTION identically ({arrow!r}); " + f"JSONPathMode is not reaching the formatter" + ) + + +# --------------------------------------------------------------------------- +# Layer 2: the SQL must be this backend's own +# --------------------------------------------------------------------------- + + +def test_function_mode_emits_no_foreign_syntax(dialect): + """The rendered SQL must not contain another backend's function names. + + Catches the core's MySQL-shaped fallback being inherited by a dialect + that has none of those functions. + """ + if not dialect.supports_json_type(): + pytest.skip(f"{dialect.name} has no JSON support") + + own = _own_function_names(dialect) + sql, _ = _text_sql(dialect, "function") + + if not own: + # Nothing declared, so nothing may be claimed either. + assert not any(name in sql for name in _FOREIGN_FUNCTION_NAMES), ( + f"{dialect.name} declares no JSON function names but rendered " + f"{sql!r}" + ) + return + + assert any(name in sql for name in own), ( + f"{dialect.name} rendered {sql!r}, which contains none of its own " + f"declared JSON functions {own}" + ) + + +#: The JSON function names worth asking about. Every backend's +#: supports_json_function takes a name rather than returning a set, so the +#: question is asked one name at a time. +_JSON_FUNCTION_NAMES = ( + "JSON_EXTRACT", "JSON_UNQUOTE", "JSON_QUERY", "JSON_VALUE", "JSON_TABLE", + "JSON_CONTAINS", "JSON_TYPE", "JSON_SEARCH", "GET_PATH", "OPENJSON", + "JSONExtractRaw", "JSONExtractString", + "jsonb_path_query_first", "jsonb_path_query_array", "jsonb_path_query", +) + + +def _own_function_names(dialect) -> tuple: + """JSON function names this dialect declares for itself. + + Asks ``supports_json_function(name)`` per name, because that is the shape + every backend implements. Calling it with no arguments — which an earlier + version of this helper did — raises TypeError on all of them and returns + nothing, so the check silently degraded to "render no function name at + all" and no backend was ever really compared against its own set. + + A dialect with no such probe gets an empty result, which is legitimate: it + means the dialect renders with operators only. + """ + probe = getattr(dialect, "supports_json_function", None) + if probe is None: + return () + own = [] + for name in _JSON_FUNCTION_NAMES: + try: + if probe(name): + own.append(name) + except Exception: # noqa: BLE001 - a probe that cannot answer is not a failure here + return () + return tuple(own) + + +# --------------------------------------------------------------------------- +# Honesty about the things a dialect does not have +# --------------------------------------------------------------------------- + + +def test_json_table_claim_matches_the_formatter(dialect): + """A True probe must not sit in front of a formatter that always raises. + + PostgreSQL and SQL Server both shipped this exact contradiction. + """ + claimed = dialect.supports_json_table() + try: + dialect.format_json_table_expression(None) + except Exception: # noqa: BLE001 - the refusal is what we are checking + assert claimed is False, ( + f"{dialect.name} claims supports_json_table() but its formatter " + f"refuses; callers trusting the probe get an exception at render " + f"time" + ) + else: + assert claimed is True, ( + f"{dialect.name} implements format_json_table_expression but " + f"reports supports_json_table() as False" + ) + + +@pytest.mark.parametrize( + "probe", + [ + "supports_json_type", + "supports_jsonb", + "supports_json_path", + "supports_json_arrow_operators", + ], +) +def test_json_probes_are_defined_once_in_the_mro(dialect, probe): + """Two definitions with different version gates let the MRO pick a winner. + + PostgreSQL's ``supports_jsonb_subscript`` existed three times with gates + of 11.0 and 14.0, so which answer a caller got depended on class ordering + rather than on intent. + + A dialect may legitimately override a core default, which puts the probe + in two classes. What it may not do is define the same probe in several of + its own mixins, where the winner is an accident of ordering. + """ + definitions = [ + klass.__name__ for klass in type(dialect).__mro__ if probe in vars(klass) + ] + # Three is the normal shape: the dialect's own override, the mixin default + # it overrides, and the Protocol declaration. The defect was a probe + # defined in several *backend-specific* mixins with different version + # gates, which made the winner depend on class order. Allow the normal + # three and reject a fourth. + assert len(definitions) <= 3, ( + f"{dialect.name} resolves {probe} from {len(definitions)} classes: " + f"{definitions}" + ) diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/README.md b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/README.md new file mode 100644 index 0000000..de80eba --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/README.md @@ -0,0 +1,132 @@ +# The typed-column contracts, and how to satisfy them in a backend + +This directory holds the cross-backend contracts of the typed-column work: + +* ``test_typed_column_contracts.py`` — the column-expression layer: an + explicit ``UseColumnType`` declaration means the same class on every + backend, inference follows each backend's own ``suggested_column_types()`` + table, narrowing is real, rendering does not depend on the column class, + and JSON paths are built identically everywhere. +* ``test_protocol_guarantees.py`` — the protocol's own claims, asserted per + backend: every common entry is answered (a class or an explicit ``None``), + the table's answers stay class-or-``None``, the column object has no slot + for a field's DataType and carries no value-family label, and an annotation + outside the vocabulary fails naming ``UseColumnType`` rather than falling + back to a universal column. +* ``column_helpers.py`` — the test-side helper that mirrors the field + accessor's selection step, plus the canonical ``COMMON_TYPES`` list the + completeness contract holds each backend to. +* ``test_enum_capability_contracts.py`` / ``test_uuid_capability_contracts.py`` + — the concept-specific capability contracts (enum storage, UUID storage). + +A backend satisfies these through the dialect's own declarations, not by +editing anything here. The rules below are the discipline the nine backends +converged on; each backend's local plan directory +(``.claude/plan//`` in that backend's repository) records the +repository-specific history behind them. + +## The three-list discipline (expression round-trip matrix) + +Every backend's round-trip matrix keeps three lists, and the two integrity +tests pin them **in both directions**: what is named is real, and what is +real is named. An entry that cannot be explained by one of the rules below +is a bug in the lists, not a peculiarity to document and ignore. + +1. **Measure with valid arguments before deciding anything.** An invalid + argument raises the constructor's own validation error and hides the + dialect's real answer. The recurring example: a UUID constant expression + probed with an invalid ``which`` raises ``ValueError`` first and reads as + rescuable, while a *valid* ``which`` reveals the dialect refusing the + whole concept in ``__init__``. Probe with the argument the constructor + would accept in a working case. + +2. **A class that refuses in ``__init__`` regardless of arguments** goes in + ``UNCONSTRUCTIBLE``, with a comment stating the measured reason. No + registered constructor can change that answer, so none is written. + +3. **A class a registered special constructor rescues**: + * renders → register the constructor, and appear in no other list; + * refuses to render → register the constructor **and** pin the refusal + in ``LEGITIMATE_NON_RENDERS`` as ``(exception type, message fragment)``. + The pin now has something to assert against. + +4. **Never in two lists at once.** The pin table is consulted before the + ``UnsupportedFeatureError`` branch, so a pin on an unconstructible class + asserts a rendering nothing can ever ask for. If a pin and an + ``UNCONSTRUCTIBLE`` entry name the same class, one of them is wrong. + +5. **Version-gated behaviour is not statically pinnable.** The pin table is + static; a type or refusal that flips with the server version (MariaDB's + UUID type at 10.7 is the recorded case) fails the pin on one end of the + version matrix no matter which answer is pinned. Leave it unpinned and + assert it in a version-parametrised test instead. + +6. **The integrity tests fail without naming the offender.** When + ``test_pinned_non_render_really_does_not_render`` reports + ``DID NOT RAISE``, run a probe that iterates the pin table and reports + each entry's actual outcome; do not bisect by hand. + +## Measurement discipline + +* **Measure, then declare.** Every list entry, pin and constructor cites a + measurement — a probe against the dialect, or a scenario-server answer. + Refusals quote the server's own response where one exists (ClickHouse's + ``Expected one of: ...`` list is the model). +* **Reproduce locally before trusting CI.** CI's ``--log-failed`` output is + partial: a class that fails may not appear in it. The local run is + authoritative for *which* classes fail; CI is authoritative for what the + full suite does in the CI environment. +* **Local scope is bounded** (see the organisation rules): the targeted + files that changed, not the suite. + +## The ``parse_type`` contract (the introspection inverse) + +Every dialect implements ``parse_type``; it is also the last member of the +``DataTypeSupport`` protocol, so without it ``parse_data_type_str`` falls +back to a generic ``CustomType`` and the protocol check fails. BigQuery was +the last to gain it (``000b97f``) and documents the shape in full; the rules +that are the same everywhere: + +1. **One concept in, one class out**, answered by *storage*. Where a + backend collapses concepts on the way in (four integer widths stored as + one integer type), the parse answers with the concept that was **not** + widened — the class whose rendering is that storage — and re-renders the + identical string. A narrow declaration reading back as its widened + concept is the differ reporting a real widening. +2. **Bare-word defaults come from ``type_parameter_defaults()``**, never + from a number written into the parser. +3. **The spelling that was written is carried** on the instance where the + concept has more than one; it never takes part in equality. +4. **A word the framework does not model is ``CustomType`` carrying the + word**, not a guess. A word whose text is not identifier-shaped + (BigQuery's ``STRUCT<...>``) is refused by the type-name validation + rather than collapsed to a bare name two different columns would share. +5. **Empty input is a caller error** (``ValueError``), not a type name. + +## After a core refactor: the sweep checklist + +The rebase that landed this branch taught the hard lessons; they generalise +to any future core change that touches the shared vocabulary: + +* **Merge conflicts dimension by dimension.** A block that mixes a + signature change with an assertion change (SQL Server's ``Table(...)`` + argument with ClickHouse's syntax assertions) takes a piece from each + side, not one side wholesale. Decide per dimension by reading which name + the *unchanged* downstream code uses. +* **Auto-merged regions still get swept.** PostgreSQL had sixteen failing + call sites outside every conflict block because a multi-line argument + list can wrap the argument the check looks for. Grep by content, not by + line shape. +* **A signature and a body can both look merged and disagree.** A + two-state default over a tri-state body passed lint and import on every + backend; only runtime instantiation caught it. Instantiate representative + expressions after the merge, not just before commit. +* **A registered constructor with the wrong signature breaks working + cases** — ``make_instance`` prefers specials and does not fall back. + Test the constructor by itself before registering it. +* **Read the constructor before wrapping a catalogue object.** DDL targets + are objects (``Type``, ``Table``, ``MaterializedView``, ``Sequence``); + the class one backend needed is not the class the next one validates. +* **Check a helper script's exit code before ``git add -A`` and + ``rebase --continue``.** An unchecked abort once committed conflict + markers into history. diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/column_helpers.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/column_helpers.py new file mode 100644 index 0000000..fb79fad --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/column_helpers.py @@ -0,0 +1,97 @@ +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/column_helpers.py +"""Test-side helpers for the typed-column contracts, and the canonical entry list. + +The production path is ``base/field_proxy.py``: ``Model.c.`` asks the +model for the field's column-type declaration (dialect-free), then the field +accessor selects the class the model's backend answers. This module mirrors +that selection step for a bare annotation, so a contract test can build a +column without declaring a model. + +``COMMON_TYPES`` is the contract-side canonical list of the common Python +types every backend must answer for. It lives here, not in the framework: the +framework resolves by walking whatever keys a backend's table declares, and +the required key set is a contract the tests state and hold each backend to. + +Deliberately **not** a fallback: an annotation the selection cannot classify +raises :class:`ColumnTypeResolutionError` exactly as ``Model.c.`` +would, so a contract test that builds a column from a bad annotation fails +here for the same reason the model would. +""" + +import datetime +import decimal +import enum +import uuid +from typing import Any, Optional, Tuple, Type + +from rhosocial.activerecord.backend.expression.column_types import ColumnBase +from rhosocial.activerecord.backend.expression.core import Column +from rhosocial.activerecord.base.field_proxy import ( + ColumnTypeResolutionError, + FieldAccessor, +) + +#: The common Python types, in declared reading order. The order matters where +#: a subclass walk meets an ambiguous annotation: ``bool`` ahead of ``int``, +#: and ``enum.Enum`` tested before the walk (a ``(int, Enum)`` stays an enum). +COMMON_TYPES: Tuple[Any, ...] = ( + bool, + int, + float, + decimal.Decimal, + str, + bytes, + bytearray, + datetime.date, + datetime.time, + datetime.datetime, + datetime.timedelta, + uuid.UUID, + dict, + list, + tuple, + set, + frozenset, + enum.Enum, +) + + +def resolve_column_class(dialect: Any, annotation: Any, declared: Any = None) -> Type[ColumnBase]: + """The class *dialect* selects for *annotation*. + + The field accessor's own selection step, invoked without a model. + *declared* is the field's column-type declaration when a test exercises + one; ``None`` asks the backend's tables. + """ + return FieldAccessor._select_column_class(dialect, annotation, declared) + + +def build_column( + dialect: Any, + column_name: str, + annotation: Any, + table: Optional[str] = None, + schema_name: Optional[str] = None, + column_type: Any = None, +) -> ColumnBase: + """Build the column *dialect* selects for *annotation*. + + The two steps the model layer performs in ``base/field_proxy.py``, written + out here because a contract test is not a model: select through the + backend's tables, then construct the class the answer names. + """ + column_class = resolve_column_class(dialect, annotation, column_type) + if column_class is Column: + # The untyped column keeps its own narrower constructor and can only + # arrive by being declared explicitly, so this is a shape difference + # rather than a fallback. + return Column(dialect, column_name, table=table, schema_name=schema_name) + return column_class(dialect, column_name, table=table, schema_name=schema_name) + + +__all__ = [ + "COMMON_TYPES", + "ColumnTypeResolutionError", + "build_column", + "resolve_column_class", +] diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/conftest.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/conftest.py new file mode 100644 index 0000000..39c4430 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/conftest.py @@ -0,0 +1,36 @@ +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/conftest.py +"""Shared fixtures for the typed-column contracts. + +The ``dialect`` fixture used to be written out in each contract file. Three +copies of the same body had accumulated, each with a slightly different +docstring, and the duplication was load-bearing in the wrong direction: the +reason the fixture reads ``__backend__`` instead of calling ``Model.backend()`` +is subtle enough that it should be explained once, in one place, rather than +three times with three different amounts of detail. + +The local definitions in the existing contract files are left alone — pytest +lets a module-level fixture shadow a conftest one, so nothing changes for them — +and new contract files get this one without repeating the reasoning. +""" + +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/conftest.py +import pytest + + +@pytest.fixture +def dialect(json_user_fixture): + """The dialect under test, taken from a provider-configured model. + + Read off ``__backend__`` rather than calling ``Model.backend()``: that + resolves the *currently active* backend and raises "No backend configured" + on a shard with no live connection, which is the situation here — every + assertion is on an expression tree or on SQL from a dialect that was never + connected. ``__backend__`` is the instance the provider configured and + needs no connection. + + The model arrives as a direct argument rather than through + ``request.getfixturevalue``, because that cannot be combined with + ``@pytest.mark.parametrize``: a parametrised test has no fixture parameter + to resolve, and pytest rejects the request. + """ + return json_user_fixture.__backend__.dialect diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_column_operations_core.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_column_operations_core.py new file mode 100644 index 0000000..6356183 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_column_operations_core.py @@ -0,0 +1,175 @@ +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_column_operations_core.py +"""What ``Model.c.`` can and cannot do, at the core level. + +Each family's column class decides which operations exist (one operation group +per receiver family, attached to both the column and the value side), and this +file exercises that through the field proxy — the entry point a model actually +uses. Both halves are pinned: + +* the operations a family offers are reachable and produce expressions; +* the operations it does not offer refuse *at construction*, not in SQL — + ``created_at + created_at`` used to render ``"created" + "created"`` and + fail only at the database, which is the failure mode the narrow classes + exist to remove. + +Only core classes are asserted here: no backend-specific wrappers, no rendered +SQL text (``LENGTH`` is ``LEN`` on one backend and ``lengthUTF8`` on another; +the gate is this file's subject, not the spelling). A backend whose table +answers no column class for an annotation — Firebird's ``dict`` is the +recorded case — skips the tests that need it. +""" + +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_column_operations_core.py +import datetime +from typing import ClassVar + +import pytest + +from rhosocial.activerecord.backend.expression import functions +from rhosocial.activerecord.backend.expression.core import IntegerValueExpression +from rhosocial.activerecord.backend.expression.operators import BinaryArithmeticExpression +from rhosocial.activerecord.base.field_proxy import FieldProxy +from rhosocial.activerecord.model import ActiveRecord + + +@pytest.fixture +def model(dialect): + """A model whose fields cover the core common types, bound to *dialect*. + + ``__backend__`` is set directly rather than by connecting: every assertion + is on expression construction, so no database is needed. + """ + + class Operations(ActiveRecord): + __table_name__ = "core_column_operations" + __primary_key__: ClassVar[str] = "id" + c: ClassVar[FieldProxy] = FieldProxy() + + id: int + name: str + age: int + score: float + payload: dict + created: datetime.datetime + flag: bool + blob: bytes + + Operations.__backend__ = type("B", (), {"dialect": dialect})() + return Operations + + +def _expression(result): + """Assert *result* is an expression that can render, and hand it back.""" + assert callable(getattr(result, "to_sql", None)), f"{result!r} is not an expression" + return result + + +def _skip_without_column_class(dialect, annotation): + """Skip when this backend's table answers no class for *annotation*. + + A refusal is a legitimate answer (the resolution contract says so), and a + contract about the core operation surface must not turn it into a failure + on the backends that refuse. + """ + if dialect.suggested_column_types().get(annotation) is None: + pytest.skip(f"this backend answers no column class for {annotation!r}") + + +# --------------------------------------------------------------------------- +# The operations each family offers are reachable +# --------------------------------------------------------------------------- + + +def test_string_operations(model): + assert _expression(model.c.name.upper()) + assert _expression(model.c.name.substr(1, 2)) + assert _expression(model.c.name.trim()) + assert _expression(model.c.name.like("%a%")) + assert _expression(model.c.name.ilike("%a%")) + + +def test_string_to_integer_is_a_number_that_takes_arithmetic(model): + """``length`` reads a string and gives a number; the number is an integer. + + And a number is not a string: this is where the old duplicated surface + produced ``LENGTH(LENGTH(name))``, which constructed and failed only in + SQL. The integer result offers arithmetic and comparison instead. + """ + length = model.c.name.length() + assert isinstance(length, IntegerValueExpression) + assert _expression(length + 1) + assert _expression(length * 2) + assert _expression(length > 3) + + +def test_numeric_operations(model): + assert _expression(model.c.age + 1) + assert _expression(model.c.age * 2) + assert _expression(model.c.age % 2) + assert _expression(model.c.age.abs()) + assert _expression(model.c.age > 18) + assert _expression(model.c.score.sqrt()) + assert _expression(model.c.score.round(2)) + assert _expression(model.c.score.exp()) + + +def test_json_operations(model, dialect): + _skip_without_column_class(dialect, dict) + assert _expression(model.c.payload.json_path("a")) + assert _expression(model.c.payload.json_value("a")) + + +def test_temporal_operations_and_interval_arithmetic(model, dialect): + assert _expression(model.c.created.date_trunc("month")) + assert _expression(model.c.created.extract("year")) + assert _expression(model.c.created.date_add(1, "day")) + one_day = functions.interval(dialect, 1, "day") + shifted = model.c.created + one_day + assert isinstance(shifted, BinaryArithmeticExpression) + assert isinstance(model.c.created - model.c.created, BinaryArithmeticExpression) + + +def test_boolean_operations(model): + assert _expression(model.c.flag.is_true()) + assert _expression(model.c.flag.is_false()) + + +def test_binary_operations(model): + assert _expression(model.c.blob == b"bytes") + assert _expression(model.c.blob.is_not_null()) + + +# --------------------------------------------------------------------------- +# The operations a family does not offer refuse at construction +# --------------------------------------------------------------------------- + + +def test_operations_of_another_family_are_missing(model): + """A missing operation is an ``AttributeError``, the package's vocabulary.""" + with pytest.raises(AttributeError): + model.c.age.like("%1%") # LIKE belongs to strings + with pytest.raises(AttributeError): + model.c.age.upper() + with pytest.raises(AttributeError, match="no sum"): + model.c.created + model.c.created # two points in time have no sum + + +def test_a_number_result_carries_no_string_operations(model): + """``length`` returns a number, and the number is where the chain stops.""" + with pytest.raises(AttributeError): + model.c.name.length().length() + with pytest.raises(AttributeError): + model.c.name.length().upper() + + +def test_arithmetic_a_string_never_declared_is_a_type_error(model): + """String carries no arithmetic at all, so the interpreter refuses first. + + A family whose refusal is *declared* on the class raises + ``AttributeError`` with a message (the temporal case above); a family + that simply has no operator is refused by Python, which reports + ``TypeError``. Both are construction-time refusals; the difference is + which layer owns the message, and that is what the two tests pin. + """ + with pytest.raises(TypeError): + model.c.name + 1 diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_enum_capability_contracts.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_enum_capability_contracts.py new file mode 100644 index 0000000..26b14c8 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_enum_capability_contracts.py @@ -0,0 +1,117 @@ +# testsuite/feature/query/typed_column/test_enum_capability_contracts.py +"""What a backend says about the generic ``enum`` type. + +The core has an ``EnumType`` and no backend implemented a formatter for it, so +a model declaring an enumerated field failed everywhere. Two things have to +hold afterwards, and they pull in opposite directions: + +* A backend that can render it must say so. Claiming support without a + formatter, or staying silent about a type the backend renders natively, both + leave the caller to guess. +* A backend that cannot must name a substitute it *can* render. A suggestion + pointing at a type the dialect has no formatter for is no suggestion: the + swap fails the same way. + +Neither set may overlap, because a type in both is one of the two a lie. +""" + +# testsuite/feature/query/typed_column/test_enum_capability_contracts.py +import pytest + +from rhosocial.activerecord.backend.expression.types import EnumType + + +@pytest.fixture +def dialect(json_user_fixture): + """The dialect under test, taken from a provider-configured model. + + The same source as the other contracts here: ``__backend__`` is the + instance the provider configured and needs no connection, while + ``Model.backend()`` resolves the currently active backend and raises on a + shard with no live connection. The model arrives as a direct argument + because ``getfixturevalue`` cannot be combined with parametrisation. + """ + return json_user_fixture.__backend__.dialect + + +def _renderable(dialect) -> set: + """The generic type names this dialect can actually render.""" + return set(dialect.supports_data_types()) + + +def _suggested(dialect) -> dict: + return dialect.suggested_data_types() or {} + + +def test_supported_and_suggested_are_disjoint(dialect): + """A name in both is one of the two being a lie.""" + overlap = sorted(_renderable(dialect) & set(_suggested(dialect))) + assert not overlap, ( + f"{dialect.name} both renders and suggests a substitute for {overlap}; " + f"a suggestion exists for a type the dialect cannot render" + ) + + +def test_an_advertised_enum_really_renders(dialect): + """``supports_data_types`` is read as a promise about DDL generation.""" + if "enum" not in _renderable(dialect): + pytest.skip(f"{dialect.name} does not advertise the generic enum type") + sql, _ = dialect.format_data_type(EnumType(dialect, ["a", "b"])) + assert sql, f"{dialect.name} advertised enum and rendered nothing" + + +def test_an_advertised_enum_lists_its_values(dialect): + """The values are the whole point; a bare ENUM() has lost them.""" + if "enum" not in _renderable(dialect): + pytest.skip(f"{dialect.name} does not advertise the generic enum type") + sql, _ = dialect.format_data_type(EnumType(dialect, ["draft", "live"])) + assert "draft" in sql and "live" in sql, sql + + +def test_a_suggested_substitute_is_renderable_here(dialect): + """The suggestion is the only route forward, so it has to work. + + SQL Server suggested EnumType for enum, which is the type it cannot + render: swapping one for the other failed identically and the caller was + sent round in circles with what looked like advice. + """ + substitute = _suggested(dialect).get("enum") + if substitute is None: + pytest.skip(f"{dialect.name} suggests nothing for enum") + assert substitute is not EnumType, ( + f"{dialect.name} suggests EnumType for enum, which it cannot render" + ) + assert "enum" not in _renderable(dialect) or substitute is not EnumType + + +def test_a_substitute_is_dispatchable_here(dialect): + """The substitute needs a formatter on this dialect. + + Checked on the class rather than an instance, because a substitute is + allowed to need arguments: PostgreSQL's enum is a named type created by + CREATE TYPE, so its column type cannot be constructed without a name, and + MySQL's needs its values. Neither is a defect; what has to hold is that + the class dispatches to something this dialect implements. + """ + substitute = _suggested(dialect).get("enum") + if substitute is None: + pytest.skip(f"{dialect.name} suggests nothing for enum") + dispatch = getattr(dialect, f"format_data_type_{substitute.name}", None) + assert dispatch is not None, ( + f"{dialect.name} suggests {substitute.__name__} but has no " + f"format_data_type_{substitute.name}, so the advice cannot be taken" + ) + assert substitute.name in _renderable(dialect) or dispatch is not None + + +def test_refusing_an_enum_names_the_substitute(dialect): + """The message is the only place the answer reaches a caller.""" + substitute = _suggested(dialect).get("enum") + if substitute is None or "enum" in _renderable(dialect): + pytest.skip(f"{dialect.name} does not refuse the generic enum type") + with pytest.raises(TypeError) as excinfo: + dialect.format_data_type(EnumType(dialect, ["a"])) + assert substitute.__name__ in str(excinfo.value), ( + f"{dialect.name} refuses enum without naming {substitute.__name__}: " + f"{excinfo.value}" + ) diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_protocol_guarantees.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_protocol_guarantees.py new file mode 100644 index 0000000..45f3b96 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_protocol_guarantees.py @@ -0,0 +1,324 @@ +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_protocol_guarantees.py +"""The guarantees the column-type protocol claims, asserted per backend. + +``test_typed_column_contracts.py`` answers *what a column class means*: that an +explicit declaration is backend-independent, and that inference follows the +backend's own tables. This file answers a different question — whether the +protocol's own claims hold — and the four are: + +1. **Completeness** — every backend answers every common entry. A hole is a + failure, not a skip: a backend that forgets an entry would otherwise pass + its own suite and fail only on whichever model happened to use the + forgotten type, a failure whose site and whose cause are far apart. +2. **Answer states** — each answer is a column class, or ``None`` for "this + backend genuinely has no workaround here". Anything else — a string, + another type — is a malformed table and is read as a column class by + whichever consumer meets it first. +3. **Orthogonality** — a field's DDL type cannot reach its expressions, + because the column object has no slot for it and carries no value-family + label at all. +4. **No universal column** — an annotation the vocabulary does not cover + fails, naming the way out; it does not silently degrade to a permissive + column. + +Why these are contracts and not unit tests +------------------------------------------ +Each one can be violated by a backend that still passes every test in its own +repository, and the symptom would be a model behaving differently on one server +than on the rest — the exact failure the typed-column work exists to remove. So +they run against every backend, through the provider's configured dialect. +""" + +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_protocol_guarantees.py +import inspect +from typing import Any, Optional, Union + +import pytest + +from rhosocial.activerecord.backend.expression import ( + Column, + ColumnBase, +) +from rhosocial.activerecord.base.field_proxy import ColumnTypeResolutionError +from rhosocial.activerecord.base.fields import UseColumnType + +from rhosocial.activerecord.testsuite.feature.query.typed_column.column_helpers import ( + COMMON_TYPES, + resolve_column_class, +) + + +def _name(entry): + return getattr(entry, "__name__", str(entry)) + + +# --------------------------------------------------------------------------- +# 1. Completeness: a hole is a failure, not a skip +# --------------------------------------------------------------------------- + + +def test_every_common_entry_of_the_protocol_is_answered(dialect): + """Every entry of the shared list gets an answer from this backend. + + The answer may be a real column class or an explicit ``None``: refusing is + a legitimate, measured conclusion, and the explicit ``None`` exists + precisely so that "no" and "unconsidered" stay distinguishable. What is not + allowed is absence, because absence is what an oversight looks like. + """ + table = dialect.suggested_column_types() + unanswered = [entry for entry in COMMON_TYPES if entry not in table] + + assert not unanswered, ( + f"{type(dialect).__name__}.suggested_column_types() leaves " + f"{len(unanswered)} entr{'y' if len(unanswered) == 1 else 'ies'} of the " + f"common list unanswered: {[_name(e) for e in unanswered]}. Answer " + f"None rather than omitting, so a refusal stays distinguishable from " + f"an oversight." + ) + + +def test_an_omitted_entry_fails_at_selection_rather_than_defaulting(): + """A table with a hole is the backend's omission, and nothing papers over it. + + There is no fallback table and no permissive column: the selection either + finds the entry in the backend's own table or fails, telling the caller how + to declare explicitly. Deliberately not parameterised over the backends: + every backend passes the completeness contract above, so forcing a hole + requires a dialect built for the purpose. + + The hole is made by removing ``str`` rather than ``bool``. ``bool`` + subclasses ``int``, so dropping it from the table would leave the subclass + walk to find ``int`` and the omission would be invisible — which is why the + completeness contract above checks for *key presence* rather than for + resolvability. + """ + table = {entry: Column for entry in COMMON_TYPES} + table.pop(str) + + class _WithHole: + NAME = "with_hole" + + def suggested_column_types(self): + return dict(table) + + def suggested_extra_column_types(self): + return {} + + with pytest.raises(ColumnTypeResolutionError) as err: + resolve_column_class(_WithHole(), str) + + message = str(err.value) + assert "UseColumnType" in message, ( + f"An omitted entry must leave the caller a route forward. Got: {message!r}" + ) + + +# --------------------------------------------------------------------------- +# 2. The table has exactly two answer states +# --------------------------------------------------------------------------- + + +def test_the_answer_for_each_entry_is_a_class_or_none(dialect): + """No third state: a column class, or an explicit ``None``. + + ``None`` is the deliberate "this backend genuinely has no column for this + value, and no workaround expresses it" — the last resort, not a first + answer. A table entry that is anything else is read as a column class by + whichever consumer meets it first and would fail much later, in SQL this + contract cannot see. Checking the *table* makes a malformed cell the + backend's own test failure rather than a surprise at the first query. + """ + table = dialect.suggested_column_types() + malformed = [ + (_name(entry), value) + for entry, value in table.items() + if value is not None + and not (isinstance(value, type) and issubclass(value, ColumnBase)) + ] + + assert not malformed, ( + f"{type(dialect).__name__} answers with something that is neither a " + f"column class nor None: {malformed}. The two states are a class and " + f"the explicit None; absence is reserved for entries the protocol does " + f"not carry." + ) + + +def test_every_answered_class_is_constructible(dialect): + """A class in the table is only real if a column can be built from it. + + The table answering correctly and the class refusing to construct would + leave the failure at query time with a message about a column rather than + about the annotation, which is the outcome the narrow column classes were + introduced to remove. + """ + for entry, value in dialect.suggested_column_types().items(): + if value is None: + continue + column = value(dialect, "c") + assert isinstance(column, ColumnBase), ( + f"{type(dialect).__name__} answers {value!r} for {_name(entry)}, " + f"which does not build a column" + ) + + +# --------------------------------------------------------------------------- +# 3. Orthogonality: the DDL type cannot reach the expression +# --------------------------------------------------------------------------- + + +def test_the_column_object_has_no_slot_for_a_fields_data_type(dialect): + """The mechanical form of the orthogonality ruling. + + The column object carries ``(name, column class)`` and its constructor + offers no place to put a field's DataType or a value-family label, so + making an expression depend on one would first require changing the object + model — which is reviewable, unlike a value quietly read out of field + metadata at render time. + """ + parameters = [ + name + for name in inspect.signature(Column.__init__).parameters + if name != "self" + ] + + leaked = [ + name + for name in parameters + if any( + word in name.lower() + for word in ("data_type", "sql_type", "datatype", "value_type") + ) + ] + + assert not leaked, ( + f"Column.__init__ gained a slot a field's DataType or value-family " + f"label could travel through: {leaked}. Orthogonality is enforced by " + f"there being nowhere to put it; adding the parameter is the change " + f"that breaks it, and it must be reviewed as such rather than absorbed." + ) + + +def test_no_value_family_label_remains(dialect): + """The label was removed outright: the column class *is* the value type. + + A string naming the annotation's family used to ride on the column; it was + a tag that could only ever drift from the class, and the class already says + what the value can do. Asserting its absence keeps a future re-introduction + a reviewed change rather than a quiet readdition. + """ + any_class = next( + value + for value in dialect.suggested_column_types().values() + if isinstance(value, type) and issubclass(value, ColumnBase) + ) + assert not hasattr(any_class(dialect, "c"), "value_type") + + +# --------------------------------------------------------------------------- +# 4. There is no universal column +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "annotation", + [ + Any, + Union[int, str], + Optional[Any], + object, + type("Unregistered", (), {}), + ], + ids=lambda v: getattr(v, "__name__", str(v)), +) +def test_an_annotation_outside_the_vocabulary_fails(dialect, annotation): + """An unclassifiable annotation fails, and says how to proceed. + + The permissive ``Column`` that used to catch everything is gone: it would + have offered ``like()`` on an integer field and ``__add__`` on a JSON one, + silently, on a model whose author never asked for either. What replaces it + is a failure that names the escape hatch, so the caller either annotates + with a type the protocol carries or declares the operations explicitly. + + The message is part of the contract: a bare "could not resolve" leaves the + reader guessing which of the two routes to take. + """ + with pytest.raises(ColumnTypeResolutionError) as err: + resolve_column_class(dialect, annotation) + + message = str(err.value) + assert "UseColumnType" in message, ( + f"The failure must name the escape hatch. Got: {message!r}" + ) + + +def test_the_escape_hatch_reaches_past_a_none_answer(dialect): + """Declaring wins before the tables are consulted. + + The whole point of the declaration is that it is the *only* route past a + refusal, so it has to be consulted before the table — otherwise a backend + with an explicit ``None`` for an annotation would be unreachable for that + field even though its value supports operations the backend can render. + Firebird's JSON answer is the recorded case that keeps this honest. + """ + + class _RefusingDict: + NAME = "refusing_dict" + + def suggested_column_types(self): + table = dict(dialect.suggested_column_types()) + table[dict] = None + return table + + def suggested_extra_column_types(self): + return dialect.suggested_extra_column_types() + + with pytest.raises(ColumnTypeResolutionError): + resolve_column_class(_RefusingDict(), dict) + + declared = UseColumnType(Column) + assert resolve_column_class(_RefusingDict(), dict, declared) is Column + + +# --------------------------------------------------------------------------- +# The one string operation whose unit is not portable +# --------------------------------------------------------------------------- + + +def test_length_renders_one_plain_function_call(dialect): + """``LENGTH`` stays one bare call, with no unit clause smuggled in. + + What ``.length`` *means* differs by backend — characters on PostgreSQL, + Oracle and Firebird, bytes on the MySQL family and ClickHouse, UTF-16 code + units on SQL Server — and that difference cannot be closed without a live + server, so it is documented rather than tested here. What *can* be pinned + locally is the syntax: a backend that started emitting ``CHAR_LENGTH(x + USING CHARACTERS)``, or wrapped the call in a cast, or split it into two + calls, would be choosing a unit by rendering rather than declaring it. + + That is the failure this contract is for. A dialect is free to rename the + function — SQL Server's ``LENGTH`` becomes ``LEN`` — because a rename is + still one call with the same argument, and a reader comparing two backends + can see what happened. A unit clause would be invisible in the same + comparison and would change the answer. + """ + table = dialect.suggested_column_types() + column_class = table.get(str) + if not (isinstance(column_class, type) and issubclass(column_class, ColumnBase)): + pytest.skip(f"{type(dialect).__name__} does not answer a class for str") + column = column_class(dialect, "s") + + sql, params = column.length().to_sql() + + assert params == (), f"length() must bind nothing, got {params!r}" + assert sql.count("(") == 1, ( + f"length() must render one call, got {sql!r}. A second call or a " + f"nested function is a unit being chosen by rendering." + ) + assert "USING" not in sql.upper(), ( + f"length() must not carry a unit clause — the unit is the backend's " + f"fact to record, not this call's to assert. Got {sql!r}" + ) + assert dialect.format_identifier("s") in sql, ( + f"length() must measure the column itself, got {sql!r}" + ) diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_typed_column_contracts.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_typed_column_contracts.py new file mode 100644 index 0000000..779dd68 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_typed_column_contracts.py @@ -0,0 +1,388 @@ +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_typed_column_contracts.py +"""Type-narrowed columns must behave identically on every backend. + +The point of narrowing a column by its field's Python annotation is that the +*same model code* behaves the same everywhere. A backend that renamed a class, +returned a different base, or rendered different SQL would break that +promise while still passing its own dialect tests, so these contracts run +against every backend. + +Nothing here needs a live database: every assertion is on the expression tree +or on SQL produced from a dialect that was never connected to. + +Two contracts, not one +---------------------- +Which column class an annotation means is the **backend's answer** (through +the ``ColumnTypeSupport`` tables), so "the class is backend-independent" holds +only for explicit declarations: + +* an **explicit** ``UseColumnType(SomeColumn)`` is backend-independent by + contract -- naming a class is a statement about operations, and the + operations are the same everywhere. A backend that disagreed here would be + changing what a column class *means*, which the typed_column contracts + forbid everywhere; +* **inference** is per backend. Each dialect's ``suggested_column_types()`` is + the authority, and what the selection builds must equal that table's answer + for the annotation. A backend that suggests ``ArrayColumn`` for ``list`` + where another suggests ``JSONColumn`` is not breaking a contract -- it is + reporting a real difference in what those servers can do. + +The inference cases below use *exact* table keys (``str`` is the ``str`` +entry), so comparing against the table by key does not restate the +normalisation rules. +""" + +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_typed_column_contracts.py +import datetime +import decimal +import uuid +from typing import Any, Optional + +import pytest + +from rhosocial.activerecord.backend.expression import ( + ArrayColumn, + BinaryColumn, + BooleanColumn, + Column, + ColumnBase, + ComparisonPredicate, + InPredicate, + IsNullPredicate, + Literal, + TimestampColumn, + IntegerColumn, + JSONColumn, + NumericColumn, + StringColumn, + UUIDColumn, + build_json_path, +) +from rhosocial.activerecord.base.fields import UseColumnType + +from rhosocial.activerecord.testsuite.feature.query.typed_column.column_helpers import ( + build_column, +) + + +def _class_or_skip(dialect, annotation, column_name): + """The class this dialect answers for *annotation*, or a skip when it refuses. + + ``None`` is a legitimate answer -- the backend genuinely has no column for + the value, and the field needs an explicit declaration -- so a test about + the class *surface* skips rather than fails. A *missing* key is different: + that is an omission, whatever the reason, and it fails here. + + The annotation is used as an exact table key (``str`` is the ``str`` + entry); these callers pass the very types the table is keyed by. + """ + table = dialect.suggested_column_types() + assert annotation in table, ( + f"{type(dialect).__name__}.suggested_column_types() does not answer " + f"for {annotation!r}; every common entry must be answered (see " + f"test_protocol_guarantees.py)." + ) + if table[annotation] is None: + pytest.skip(f"{type(dialect).__name__} reports no column class for {annotation!r}") + return build_column(dialect, column_name, annotation) + + +# --------------------------------------------------------------------------- +# Contract 1: an explicit declaration is backend-independent +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "annotation, column_class", + [ + (str, StringColumn), + (int, IntegerColumn), + (dict, JSONColumn), + (list, ArrayColumn), + (uuid.UUID, UUIDColumn), + (datetime.datetime, TimestampColumn), + ], + ids=lambda v: getattr(v, "__name__", str(v)), +) +def test_an_explicit_declaration_is_the_same_class_everywhere(dialect, annotation, column_class): + """``UseColumnType(X)`` means ``X`` on every backend, whatever the annotation. + + This is the contract that makes a model portable, and it survives the move + of inference onto the backend's tables: naming a class says which operations + the value carries, and those are the same on every backend. A backend that + answered anything else here would be redefining a column class rather than + reporting what it can do -- and the annotation is deliberately *not* the one + the class belongs to, so nothing about the backend's table can leak into the + answer. + """ + declared = UseColumnType(column_class) + assert type(build_column(dialect, "f", annotation, column_type=declared)) is column_class + + +def test_a_declaration_overrides_an_annotation_the_backend_cannot_place(dialect): + """The escape hatch works even where the backend's own answer is absent. + + Declaring is the route past a table answering ``None``, so it has to reach + resolution before the table is consulted -- which is also what makes it + usable for a backend that has no native column for the annotation. + """ + declared = UseColumnType(JSONColumn) + assert type(build_column(dialect, "f", dict, column_type=declared)) is JSONColumn + + +# --------------------------------------------------------------------------- +# Contract 2: inference follows this backend's own table +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "annotation", + [ + str, + int, + float, + decimal.Decimal, + bool, + bytes, + dict, + list, + datetime.datetime, + datetime.date, + uuid.UUID, + ], + ids=lambda v: getattr(v, "__name__", str(v)), +) +def test_inference_matches_this_backends_own_table(dialect, annotation): + """What a model builds is what this dialect's table says, not a fixed class. + + The assertion is table-relative on purpose. Pinning each annotation to one + class across all backends was the old contract, and it was wrong: seven + backends carry ``list`` as a JSON document while PostgreSQL has a real array + type, and both are correct answers to "what can this value do here". What is + *not* negotiable is that the selection and the table agree -- otherwise the + table is documentation rather than the source of the answer. + """ + table = dialect.suggested_column_types() + assert annotation in table, ( + f"{type(dialect).__name__}.suggested_column_types() does not answer " + f"for {annotation!r}; every common entry must be answered (see " + f"test_protocol_guarantees.py)." + ) + + expected = table[annotation] + if expected is None: + pytest.skip(f"{type(dialect).__name__} reports no column class for {annotation!r}") + + assert type(build_column(dialect, "f", annotation)) is expected + + +@pytest.mark.parametrize("annotation", [Optional[str], Optional[dict], Optional[int]]) +def test_optional_is_transparent(dialect, annotation): + """``Optional[T]`` must resolve exactly as ``T`` does on this backend. + + Compared against the same dialect's own answer for ``T`` rather than against + a class, for the same reason the inference contract above is: the answer may + legitimately vary by backend, and what must not vary is the *effect* of + wrapping the annotation in ``Optional``. + """ + inner = { + Optional[str]: str, + Optional[dict]: dict, + Optional[int]: int, + }[annotation] + resolved = _class_or_skip(dialect, inner, "f") + assert type(build_column(dialect, "f", annotation)) is type(resolved) + + +@pytest.mark.parametrize( + "column_class", + [ + Column, + StringColumn, + NumericColumn, + TimestampColumn, + BooleanColumn, + BinaryColumn, + UUIDColumn, + JSONColumn, + ArrayColumn, + ], + ids=lambda c: c.__name__, +) +def test_every_column_class_shares_one_base(dialect, column_class): + """The eight isinstance() consumers test for ColumnBase, so all must match. + + A class that slipped out of the hierarchy would be silently rejected by + whichever consumer met it first — a RETURNING clause, an UPDATE SET, an + upsert — and the symptom would look like an unrelated backend bug. + """ + assert isinstance(column_class(dialect, "c"), ColumnBase) + + +def test_the_bare_column_offers_no_operation(dialect): + """The hand-built ``Column`` is a reference, and it promises nothing. + + Nothing established what it holds, so there is no operation it can offer + that the database is guaranteed to accept. The operations live on the typed + classes; a caller holding a bare column name builds its predicate + explicitly instead -- ``ComparisonPredicate``, ``InPredicate``, + ``IsNullPredicate`` -- and those ask the *dialect* for the SQL, so they work + on any column. + + The failure this replaces is the reason the contract is stated as an + absence. ``==`` is not overridden, so ``Column(...) == 1`` does not raise: + it answers Python's identity ``False``, and a query built on that filters on + nothing while looking entirely healthy. + """ + col = Column(dialect, "anything") + for operation in ("like", "ilike", "in_", "between", "is_true", "json_path", "__add__", "__mul__"): + assert not hasattr(col, operation), f"Column grew {operation}" + + assert (col == Column(dialect, "anything")) is False, "identity comparison, not SQL" + assert (col == 1) is False, "identity comparison, not SQL" + + # What it *does* keep is the shared reference surface: naming a column needs + # no type, and neither does rendering one. + assert Column.__mro__[1] is ColumnBase + for operation in ("as_", "is_null", "is_not_null", "cast"): + assert hasattr(col, operation), f"Column lost {operation}" + + # ...and a model that declares it gets it, narrow nothing implied. + declared = build_column(dialect, "anything", Any, column_type=UseColumnType(Column)) + assert type(declared) is Column + + +def test_a_bare_column_still_takes_an_explicit_predicate(dialect): + """The replacement path: the query layer builds the predicate, the column only names it. + + This is what callers of the old ``Column(...) == 1`` sugar write now, and it + is why retiring the sugar costs nothing: the predicate needs the dialect, + not a capability on the column, so it renders for any column. + """ + col = Column(dialect, "status") + ident = dialect.format_identifier("status") + assert ComparisonPredicate(dialect, "=", col, Literal(dialect, "active")).to_sql() == (f"{ident} = ?", ("active",)) + assert InPredicate(dialect, col, Literal(dialect, ("open", "paid"))).to_sql() == ( + f"{ident} IN (?, ?)", + ("open", "paid"), + ) + assert IsNullPredicate(dialect, col).to_sql() == (f"{ident} IS NULL", ()) + + +def test_narrowing_actually_narrows(dialect): + """The narrowed classes must not leak operations they cannot support. + + Built through the backend's own table, so this also says the operations + follow from the class the backend chose rather than from the annotation: a + backend that suggests ``StringColumn`` for an ``int`` field would offer + ``like()`` on it, and that is a fact about the pairing, not about this + contract. + """ + string_column = _class_or_skip(dialect, str, "s") + assert hasattr(string_column, "like") + assert not hasattr(string_column, "__add__") + + integer_column = _class_or_skip(dialect, int, "n") + assert hasattr(integer_column, "__add__") + assert not hasattr(integer_column, "like") + + json_column = _class_or_skip(dialect, dict, "j") + assert hasattr(json_column, "json_path") + assert not hasattr(json_column, "like") + assert not hasattr(json_column, "__add__") + + +# --------------------------------------------------------------------------- +# Rendering is unchanged by narrowing +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "column_class", + [ + Column, + StringColumn, + NumericColumn, + TimestampColumn, + BooleanColumn, + BinaryColumn, + UUIDColumn, + JSONColumn, + ArrayColumn, + ], + ids=lambda c: c.__name__, +) +def test_rendering_is_identical_across_column_classes(dialect, column_class): + """Narrowing changed the API surface, not the SQL. + + Compared against a plain Column on the same dialect rather than against a + hardcoded string: how a backend qualifies a name is its own business, and + BigQuery renders a path as one quoted identifier where most render three. + The invariant under test is that the typed layers render exactly what the + untyped one does. + """ + typed = column_class(dialect, "settings", table="t", schema_name="s") + plain = Column(dialect, "settings", table="t", schema_name="s") + assert typed.to_sql() == plain.to_sql() + + +def test_qualified_and_aliased_forms(dialect): + ident = dialect.format_identifier + assert Column(dialect, "c").to_sql() == (ident("c"), ()) + assert Column(dialect, "c", table="t").to_sql() == ( + f'{ident("t")}.{ident("c")}', + (), + ) + assert Column(dialect, "c", alias="a").to_sql() == ( + f'{ident("c")} AS {ident("a")}', + (), + ) + + +# --------------------------------------------------------------------------- +# JSON path building is one shared implementation +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "keys, expected", + [ + (("a",), "$.a"), + (("a", "b"), "$.a.b"), + (("tags", 0), "$.tags[0]"), + (("a", 0, "b"), "$.a[0].b"), + (("tags", "[0]"), "$.tags[0]"), + ], + ids=lambda v: str(v), +) +def test_json_path_is_built_the_same_everywhere(keys, expected): + """Path syntax must not vary per backend — it is one function.""" + assert build_json_path(*keys) == expected + + +def test_int_index_and_str_key_stay_distinct(): + """``"0"`` names a key; only an int is an index. + + ``$.tags.0`` matches nothing, so the distinction rides on the type rather + than the spelling. + """ + assert build_json_path("tags", "0") == "$.tags.0" + assert build_json_path("tags", 0) == "$.tags[0]" + + +def test_bool_is_rejected_as_a_path_segment(): + """bool subclasses int, so True would silently become index 1.""" + with pytest.raises(TypeError, match="bool"): + build_json_path("a", True) + + +def test_json_chaining_does_not_renest_the_path(dialect): + """Chaining accumulates into one path per step, never ``$``-anchored again. + + Re-anchoring would make ``col.json_value("a").json_value("b")`` read + ``b`` from the document root instead of from ``a``. + """ + col = _class_or_skip(dialect, dict, "settings") + chained = col.json_value("a").json_value("b").to_sql()[0] + assert chained.count("$") == 2 + assert "$." not in chained.replace("$.a", "").replace("$.b", "") diff --git a/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_uuid_capability_contracts.py b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_uuid_capability_contracts.py new file mode 100644 index 0000000..8f4ce1b --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_uuid_capability_contracts.py @@ -0,0 +1,232 @@ +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_uuid_capability_contracts.py +"""UUID capabilities must be consistent, and honest when absent. + +Storage and generation are separate concerns. ``UUIDType`` says how a value +is stored; ``UUIDSupport`` says whether the database can produce one. A +backend that advertises generation and then emits another backend's function +is worse than one that admits it cannot, because the caller's query fails at +runtime instead of at capability-check time. + +No live database is needed: every assertion is on rendered SQL. +""" + +# src/rhosocial/activerecord/testsuite/feature/query/typed_column/test_uuid_capability_contracts.py +import pytest + +from rhosocial.activerecord.backend.dialect.exceptions import UnsupportedFeatureError +from rhosocial.activerecord.backend.expression import ( + Literal, + UUIDCastExpression, + UUIDConstantExpression, + UUIDGenerationExpression, +) + + +@pytest.fixture +def dialect(json_user_fixture): + """The dialect under test, taken from a provider-configured model. + + Read off ``__backend__`` rather than calling ``Model.backend()``: that + resolves the *currently active* backend and raises "No backend configured" + on a shard with no live connection, which is the situation here — every + assertion is on an expression tree or on SQL from a dialect that was never + connected. ``__backend__`` is the instance the provider configured and + needs no connection. + + The model arrives as a direct argument rather than through + ``request.getfixturevalue``, because that cannot be combined with + ``@pytest.mark.parametrize``: a parametrised test has no fixture parameter + to resolve, and pytest rejects the request. + """ + return json_user_fixture.__backend__.dialect + + +_NIL = "00000000-0000-0000-0000-000000000000" +_MAX = "ffffffff-ffff-ffff-ffff-ffffffffffff" + + +# --------------------------------------------------------------------------- +# Probe / implementation agreement +# --------------------------------------------------------------------------- + + +def _supports(dialect, probe: str) -> bool: + """Whether *dialect* advertises *probe*, treating absence as "no". + + A backend that does not implement UUID at all declares no probe, which is + the same answer as declaring False. Reading it with a bare getattr() turns + a capability question into an AttributeError, which is the confusion this + contract exists to catch — so it reads it the way the expression layer + does. + """ + found = getattr(dialect, probe, None) + return bool(found()) if callable(found) else False + + +@pytest.mark.parametrize( + "probe, factory", + [ + ("supports_uuid_generation", lambda d: UUIDGenerationExpression(d)), + ("supports_uuid_constant", lambda d: UUIDConstantExpression(d, "nil")), + ("supports_uuid_cast", lambda d: UUIDCastExpression(d, Literal(d, "x"))), + ], + ids=["generation", "constant", "cast"], +) +def test_supported_uuid_operation_renders(dialect, probe, factory): + """A True probe must render, not refuse.""" + if not _supports(dialect, probe): + pytest.skip(f"{dialect.name} does not advertise {probe}") + + sql, params = factory(dialect).to_sql() + assert sql, f"{dialect.name} advertised {probe} but rendered nothing" + + +@pytest.mark.parametrize( + "probe, factory", + [ + ("supports_uuid_generation", lambda d: UUIDGenerationExpression(d)), + ("supports_uuid_constant", lambda d: UUIDConstantExpression(d, "nil")), + ("supports_uuid_cast", lambda d: UUIDCastExpression(d, Literal(d, "x"))), + ], + ids=["generation", "constant", "cast"], +) +def test_unsupported_uuid_operation_refuses(dialect, probe, factory): + """A False probe must refuse, rather than emit a guess. + + Refusing is the whole value of an honest probe: the caller can pick + another route instead of shipping SQL the server will reject. + """ + if _supports(dialect, probe): + pytest.skip(f"{dialect.name} advertises {probe}") + + with pytest.raises(UnsupportedFeatureError): + factory(dialect).to_sql() + + +def test_refusal_carries_a_suggestion(dialect): + """An error the caller cannot act on is only marginally better than wrong SQL.""" + if _supports(dialect, "supports_uuid_generation"): + pytest.skip(f"{dialect.name} can generate UUIDs") + + with pytest.raises(UnsupportedFeatureError) as excinfo: + UUIDGenerationExpression(dialect).to_sql() + + suggestion = str(excinfo.value) + assert "uuid" in suggestion.lower(), ( + f"{dialect.name} refusal names no route forward: {suggestion!r}" + ) + + +# --------------------------------------------------------------------------- +# No cross-backend leakage +# --------------------------------------------------------------------------- + + +def test_generation_emits_no_other_backends_function(dialect): + """Each backend's generator is its own; borrowing one fails at runtime.""" + if not _supports(dialect, "supports_uuid_generation"): + pytest.skip(f"{dialect.name} cannot generate UUIDs") + + sql, _ = UUIDGenerationExpression(dialect).to_sql() + foreign = { + "Postgres": ("uuid_generate_v1", "uuid_generate_v4"), + "SQLServer": ("NEWID", "NEWSEQUENTIALID"), + "MySQL": ("UUID()",), + "MariaDB": ("UUID()",), + "Oracle": ("SYS_GUID()",), + "Snowflake": ("UUID_STRING()",), + "BigQuery": ("GENERATE_UUID()",), + "ClickHouse": ("generateUUIDv4",), + "Firebird": ("GEN_UUID()",), + } + borrowed = foreign.get(dialect.name, ()) + for name in borrowed: + assert name.lower() not in sql.lower(), ( + f"{dialect.name} rendered {sql!r}, which is another backend's " + f"generator ({name})" + ) + + +def test_generation_is_aliasable(dialect): + if not _supports(dialect, "supports_uuid_generation"): + pytest.skip(f"{dialect.name} cannot generate UUIDs") + + sql, _ = UUIDGenerationExpression(dialect, alias="u").to_sql() + ident = dialect.format_identifier("u") + assert sql.endswith(f"AS {ident}") + assert sql.startswith(ident) is False, "the alias must not replace the expression" + + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("which, value", [("nil", _NIL), ("max", _MAX)]) +def test_constants_carry_the_right_value(dialect, which, value): + """The literal must be in the statement, not only in Python. + + A constant that renders as a placeholder would compare every row against + NULL. + """ + if not _supports(dialect, "supports_uuid_constant"): + pytest.skip(f"{dialect.name} has no UUID constants") + + sql, _ = UUIDConstantExpression(dialect, which).to_sql() + assert value in sql, f"{dialect.name} rendered {sql!r} without the {which} value" + + +def test_unknown_constant_kind_is_rejected(dialect): + """A typo must not silently render something else.""" + with pytest.raises(ValueError, match="which must be one of"): + UUIDConstantExpression(dialect, "middle") + + +# --------------------------------------------------------------------------- +# Cast +# --------------------------------------------------------------------------- + + +def test_cast_keeps_its_parameters(dialect): + """The operand must stay bound, not be inlined into the statement.""" + if not _supports(dialect, "supports_uuid_cast"): + pytest.skip(f"{dialect.name} cannot cast to UUID") + + expr = UUIDCastExpression(dialect, Literal(dialect, "not-a-uuid")) + sql, params = expr.to_sql() + assert params == ("not-a-uuid",) + assert "not-a-uuid" not in sql, f"{dialect.name} inlined the operand: {sql!r}" + + +# --------------------------------------------------------------------------- +# Storage is a separate concern from generation +# --------------------------------------------------------------------------- + + +def test_storage_capability_is_declared_independently(dialect): + """A backend may store UUIDs without being able to generate them. + + SQLite is the motivating case: it stores a UUID fine as text and has no + UUID function at all, so the two probes must be allowed to disagree. + """ + from rhosocial.activerecord.backend.expression.types import UUIDType + + supported = dialect.supports_data_types() + suggested = dialect.suggested_data_types() + stores_natively = "uuid" in supported + has_substitute = "uuid" in suggested + + # Only the combination is wrong. A dialect may render the type, or suggest + # a substitute, or say nothing about UUID at all — declaring neither is a + # legitimate answer and is skipped below. Asserting the two differ would + # reject that third case before the skip could catch it. + assert not (stores_natively and has_substitute), ( + f"{dialect.name} both renders the uuid type and suggests a substitute " + f"for it; a suggestion exists for types the dialect cannot render" + ) + if not stores_natively and not has_substitute: + pytest.skip(f"{dialect.name} declares no UUID storage at all") + + if stores_natively: + rendered = dialect.format_data_type(UUIDType()) + assert rendered[0] diff --git a/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions.py b/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions.py index 5e59cc8..9287ebc 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions.py @@ -6,10 +6,10 @@ from rhosocial.activerecord.testsuite.utils import requires_window_functions from rhosocial.activerecord.backend.expression.advanced_functions import ( - WindowFunctionCall, WindowSpecification, WindowFrameSpecification, ) +from rhosocial.activerecord.backend.expression.core import FunctionCall from rhosocial.activerecord.backend.expression.query_parts import OrderByClause from rhosocial.activerecord.backend.expression import core @@ -37,7 +37,7 @@ def test_row_number_window_function(order_fixtures): order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "DESC")]) window_spec = WindowSpecification(dialect, order_by=order_by_clause) - window_func = WindowFunctionCall( + window_func = FunctionCall( dialect, "ROW_NUMBER", window_spec=window_spec, @@ -91,7 +91,7 @@ def test_partition_by_window_function(order_fixtures): partition_by = [core.Column(dialect, "status")] order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "ASC")]) window_spec = WindowSpecification(dialect, partition_by=partition_by, order_by=order_by_clause) - window_func = WindowFunctionCall( + window_func = FunctionCall( dialect, "ROW_NUMBER", window_spec=window_spec, @@ -136,7 +136,7 @@ def test_aggregate_window_functions(order_fixtures): order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "DESC")]) window_spec = WindowSpecification(dialect, order_by=order_by_clause) - sum_func = WindowFunctionCall(dialect, "SUM", args=[core.Column(dialect, "total_amount")], window_spec=window_spec, alias="running_sum") + sum_func = FunctionCall(dialect, "SUM", core.Column(dialect, "total_amount"), window_spec=window_spec, alias="running_sum") query = Order.query().select("id", "total_amount", sum_func) query.order_by(("total_amount", "DESC")) @@ -177,7 +177,7 @@ def test_named_window_definitions(order_fixtures): order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "DESC")]) window_spec = WindowSpecification(dialect, order_by=order_by_clause) - window_func = WindowFunctionCall( + window_func = FunctionCall( dialect, "ROW_NUMBER", window_spec=window_spec, @@ -222,10 +222,10 @@ def test_window_frame_specifications(order_fixtures): window_frame = WindowFrameSpecification(dialect, 'ROWS', 'UNBOUNDED PRECEDING', 'CURRENT ROW') window_spec = WindowSpecification(dialect, order_by=order_by_clause, frame=window_frame) - first_value_func = WindowFunctionCall( + first_value_func = FunctionCall( dialect, "FIRST_VALUE", - args=[core.Column(dialect, "total_amount")], + core.Column(dialect, "total_amount"), window_spec=window_spec, alias="first_amount" ) @@ -268,10 +268,10 @@ def test_unbounded_window_frames(order_fixtures): window_frame = WindowFrameSpecification(dialect, 'ROWS', 'UNBOUNDED PRECEDING', 'UNBOUNDED FOLLOWING') window_spec = WindowSpecification(dialect, order_by=order_by_clause, frame=window_frame) - sum_func = WindowFunctionCall( + sum_func = FunctionCall( dialect, "SUM", - args=[core.Column(dialect, "total_amount")], + core.Column(dialect, "total_amount"), window_spec=window_spec, alias="total_sum" ) diff --git a/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions_async.py b/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions_async.py index 4a34383..a09d54b 100644 --- a/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions_async.py +++ b/src/rhosocial/activerecord/testsuite/feature/query/window_functions/test_window_functions_async.py @@ -6,10 +6,10 @@ from rhosocial.activerecord.testsuite.utils import requires_window_functions from rhosocial.activerecord.backend.expression.advanced_functions import ( - WindowFunctionCall, WindowSpecification, WindowFrameSpecification, ) +from rhosocial.activerecord.backend.expression.core import FunctionCall from rhosocial.activerecord.backend.expression.query_parts import OrderByClause from rhosocial.activerecord.backend.expression import core @@ -37,7 +37,7 @@ async def test_row_number_window_function(async_order_fixtures): order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "DESC")]) window_spec = WindowSpecification(dialect, order_by=order_by_clause) - window_func = WindowFunctionCall( + window_func = FunctionCall( dialect, "ROW_NUMBER", window_spec=window_spec, @@ -91,7 +91,7 @@ async def test_partition_by_window_function(async_order_fixtures): partition_by = [core.Column(dialect, "status")] order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "ASC")]) window_spec = WindowSpecification(dialect, partition_by=partition_by, order_by=order_by_clause) - window_func = WindowFunctionCall( + window_func = FunctionCall( dialect, "ROW_NUMBER", window_spec=window_spec, @@ -136,7 +136,7 @@ async def test_aggregate_window_functions(async_order_fixtures): order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "DESC")]) window_spec = WindowSpecification(dialect, order_by=order_by_clause) - sum_func = WindowFunctionCall(dialect, "SUM", args=[core.Column(dialect, "total_amount")], window_spec=window_spec, alias="running_sum") + sum_func = FunctionCall(dialect, "SUM", core.Column(dialect, "total_amount"), window_spec=window_spec, alias="running_sum") query = AsyncOrder.query().select("id", "total_amount", sum_func) query.order_by(("total_amount", "DESC")) @@ -177,7 +177,7 @@ async def test_named_window_definitions(async_order_fixtures): order_by_clause = OrderByClause(dialect, [(core.Column(dialect, "total_amount"), "DESC")]) window_spec = WindowSpecification(dialect, order_by=order_by_clause) - window_func = WindowFunctionCall( + window_func = FunctionCall( dialect, "ROW_NUMBER", window_spec=window_spec, @@ -222,10 +222,10 @@ async def test_window_frame_specifications(async_order_fixtures): window_frame = WindowFrameSpecification(dialect, 'ROWS', 'UNBOUNDED PRECEDING', 'CURRENT ROW') window_spec = WindowSpecification(dialect, order_by=order_by_clause, frame=window_frame) - first_value_func = WindowFunctionCall( + first_value_func = FunctionCall( dialect, "FIRST_VALUE", - args=[core.Column(dialect, "total_amount")], + core.Column(dialect, "total_amount"), window_spec=window_spec, alias="first_amount" ) @@ -268,10 +268,10 @@ async def test_unbounded_window_frames(async_order_fixtures): window_frame = WindowFrameSpecification(dialect, 'ROWS', 'UNBOUNDED PRECEDING', 'UNBOUNDED FOLLOWING') window_spec = WindowSpecification(dialect, order_by=order_by_clause, frame=window_frame) - sum_func = WindowFunctionCall( + sum_func = FunctionCall( dialect, "SUM", - args=[core.Column(dialect, "total_amount")], + core.Column(dialect, "total_amount"), window_spec=window_spec, alias="total_sum" ) diff --git a/src/rhosocial/activerecord/testsuite/feature/relation/fixtures/models.py b/src/rhosocial/activerecord/testsuite/feature/relation/fixtures/models.py index 1f02c19..cd7486b 100644 --- a/src/rhosocial/activerecord/testsuite/feature/relation/fixtures/models.py +++ b/src/rhosocial/activerecord/testsuite/feature/relation/fixtures/models.py @@ -13,7 +13,7 @@ from rhosocial.activerecord.model import ActiveRecord, AsyncActiveRecord from rhosocial.activerecord.base import DerivedField, FieldProxy -from rhosocial.activerecord.backend.expression import Column, Literal +from rhosocial.activerecord.backend.expression import BinaryArithmeticExpression, Column, Literal from rhosocial.activerecord.backend.expression.functions import ( json_extract_text, length, concat, coalesce ) @@ -159,7 +159,7 @@ class Post(ActiveRecord): # DerivedField: hotness score hotness: ClassVar[Annotated[int, DerivedField( - lambda d: Column(d, "view_count") + Literal(d, 1), + lambda d: BinaryArithmeticExpression(d, "+", Column(d, "view_count"), Literal(d, 1)), )]] # DerivedField (JSON): first tag from metadata @@ -254,7 +254,7 @@ class AsyncPost(AsyncActiveRecord): )]] hotness: ClassVar[Annotated[int, DerivedField( - lambda d: Column(d, "view_count") + Literal(d, 1), + lambda d: BinaryArithmeticExpression(d, "+", Column(d, "view_count"), Literal(d, 1)), )]] first_tag: ClassVar[Annotated[str, DerivedField( diff --git a/src/rhosocial/activerecord/testsuite/utils/expression.py b/src/rhosocial/activerecord/testsuite/utils/expression.py index 17cc50b..b20bd2c 100644 --- a/src/rhosocial/activerecord/testsuite/utils/expression.py +++ b/src/rhosocial/activerecord/testsuite/utils/expression.py @@ -96,6 +96,13 @@ def _placeholder_for(param: inspect.Parameter, dialect: Any = None): return _values_source(dialect) if raw is str or "str" in text: return "x" + # A typed value expression wraps the call it renders, so the parameter + # wants a FunctionCall and not a bare name. This must be tested before + # the str branch above: "FunctionCall" contains no "str", but the + # fallback below returns "x" for it, which the wrapper then calls + # to_sql() on. + if "FunctionCall" in text: + return _function_call(dialect) if "DataType" in text or "Type" in text.split(".")[-1:]: from rhosocial.activerecord.backend.expression.types import IntegerType @@ -127,6 +134,20 @@ def _literal(dialect): return Literal(dialect, 1) +def _function_call(dialect): + """A niladic function call, for a parameter that wraps one. + + The typed value expressions (``BooleanValueExpression`` and its siblings) + hold a ``call`` and render it through ``to_sql()``. Handing them a string + builds an instance that cannot render, and the failure surfaces as + ``'str' object has no attribute 'to_sql'`` from inside the wrapper rather + than as a construction problem. + """ + from rhosocial.activerecord.backend.expression.core import FunctionCall + + return FunctionCall(dialect, "FUNC", niladic=True) + + def _comparison(dialect): from rhosocial.activerecord.backend.expression.core import Column from rhosocial.activerecord.backend.expression.predicates import ComparisonPredicate @@ -180,6 +201,28 @@ def special_constructors(): placeholder values. Keyed by a suffix that matches the class's full module.ClassName. Registered by name only — this is a static registry and backends may register their own entries for backend-specific expressions. + + **A key that matches no class is dead code, and nothing says so.** + :func:`make_instance` dispatches on ``full_name.endswith(key)``, so + renaming a class turns its entry into a literal that matches nothing: the + class then falls through to :func:`_try_construct`, the source is reported + as ``"heuristic"`` instead of ``"special"``, and if the heuristic also + fails the class is silently reported as unconstructible rather than as + mis-registered. This is how ``"advanced_functions.JSONExpression"`` came to + match nothing at all once core split that one class into + ``JSONDocumentExpression`` and ``JSONTextExpression``. + + The failure is one-directional, which is what makes it dangerous: renaming + the key once breaks the match silently, and a *single* key covering both + new names is not expressible, because ``endswith`` compares a suffix. A + class that was split needs one entry per resulting class. + + :func:`register_special_constructor` writes to the module-level + ``_SPECIAL_REGISTRY``, which is what :func:`make_instance` reads. Note that + this function rebuilds its dict on every call, so a registration made + elsewhere is **not** visible to a caller that invokes + ``special_constructors()`` directly — core's expression-contract test does + exactly that, and therefore sees only the literals below. """ return { "statements.ddl_table.ColumnDefinition": _column_definition, @@ -199,7 +242,10 @@ def special_constructors(): "statements.ddl_schema.DropSchemaExpression": _drop_schema_expr, "statements.ddl_database.CreateDatabaseExpression": _create_database_expr, "statements.ddl_database.DropDatabaseExpression": _drop_database_expr, - "advanced_functions.JSONExpression": _json_expr, + # core splits what was one JSONExpression into a document and a text + # class, so one key cannot cover both: endswith matches a suffix. + "advanced_functions.JSONDocumentExpression": _json_document_expr, + "advanced_functions.JSONTextExpression": _json_text_expr, "query_parts.JoinClause": _join_expr, "statements.ddl_partition.PartitionClause": _partition_clause, "statements.dml.OnConflictClause": _on_conflict, @@ -207,6 +253,7 @@ def special_constructors(): "statements.dml.InsertExpression": _insert_expr, "statements.dml.DeleteExpression": _delete_expr, "statements.ddl_alter.AlterTableExpression": _alter_table_expr, + "types.array.ArrayType": _array_type, "statements.ddl_view.CreateViewExpression": _create_view_expr, "graph.GraphVertex": _graph_vertex, "graph.GraphEdge": _graph_edge, @@ -233,11 +280,53 @@ def _column_definition(dialect): return ColumnDefinition(dialect, "col", IntegerType(dialect)) -def _values_source(dialect): - """An INSERT's ``VALUES`` source: one row, one literal. +def _json_document_expr(dialect): + """``->``: a JSON *document*, reached by path. - The generic placeholder cannot build this one -- ``values_list`` must be a - non-empty list of equal-length rows -- so it gets an explicit constructor. + ``path`` and ``operation`` keep their defaults, so this renders exactly + what the pre-split single ``JSONExpression`` factory rendered. + """ + from rhosocial.activerecord.backend.expression.advanced_functions import ( + JSONDocumentExpression, + ) + from rhosocial.activerecord.backend.expression.core import Literal + + return JSONDocumentExpression(dialect, Literal(dialect, '{"a": 1}'), path="$.a") + + +def _json_text_expr(dialect): + """``->>``: the *text* of a JSON path. + + ``JSONTextExpression`` declares no ``__init__`` of its own, so it inherits + the document class's -- including ``operation="->"``. The default is + therefore inherited too, and this factory has to say ``->>`` explicitly. + Leaving it off would hand the contract test a ``JSONTextExpression`` whose + ``operation`` says ``->``, which is the one thing the class exists to + distinguish. + + The rendered SQL is unaffected either way: the formatters branch on + ``expr.operation``, not on the class, and dispatch is on ``format_method``, + which both classes share. + """ + from rhosocial.activerecord.backend.expression.advanced_functions import ( + JSONTextExpression, + ) + from rhosocial.activerecord.backend.expression.core import Literal + + return JSONTextExpression( + dialect, Literal(dialect, '{"a": 1}'), path="$.a", operation="->>" + ) + + +def _values_source(dialect): + """A ``VALUES`` clause with one row of one column. + + ``ValuesSource`` rejects an empty ``values_list``, and the heuristic filler + returns ``[]`` for any ``list``-origin annotation -- which is why this class + needs an explicit constructor at all. ``[]`` would also be the wrong thing + to satisfy it with: a ``VALUES`` clause with no rows is not an INSERT + source, and the zero-row spelling already has its own class, + ``DefaultValuesSource``, sitting next to this one. """ from rhosocial.activerecord.backend.expression.core import Literal from rhosocial.activerecord.backend.expression.statements.dml import ValuesSource @@ -245,11 +334,30 @@ def _values_source(dialect): return ValuesSource(dialect, [[Literal(dialect, 1)]]) -def _json_expr(dialect): - from rhosocial.activerecord.backend.expression.advanced_functions import JSONExpression - from rhosocial.activerecord.backend.expression.core import Literal +def _array_type(dialect): + """An array of integers -- one dimension. + + ``ArrayType`` requires an ``element_type`` and raises ``TypeError`` without + one ("an array with no element type is not a type"). Two things stop the + heuristic filler from reaching that requirement, and neither is fixable by + giving the parameter a default: + + * the filler skips defaulted parameters, so it calls ``ArrayType(dialect)`` + with nothing else; and + * ``dialect`` must come first and ``element_type`` must follow it, so a + *required* ``element_type`` after a defaulted ``dialect`` is not + expressible in Python at all. + + Making the parameter required therefore cannot help, and a default of + ``None`` is already what core declares. So the element type has to be + supplied here, as a concrete ``DataType``. + """ + from rhosocial.activerecord.backend.expression.types import ( + ArrayType, + IntegerType, + ) - return JSONExpression(dialect, Literal(dialect, '{"a": 1}'), path="$.a") + return ArrayType(dialect, element_type=IntegerType(dialect)) def _sequence(dialect): diff --git a/src/rhosocial/activerecord/testsuite/utils/parse_contract.py b/src/rhosocial/activerecord/testsuite/utils/parse_contract.py new file mode 100644 index 0000000..cba4619 --- /dev/null +++ b/src/rhosocial/activerecord/testsuite/utils/parse_contract.py @@ -0,0 +1,162 @@ +# src/rhosocial/activerecord/testsuite/utils/parse_contract.py +"""The shared parse-vs-format round-trip sweep for typed-column contracts. + +Every dialect's ``parse_type`` is the introspection inverse of its +``format_data_type_`` renderings. Two invariants make a dialect +coherent, and this module checks both over a backend's full type registry: + +**String stability** -- parsing a rendering and re-rendering the answer +must produce the identical string. Whatever ``parse`` answers, its answer +must name the same storage the server holds, or the schema differ would +report a change no one made. + +**Class honesty** -- the answer is either the declared instance itself +(``parsed == declared``, the round-trip-equal case) or the *documented* +answer class for that declaration: a backend that widens a declaration on +the way in (four integer widths stored as one integer type, a JSONB +concept stored as the JSON type) answers with the concept that was *not* +widened, and that answer is recorded here once per backend rather than +being re-derived by every test. + +What the sweep deliberately does **not** do: + +* It does not assert anything about a class the registry cannot construct + or cannot render -- those are the round-trip matrix's own lists + (``UNCONSTRUCTIBLE`` / pinned non-renders) and have their own tests. +* It does not guess what ``CustomType`` should parse to -- a + ``CustomType``'s raw is caller-supplied text that may happen to be a + word the dialect knows, so the class is skipped by default. +* It does not accept a ``CustomType`` answer for a rendered word. A + dialect that renders a word its own parse does not model has a gap on + one side or the other, and the sweep says so instead of blessing it. +""" + +from typing import Any, Dict, Optional + +from rhosocial.activerecord.backend.expression.types import CustomType, DataType + +from .expression import make_instance + + +def parse_roundtrip_failures( + dialect: Any, + registry: Dict[str, Any], + *, + widening: Optional[Dict[str, Any]] = None, + skip: Any = (), +) -> Any: + """Sweep *registry* and return the parse round-trip failures as text. + + Args: + dialect: the dialect whose ``parse_type`` is under test. + registry: a mapping of fully-qualified name to class, as the + backend's round-trip module keeps it (``REGISTERED`` or + ``ALL_CLASSES``). Only concrete ``DataType`` subclasses are + swept. + widening: the backend's documented answer table, + ``{declared class name: answer}``. An entry here asserts the + parse answers with exactly that class and that the answer + re-renders the identical string, **unless** the value is a + ``(class name, expected string)`` tuple: that form is for the + documented answers whose re-render deliberately *differs* -- + a backend that stores a word as its documented alias (Snowflake + writes TINYINT and stores INTEGER) or completes a bare word + with the server's default (its bare VARCHAR *is* + VARCHAR(16777216)). The tuple pins both the answer class and + the string it re-renders, so the alias is recorded as a + reviewable fact rather than leaking out as string instability. + The reason for each entry belongs in the backend's test file + next to the table, where a reader can check it. + skip: class names to leave out of the sweep entirely, in addition + to the built-in ``CustomType`` skip. Use it only for classes + that are not column types at all (a CAST target, say) and say + why in the test file. + + Returns: + A list of human-readable failures. An empty list is the contract + met; each element names the class, the rendering and the fault, + so a red test points straight at the offending entry instead of + leaving the reader to bisect. + """ + widening = dict(widening or {}) + skipped = set(skip) | {"CustomType"} + failures = [] + for fqn in sorted(registry): + cls = registry[fqn] + if not (isinstance(cls, type) and issubclass(cls, DataType)): + continue + if cls is DataType or cls is CustomType: + continue + if cls.__name__ in skipped: + continue + instance, source = make_instance(cls, dialect) + if instance is None: + continue # the matrix's UNCONSTRUCTIBLE list owns this class + try: + rendered, _params = instance.to_sql() + except Exception: + continue # the matrix's pinned non-renders own this class + try: + parsed = dialect.parse_type(rendered) + except Exception as exc: + failures.append( + "%s renders %r but parse_type raises %s: %s" + % (cls.__name__, rendered, type(exc).__name__, exc) + ) + continue + if isinstance(parsed, CustomType): + failures.append( + "%s renders %r but parse_type answers CustomType -- a " + "dialect must recognise its own renderings" + % (cls.__name__, rendered) + ) + continue + expected_entry = widening.get(cls.__name__) + if expected_entry is None: + if parsed != instance: + failures.append( + "%s renders %r but parse_type answers %r -- neither " + "the declaration nor a documented widening" + % (cls.__name__, rendered, parsed) + ) + continue + if isinstance(expected_entry, tuple): + expected, expected_restring = expected_entry + else: + expected, expected_restring = expected_entry, None + if type(parsed).__name__ != expected: + failures.append( + "%s renders %r; the documented answer is %s but " + "parse_type answers %s" + % (cls.__name__, rendered, expected, type(parsed).__name__) + ) + continue + try: + re_rendered, _ = parsed.to_sql() + except Exception as exc: + failures.append( + "%s renders %r; the documented answer %s re-renders with " + "%s: %s" + % (cls.__name__, rendered, expected, type(exc).__name__, exc) + ) + continue + if expected_restring is not None: + if re_rendered != expected_restring: + failures.append( + "%s renders %r; the documented answer %s re-renders " + "%r but the table says %r" + % (cls.__name__, rendered, expected, + re_rendered, expected_restring) + ) + elif re_rendered != rendered: + failures.append( + "%s renders %r but the documented answer %s re-renders " + "%r -- the strings must name the same storage, or the " + "table entry must record the documented re-render as a " + "(class, string) tuple" + % (cls.__name__, rendered, expected, re_rendered) + ) + return failures + + +__all__ = ["parse_roundtrip_failures"] diff --git a/tests/rhosocial/activerecord/testsuite/core/test_pool.py b/tests/rhosocial/activerecord/testsuite/core/test_pool.py new file mode 100644 index 0000000..6671865 --- /dev/null +++ b/tests/rhosocial/activerecord/testsuite/core/test_pool.py @@ -0,0 +1,58 @@ +# tests/rhosocial/activerecord/testsuite/core/test_pool.py +""" +Regression tests for the database pool's platform-specific slot locking. +""" +import sys + +import pytest + +from rhosocial.activerecord.testsuite.core import pool + + +@pytest.fixture +def no_fcntl(monkeypatch): + """Make ``import fcntl`` fail the way it does on Windows.""" + monkeypatch.setitem(sys.modules, "fcntl", None) + + +class TestSerialRunNeedsNoPosixLocking: + """A serial run never takes a slot lock, so it must not import ``fcntl``.""" + + def test_pooling_is_inactive_without_xdist_worker(self, monkeypatch): + """Without an xdist worker id there is nothing to pool.""" + monkeypatch.delenv("PYTEST_XDIST_WORKER", raising=False) + assert pool.pooling_active() is False + + def test_release_all_without_held_slot_does_not_import_fcntl(self, no_fcntl): + """The per-test teardown hook returns before touching ``fcntl``. + + The hook runs after *every* test, so importing the module + unconditionally failed each test on Windows even though no slot lock + had ever been taken. + """ + assert pool._HELD == {} + pool.release_all() + + +class TestSlotLocksRequirePosix: + """Holding a slot implies the locks were taken, which is POSIX-only. + + A slot is only ever acquired by :func:`pool._acquire_slot`, which itself + needs ``fcntl``; so the branch that unlocks is unreachable where ``fcntl`` + cannot be imported. The import stays inside that branch rather than + degrading to a silent no-op, because dropping the unlock would let two + workers believe they own the same database. + """ + + def test_held_slot_reaches_the_locking_module(self, no_fcntl, tmp_path): + """A held slot reaches ``fcntl`` -- and fails loudly without it.""" + lock_file = tmp_path / "slot.lock" + with open(lock_file, "a+") as handle: + pool._HELD["scenario"] = 0 + pool._OPEN_LOCKS[("scenario", 0)] = handle + try: + with pytest.raises(ImportError): + pool.release_all() + finally: + pool._HELD.pop("scenario", None) + pool._OPEN_LOCKS.pop(("scenario", 0), None) \ No newline at end of file diff --git a/tests/rhosocial/activerecord/testsuite/utils/test_expression_utils.py b/tests/rhosocial/activerecord/testsuite/utils/test_expression_utils.py new file mode 100644 index 0000000..60ced1e --- /dev/null +++ b/tests/rhosocial/activerecord/testsuite/utils/test_expression_utils.py @@ -0,0 +1,314 @@ +# tests/rhosocial/activerecord/testsuite/utils/test_expression_utils.py +""" +Guards on ``utils.expression``'s name-based constructor registry. + +``special_constructors()`` dispatches on ``full_name.endswith(key)``, which +makes a mis-keyed entry fail *silently*: the class falls through to +``_try_construct`` and is reported as unconstructible rather than as +mis-registered, and nothing raises. That is not hypothetical here — two of the +entries in that dict were dead when these tests were written. + +* ``"advanced_functions.JSONExpression"`` stopped matching the moment core + split that one class into ``JSONDocumentExpression`` and ``JSONTextExpression``. +* ``"query_parts.JoinExpression"`` named a class core renamed to + ``JoinClause``. + +Neither could raise an error, so neither did. Both entries have since been +retargeted at the live classes, so each of them now activates the constructor +it names. These tests exist to make the dispatch observable. + +Every assertion here goes *through* the dispatch. Nothing checks that a module +imports: the stale ``JSONExpression`` key imported perfectly well and matched +nothing at all, which is precisely why an import check would have been worth +nothing here. +""" + +import inspect + +import pytest + +from rhosocial.activerecord.backend.expression.advanced_functions import ( + JSONDocumentExpression, + JSONTextExpression, +) +from rhosocial.activerecord.backend.expression.core import Column +from rhosocial.activerecord.backend.expression.serialization import ExpressionRegistry +from rhosocial.activerecord.backend.expression.statements.dml import ValuesSource +from rhosocial.activerecord.backend.expression.types.array import ArrayType +from rhosocial.activerecord.backend.expression.types import IntegerType +from rhosocial.activerecord.backend.impl.dummy.dialect import DummyDialect + +from rhosocial.activerecord.testsuite.utils.expression import ( + _try_construct, + make_instance, + special_constructors, +) + + +@pytest.fixture(scope="module") +def dialect(): + return DummyDialect() + + +def _entry_for(cls): + """The factory the registry would dispatch this class to, or None.""" + fqn = f"{cls.__module__}.{cls.__name__}" + return next( + (fn for key, fn in special_constructors().items() if fqn.endswith(key)), + None, + ) + + +class TestTheSplitJSONClassesEachDispatchToTheirOwnConstructor: + """The two access operators are two classes, so they need two entries. + + One entry cannot cover both: ``endswith`` compares a suffix, and + ``"advanced_functions.JSONExpression"`` is not a suffix of either new name. + """ + + @pytest.mark.parametrize( + "cls, operation", + [(JSONDocumentExpression, "->"), (JSONTextExpression, "->>")], + ids=["document", "text"], + ) + def test_the_registry_entry_builds_that_class_asking_for_that_operator( + self, dialect, cls, operation + ): + fn = _entry_for(cls) + assert fn is not None, ( + f"{cls.__name__} has no entry in special_constructors(), so it " + f"falls through to _try_construct with no error raised. That " + f"silent fall-through is what this test exists for." + ) + + instance = fn(dialect) + assert type(instance) is cls, ( + f"the entry for {cls.__name__} returned {type(instance).__name__}" + ) + assert instance.operation == operation, ( + f"{cls.__name__} was built with operation={instance.operation!r}. " + f"The class exists to say which operator it is, and " + f"JSONTextExpression inherits operation='->' from its parent, so " + f"the factory has to state it." + ) + + @pytest.mark.parametrize( + "cls", [JSONDocumentExpression, JSONTextExpression], ids=["document", "text"] + ) + def test_make_instance_reports_special_rather_than_heuristic(self, dialect, cls): + """``source`` is the only observable difference between matched and not. + + A dead key does not raise; it just relabels the class as ``heuristic`` + (or ``failed``). So the label is itself the evidence the key is live. + """ + instance, source = make_instance(cls, dialect) + assert source == "special", ( + f"{cls.__name__}: make_instance reported source={source!r}, so no " + f"registry entry matched it. Renaming a class turns its entry into " + f"dead code and nothing raises." + ) + assert type(instance) is cls + + def test_each_class_matches_its_own_entry_and_neither_others(self): + """Two keys, not one — asserted so a future merge cannot pass quietly.""" + specials = special_constructors() + for cls in (JSONDocumentExpression, JSONTextExpression): + matched = [ + key for key in specials + if f"{cls.__module__}.{cls.__name__}".endswith(key) + ] + assert matched == [f"advanced_functions.{cls.__name__}"], ( + f"{cls.__name__} matched {matched}" + ) + + def test_rendering_does_not_depend_on_which_of_the_two_was_built(self, dialect): + """The split must not have moved a single character of SQL. + + Both formatters branch on ``expr.operation`` and dispatch on + ``format_method``, which the two classes share — so the class documents + the operator and the operation drives the rendering. If that stopped + being true, a caller who built "the wrong one" of the pair would get + different SQL for the same query, which is the confusion the split + exists to prevent. + """ + col = Column(dialect, "j", table="t") + + def render(cls, operation): + try: + return cls(dialect, col, "a", operation=operation).to_sql() + except Exception as exc: # noqa: BLE001 - recorded as the answer + return (f"<{type(exc).__name__}: {exc}>", ()) + + rendered = {} + for operation in ("->", "->>"): + as_document = render(JSONDocumentExpression, operation) + as_text = render(JSONTextExpression, operation) + assert as_document == as_text, ( + f"operation={operation!r} renders differently depending on the " + f"class: {as_document!r} vs {as_text!r}" + ) + sql, _params = as_document + assert not sql.startswith("<"), ( + f"operation={operation!r} refused to render at all, so the " + f"equality above would be comparing two identical refusals: {sql}" + ) + rendered[operation] = sql + + # And the operation still distinguishes the two renderings, so holding + # the class fixed did not collapse them into one another either. + assert rendered["->"] != rendered["->>"], rendered + print(f"\n rendered per operation: {rendered}") + + def test_the_two_classes_still_dispatch_to_the_same_formatter(self, dialect): + """``format_method`` is what the dialect dispatches on, and it is shared. + + Read off instances: it is a property, so the class object holds a + property rather than the string. + """ + col = Column(dialect, "j", table="t") + for operation in ("->", "->>"): + assert ( + JSONDocumentExpression(dialect, col, "a", operation=operation) + .format_method + == "format_json_expression" + ) + assert ( + JSONTextExpression(dialect, col, "a", operation=operation) + .format_method + == "format_json_expression" + ) + + +class TestTheTwoUnconstructibleClassesGetRealConstructors: + """``ValuesSource`` and ``ArrayType`` cannot be reached by the filler. + + Neither is exempt from anything. The contract test's guard simply never + applied to them, because they never got as far as being built at all. + Supplying a constructor is the opposite of an exemption: it turns the guard + on for two classes it was skipping. + """ + + @pytest.mark.parametrize("cls", [ValuesSource, ArrayType], ids=["ValuesSource", "ArrayType"]) + def test_the_heuristic_alone_cannot_build_it(self, dialect, cls): + """Establishes *why* the entry exists, so it cannot be deleted as redundant.""" + assert _try_construct(cls, dialect) is None, ( + f"{cls.__name__} is now reachable by the heuristic filler. If that is " + f"deliberate its explicit constructor is redundant, and this test is " + f"the thing that should be reconsidered." + ) + + @pytest.mark.parametrize("cls", [ValuesSource, ArrayType], ids=["ValuesSource", "ArrayType"]) + def test_the_registry_entry_builds_it_and_is_reached(self, dialect, cls): + fn = _entry_for(cls) + assert fn is not None, f"{cls.__name__} has no entry in special_constructors()" + assert type(fn(dialect)) is cls + + instance, source = make_instance(cls, dialect) + assert source == "special", f"{cls.__name__}: source={source!r}" + assert type(instance) is cls + + def test_the_values_source_is_a_real_insert_source(self, dialect): + """Non-empty, list rows, equal row lengths — the three things it checks. + + An empty ``values_list`` is the tempting filler value and the one the + constructor refuses. It is also the wrong thing to satisfy it with: a + ``VALUES`` clause with no rows is not an INSERT source, and the + zero-row spelling already has its own class, ``DefaultValuesSource``. + """ + source = _entry_for(ValuesSource)(dialect) + assert source.values_list + assert all(isinstance(row, list) for row in source.values_list) + assert len({len(row) for row in source.values_list}) == 1 + + with pytest.raises(ValueError): + ValuesSource(dialect, []) + + def test_the_array_has_a_concrete_element_type_and_cannot_do_without_one( + self, dialect + ): + """An array with no element type is not a type — core raises on it. + + ``element_type`` cannot simply be made required instead: ``dialect`` is + first and carries a default, and Python forbids a required parameter + after a defaulted one, so the signature core ships is the only + expressible one. The element type has to be supplied by the caller. + """ + array = _entry_for(ArrayType)(dialect) + assert isinstance(array.element_type, IntegerType) + assert array.dimensions >= 1 + + with pytest.raises(TypeError): + ArrayType(dialect) + + params = list(inspect.signature(ArrayType.__init__).parameters) + assert params[1] == "dialect", params + assert inspect.signature(ArrayType.__init__).parameters["element_type"].default \ + is None, "if element_type ever becomes required, this factory can pass it" + + def test_both_new_entries_report_the_parameters_get_params_will_be_asked_for( + self, dialect + ): + """The point of a constructor here: ``get_params()`` now has something to report. + + ``get_params()`` walks ``__init__`` and resolves every parameter to a + stored attribute. A class that cannot be built is a class whose + ``get_params()`` is never checked, which is the coverage these two + entries recover. + """ + for cls in (ValuesSource, ArrayType): + params = _entry_for(cls)(dialect).get_params() + assert params, cls.__name__ + assert "dialect" not in params, cls.__name__ + for pname in inspect.signature(cls.__init__).parameters: + if pname in ("self", "dialect"): + continue + assert pname in params, f"{cls.__name__}.get_params() has no {pname!r}" + + +class TestNoEntrySilentlyStopsMatching: + """An entry naming a class core still has must resolve to it. + + This is the general form of the silent failure, and it needs no exemption + list to state it: it asserts only about entries whose named class *exists*, + so an entry naming something core deleted is out of scope by construction + rather than by being tolerated. + """ + + def _core_registry(self): + ExpressionRegistry._auto_register_builtins() + return {f"{cls.__module__}.{cls.__name__}" for cls in ExpressionRegistry._registry.values()} + + def test_every_entry_naming_a_live_core_class_matches_it(self): + registry = self._core_registry() + # Which (module tail, class name) pairs core actually has. + live = { + (fqn.rpartition(".")[0].rpartition(".")[2], fqn.rpartition(".")[2]) + for fqn in registry + } + + mis_keyed = [] + inert = [] + for key in special_constructors(): + module_tail, _, class_name = key.rpartition(".") + if any(fqn.endswith(key) for fqn in registry): + continue + if (module_tail, class_name) in live: + mis_keyed.append(key) + else: + inert.append(key) + + print(f"\n entries naming a class core no longer has (inert): {inert}") + assert not mis_keyed, ( + "entries in special_constructors() that name a class core still " + "has but do not match it: " + + ", ".join(sorted(mis_keyed)) + + ". Renaming a class leaves its entry matching nothing, and the " + "class then falls back to _try_construct without any error." + ) + + +class TestMakeInstanceSourcesAreHonest: + def test_source_reflects_which_path_was_taken(self, dialect): + instance, source = make_instance(Column, dialect) + assert source in {"special", "heuristic"} + assert instance is not None