Alright so here's for a few hints about I believe things should be done.
First things first Lookup must be made a subclass of Expression which is
being worked on[0].
Ideally Q would also be made a subclass of Expression but that's likely a
big can of worms so I'd focus on implementing it for Q only at first.
Now for the compiler part. Things are bit tricky here as these expressions
are not going to be bound to a model/table and most of the sql.Query and
resolve_expression machinery revolves around the availability of a
Query.model: models.Model property. I think there's two solutions here:
1. Adapt sql.Query so it can be *unbounded* meaning that it's .model
property type would change from models.Model to Optional[models.Model].
2. Follow the sql.RawQuery route and define a new sql.UnboundQuery class
that *looks* like a Query but doesn't allow any form of column references
or JOINs or filters (WHERE).
In both cases the Query like object should prevent any form of column
references and JOINs with a comprehensible error messages (e.g. in resolve_ref
and setup_join if go with 1.). I have a feeling 2. is easier to implement
but 1. seems like it could be a much more rewarding experience for you and
the community as you'll have to special case a couple of long lived
assumptions in django.db.models.sql.
Depending on whether you choose the 1. or 2. you'll have to implement a way
for database backends to specify how to *wrap* the provided expression in a
SELECT statement. Most databases won't require any special casing but I
know that Oracle will require the addition of a trailing "DUAL" clause
(SELECT ... FROM DUAL)[1] and possibly some special casing of expressions
such as exists but there's already pre-existing logic for that[2]. If you
go with 1. this can be done by returning a backend specific string in
SQLCompiler.get_from_clause when self.query.alias_map is empty[3].
In the end the call stack should be (assuming 1. is followed):
Q.check(self, using):
query = Query()
query.add_annotations(self, '_check')
query.set_values('_check')
compiler = query.get_compiler(using=db)
result = compiler.execute_sql(SINGLE)
return bool(result[0])
I hope this clears things up a bit!
Cheers,
Simon
[0] https://github.com/django/django/pull/14494
[1] https://docs.oracle.com/cd/B19306_01/server.102/b14200/queries009.htm
[2]
https://github.com/django/django/blob/ed3af3ff4b3cfb72de598f1b39a1028ba3826efb/django/db/models/expressions.py#L382-L389
[3]
https://github.com/django/django/blob/ed3af3ff4b3cfb72de598f1b39a1028ba3826efb/django/db/models/sql/compiler.py#L797-L810
Le mercredi 2 juin 2021 à 11:36:02 UTC-4, gaga...@gmail.com a écrit :
> Thanks for the thorough answer. I also realize now that it worked in my
> app only because of another side effect when my instance was saved..
>
> I started to take a look at the ORM part where the check method should be
> implemented as I'm not used to it. Shouldn't .check() be implemented on Q
> and not on Expression? Or are you including Lookup / Q in it?
>
> Then I'd guess it's just a matter of calling as_sql() from each part and
> assemble them. Everythings we need seems to be done in Query and we can't
> use it as it has to be linked to a model, so we would have to redo it?
> Although as_sql needs a compiler which itself needs a query. I admit I'm a
> bit lost in all those classes, everything seems to be too much linked to
> the models to do something without one.
>
> If you have any more hints as to where I should look, thanks again.
> Le mercredi 2 juin 2021 à 00:33:12 UTC+2, charettes a écrit :
>
>> Hello there,
>>
>> Partial unique constraints are currently not supported during validation
>> for reasons described in this ticket[0].
>>
>> For example (inspired by this Github comment[1]), if you define the
>> following model
>>
>> class Article(models.Model):
>> slug = models.CharField(max_length=100)
>> deleted_at = models.DateTimeField(null=True)
>>
>> class Meta:
>> constraints = [
>> UniqueConstraint('slug', condition=Q(deleted_at=None),
>> name='unique_slug'),
>> ]
>>
>> Then validate_unique must perform the following query to determine if the
>> constraint is violated
>>
>> SELECT NOT (%(deleted_at)s IS NULL) OR NOT EXISTS(SELECT 1 FROM article
>> WHERE NOT id = %(id)s AND slug = %(slug)s AND deleted_at IS NULL)
>>
>> In other words, the validation of a partial unique constraint must check
>> that either of these conditions are true
>> 1. The provided instance doesn't match the condition
>> 2. There's no existing rows matching the unique constraint (excluding the
>> current instance if it already exists)
>>
>> This is not something Django supports right now.
>>
>> In order to add proper support for this feature I believe (personal
>> opinion here feedback is welcome) we should follow these steps:
>>
>> 1. Add support for Expression.check(using: str) -> bool that would
>> translate IsNull(deleted_at, True).check('alias') into a backend
>> compatible 'SELECT %(deleted_at)s IS NULL' query and return whether or not
>> it passed. That would also allow the constructions of forms like
>>
>> (~Q(IsNull(deleted_at, True)) |
>> ~Exists(Article.objects.exclude(pk=pk).filter(slug=slug,
>> deleted_at=None)).check(using)
>>
>> 2. Add support for Constraint.validate(instance, excluded_fields) as
>> described in [0] that would build on top of Expression.check to implement
>> proper UniqueConstraint, CheckConstraint, and ExclusionConstraint
>> validation and allow for third-party app (e.g. django-rest-framework which
>> doesn't use model level validation[2]) to take advantage of this feature.
>> For example the unique_for_(date|month|year) feature of Date(Time)?Field
>> could be deprecated in favour of Constraint subclasses that implement
>> as_sql to enforce SQL level constraint if available by the current backend
>> and implement .validate to replace the special case logic we have currently
>> in place for these options[3].
>>
>> I hope this clarify the current situation.
>>
>> Cheers,
>> Simon
>>
>> [0] https://code.djangoproject.com/ticket/30581#comment:7
>> [1] https://github.com/django/django/pull/10796#discussion_r244216763
>> [2] https://github.com/encode/django-rest-framework/issues/7173
>> [3]
>> https://github.com/django/django/blob/e703b152c6148ddda1b072a4353e9a41dca87f90/django/db/models/base.py#L1062-L1084
>>
>> Le mardi 1 juin 2021 à 11:18:23 UTC-4, gaga...@gmail.com a écrit :
>>
>>> Hi,
>>>
>>> I changed several models from fields using `unique=True` to using
>>> `UniqueConstraint` with a condition in the Meta.
>>>
>>> As a side-effect, the uniqueness are no longer validated during cleaning
>>> of a Form and an integrity error is raised. This is because partial unique
>>> indexes are excluded :
>>>
>>> https://github.com/django/django/blob/e703b152c6148ddda1b072a4353e9a41dca87f90/django/db/models/options.py#L865-L874
>>>
>>> It seems that `total_unique_constraints` is also used to check for
>>> fields that should be unique (related fields and USERNAME_FIELD
>>> specifically).
>>>
>>> I tried modifying `total_unique_constraints` and the only tests which
>>> failed were related to the above concern and
>>> `test_total_ordering_optimization_meta_constraints` which also uses `
>>> total_unique_constraints`. My application works fine and the validation
>>> error are correctly raised in my forms.
>>>
>>> The current behaviour of `Model.validate_unique` is also not the one I
>>> expected as my conditional `UniqueConstraint` were not used (which caused
>>> the integrity error).
>>>
>>> Am I missing something? Or should we use all constraints (including
>>> partial) in `Model.validate_unique`?
>>>
>>> If this is indeed what should be done, adding an `all_unique_constraints`
>>> next to `total_unique_constraints` and using it in `Model.validate_unique`
>>> instead of `total_unique_constraints` would do the trick. I don't mind
>>> opening a ticket and doing the PR if needed.
>>>
>>> Thanks.
>>>
>>
--
You received this message because you are subscribed to the Google Groups
"Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/1c2e3872-d93f-43b1-aeff-5027d535c499n%40googlegroups.com.
