Opened 16 hours ago
Last modified 4 hours ago
#36221 new New feature
Greatest() argument count guards do not align with certain database backends
Reported by: | Martijn Verkleij | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I ran into two issues while I tried to use this comparison function django.db.models.function.comparison.Greatest
:
- It forces you to use two or more arguments, while the Postgres database backend, for example, allows for one argument.
- If the argument supplied is a generator expression, it is not unpacked and the above check triggers while more arguments are supplied.
I found these while using Django 4.2, but I have checked the current source code and it is unchanged compared to that version.
For the first one, a possible solution might be to move the check to the different database backends, or be generalized somehow to allow certain backends to have different numbers of arguments. One could even consider writing custom SQL generators for lists of length one (returning the value bare), and an exception for passing the empty list.
The second one appears to be a Python language limitation for which it is debatable whether to bother implementing anything to fix it. Converting the expression to a tuple and unpacking it solves the problem.
Change History (3)
comment:1 by , 15 hours ago
Type: | Bug → New feature |
---|
comment:2 by , 14 hours ago
Hello,
Thanks for the quick response!
I think you are fair in your assessment of the problem. Calling this function with a variable, non-explicit amount of arguments without the user of the function programming defensively can only be recipe for disaster anyway. Whether it is worth doing this for the few backends that support a single argument for this function is doubtful with that in mind.
Anyway, if a generalization like you suggest would be implemented, it could maybe work for LEAST and COALESCE too as they similarly only require one argument.
I'm to assume you meant that Greatest(expr in expr_generator) was not automatically picked up as Greatest(*(expr in expr_generator))? The signature of expressions, Greatest in this case, are not defined as (Iterable[Expression]) but *Expression so you must ensure to call them in this fashion. I don't see it as a limitation of the Python language but simply as an improper function call.
That was indeed what I meant. I was already afraid this was a case of "user error". I did not realise I could have seen this coming through the method signature, totally forgetting that Iterable[] exists and that this is how packing/unpacking works. Thanks, I learned something new today!
comment:3 by , 4 hours ago
I agree that if we decide this change to be worth it we should likely use the same approach for Least
and Coalesce
but I remain unconvinced that it's worth adding as it's pretty simple not to use these functions when dealing with a single expression.
Maybe you could try testing the water on the forum to see if other members of the community ran into a similar edge case with singleton expression wrapping? That's the usual process for requesting features anyway.
Thank you for your report but I don't this that these should be considered as bugs.
The expressions provided by Django's core ought to be good compromise between all backends it supports and these objects are backend agnostic until they are actually compiled for execution. Given it's easy enough to write a simple function of the form
I'm not entirely convinced adjusting
Greatest
to do the same but at resolving time is worth doing even if it's relatively trivial to adjustGreatest.resolve_expression
to accomplish the same (resolve to the first provided expression if there's a single one which is backend agnostic). I would qualify this as a new feature though.django/db/models/functions/comparison.py
2:two expressions")I'm to assume you meant that
Greatest(expr in expr_generator)
was not automatically picked up asGreatest(*(expr in expr_generator))
? The signature of expressions,Greatest
in this case, are not defined as(Iterable[Expression])
but*Expression
so you must ensure to call them in this fashion. I don't see it as a limitation of the Python language but simply as an improper function call.