Thanks for all the comments. I'll address the relevant bits below. > So, if the user explicitly asks for max_digits=3 in the output_field > argument, then we should enforce that. Or, alternatively we should > disallow setting max_digits for explicit output_field in expressions. >
I don't know how we could enforce model field arguments in the general case. For DecimalFields I removed the explicit call to format_number (from the converters) which now doesn't enforce max_digits/decimal_places, but there is no other code that checks model field constraints. I think the right choice is to disallow setting any constraints in the first place, but I don't know how we'd enforce that in the general case either. Is it possible with say a CharField to not have a max_length parameter > here, even though you can't make a concrete field without it? If not then > we have another issue. > The checks framework and validators enforce these kinds of constraints, but neither are triggered when constructing fields for use in output_field. However, the latest change that allows a DecimalField to be constructed without digits/decimals fails on sqlite, and any other backend that has a db_converter that does a format_value on the result before the expression has a chance to convert it. So it looks like we can't just ignore or disallow constraints without some more substantial changes due to backend converters. First, thanks for all your hard work in this area Josh. > Thanks! I think that if specified explicitly, any constraints should be honoured > and an exception raised if the computed aggregate value violates a > constraint. > This is similar to what Ansii thinks the behaviour should be. But we don't know if a user has provided the field, or if the field is being repurposed from the underlying field (for F() expressions), or if it was derived internally. Even if we did know, how would expressions know how to validate custom fields with custom constraints? I agree that it's odd that you can provide these validation parameters (to DecimalField and many other field types) when instantiating them for > output_field, and they won't be enforced, but I don't see a reasonable > way to address that in the general case. Exactly. db_column, db_index, blank .. none of these are applicable to expressions. How could we pick and choose which to enforce or ignore, especially when considering custom fields that we have no visibility of. I think your notes on the ticket are fairly accurate. Though a few things to note. We can't work with just a class, even if the get_internal_type was a classmethod. There are other methods that are called, such as get_db_converters. We could instantiate with no args, but I'm not sure that's a better API. So in a way, this is simply a bug in DecimalField (or perhaps in the way an > internal-type of DecimalField is special-cased in > ExpressionNode.convert_value), in that its "validation" constraints are > applied (and result in hard errors) in data conversion, where normally > field validation does not apply at all. > I think this is the best way to look at it, and I failed to communicate that position properly. As I mentioned above, I was under the wrong impression that DecimalField could be passed along without any arguments, but some backends (sqlite and oracle) do use them to convert database values to python values. This seems to be unique to DecimalFields though. If this limitation can be overcome, and documentation calls out that arguments passed to annotation fields are redundant, I think that this ticket will be satisfied. If users have an interest in applying constraints, then those constraints can be modelled as new expressions, enforced at the database. Something like Cast(expression, Decimal(5, 2)). That's really a new feature though, and should be raised as such if the need arises. Josh -- 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 [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0a94217a-584d-477f-8f4e-29220be6ea56%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
