lycantropos / hypothesis_sqlalchemy

hypothesis strategies for generating SQLAlchemy objects
MIT License
28 stars 8 forks source link

Variant support #30

Open jzelinskie opened 3 years ago

jzelinskie commented 3 years ago

First off, thanks for the incredibly useful library. It's exactly what I needed, except for one pitfall: Variants.

I have a Column with a Variant, which I've been trying to manually register:

# Provide a proxy type that will allow us to use Integer for sqlite and BigInteger for other DB
# engines to avoid sqlite not allowing autoincrement BigIntegers.
AutoBigIntType = BigInteger().with_variant(sqlite.INTEGER(), "sqlite")

...

@from_type.register(AutoBigIntType)
def autobiginttype_values_factory(type_: AutoBigIntType) -> Strategy[int]:
    return strategies.integers(min_value=0)

>>> E   TypeError: Invalid annotation for 'type_'. Variant() is not a class.

This is somewhat related to #21 because supporting UUIDs is sometimes solved with Variants. However, I think there are various use cases for Variants outside of the example I provided and Postgres UUIDs, so it makes sense to come up with some kind of cohesive strategy for the general usage.

I have no idea if it's correct, but my naive intuition is that when you discover a Variant, you could resolve to MyTable.columns["variant_column_name"].type.impl.

lycantropos commented 3 years ago

Thanks for the issue.

We are using functools.singledispatch, so there is no way to pass non-type objects to from_type.register.

But it looks like we can add a special case for Variant class like

from typing import Any

from hypothesis_sqlalchemy.hints import Strategy
from sqlalchemy.types import Variant
...
@from_type.register(Variant)
def variant_values_factory(type_: Variant) -> Strategy[Any]:
    return from_type(type_.impl)

Can you please test it locally whether this is what you need? I'm not familiar with Variants myself and it looks like this should be added as a modification to hypothesis_sqlalchemy.columnar.values.factory function https://github.com/lycantropos/hypothesis_sqlalchemy/blob/6c68a065802d5efdfa3d805d9e4a79199ab46ea3/hypothesis_sqlalchemy/columnar/values.py#L31-L32 something like: if column type is variant, then try to get dialect from column.bind, if there is none -- use from_type(column.type.impl), if has -- from_type(column.type.mapping.get(dialect_name, column.type.impl)) or something like that.

jzelinskie commented 3 years ago

Your code snippet works totally fine in my unit tests.

However, the engine isn't in sqlalchemy during my unit tests so I cannot confirm whether the conditional around column.bind is correct.