Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Toan Vuong):

 > Coalesce() on Oracle ​explicitly checks the output_field when it's
 compiled (for Cast.default), on other database the output_field is not
 used when Coalesce() is compiled so it doesn't crash. The crash on Oracle
 is a side-effect of the current implementation, IMO, there is no need to
 change it.

 Thanks, this is what I was missing. In our local copy of Django I
 essentially added a try/catch on that `as_oracle` function which worked,
 but I didn't realize that was only called in some cases but not others.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018abd7471e0-e7c23bf6-8dd9-4422-bb01-8b744e1ef691-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"dcd3a0316bed8605c0464793cd7ca73027eaa780" dcd3a03]:
 {{{
 #!CommitTicketReference repository=""
 revision="dcd3a0316bed8605c0464793cd7ca73027eaa780"
 [5.0.x] Fixed #34858 -- Corrected resolving output_field for
 PositiveIntegerField.

 Regression in 40b8a6174f001a310aa33f7880db0efeeb04d4c4.

 Backport of 4de31ec680df062e5964b630f1b881ead5004e15 from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018abbefeb80-8789864c-8914-4729-924c-852c92917ad1-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"4de31ec680df062e5964b630f1b881ead5004e15" 4de31ec]:
 {{{
 #!CommitTicketReference repository=""
 revision="4de31ec680df062e5964b630f1b881ead5004e15"
 Fixed #34858 -- Corrected resolving output_field for PositiveIntegerField.

 Regression in 40b8a6174f001a310aa33f7880db0efeeb04d4c4.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018abbef78fd-1b0716f9-e2dd-462a-a4b5-2d9770a00e6f-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * stage:  Accepted => Ready for checkin


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018abbc5280a-326735c9-f766-4955-baa0-8a531e7bc678-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak):

 Replying to [comment:6 Toan Vuong]:
 > Still wondering about whether we need to do something about `Case`
 between Postgres vs. Oracle, because the behavior still seems inconsistent
 there.

 `Coalesce()` on Oracle
 
[https://github.com/django/django/blob/d7972436639bacada0f94d3b9818446343af59ad/django/db/models/functions/comparison.py#L93
 explicitly checks] the `output_field` when it's compiled (for
 `Cast.default)`, on other database the `output_field` is not used when
 `Coalesce()` is compiled so it doesn't crash. The crash on Oracle is a
 side-effect of the current implementation, IMO, there is no need to change
 it.

 > Edit: Also not sure what the process is to open PRs for other
 branches(?). Seems like I should also merge this to main and stable/5.0.x?

 Mergers will backport if needed.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018abbbe9d9e-bc640fcf-b004-42be-ae0e-58b91263f219-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Toan Vuong):

 * has_patch:  0 => 1


Comment:

 Opened a PR:
 https://github.com/django/django/pull/17296

 Still wondering about whether we need to do something about `Case` between
 Postgres vs. Oracle, because the behavior still seems inconsistent there.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018aba08fc7a-beb94317-923e-4211-b472-915785dcf901-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  Toan
 |  Vuong
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Toan Vuong):

 * owner:  nobody => Toan Vuong
 * status:  new => assigned


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab96f42b3-3faa318f-eecd-4e08-ac26-4279e2b0b665-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved.

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Toan Vuong):

 Thanks for the reply! I can work on a PR for the `PositiveIntegerField`
 change following the patch you wrote.

 However, regarding your statement:

 > Coalesce() on Oracle checks the output_field of arguments and crashes

 This seems incorrect, because using `Coalesce` alone would crash on both
 Postgres and Oracle. The weird thing is if I have a `Case` on top of the
 "bad" `Coalesce`, then only Oracle crashes. The Postgres queryset executes
 successfully.  So it seems like `Case` behaves differently between the
 two, and probably incorrectly swallows the error thrown by `Coalesce` in
 the Postgres scenario which also seems like a bug?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab96c77ff-03ddef60-3d9d-4931-8c11-7d89ac153259-00%40eu-central-1.amazonses.com.

Re: [Django] #34858: Output field for combined PositiveIntegerField is not properly resolved. (was: Bug in output field handling)

[Django]

#34858: Output field for combined PositiveIntegerField is not properly resolved.
-+-
 Reporter:  Toan Vuong   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  4.2
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * cc: Simon Charette, Luke Plant (added)
 * type:  Uncategorized => Bug
 * component:  Uncategorized => Database layer (models, ORM)
 * stage:  Unreviewed => Accepted


Comment:

 Thanks for the ticket. This is not strictly related with Oracle, the main
 issue is that `output_field` is not properly resolved for a combination of
 `PositiveIntegerField`. `Coalesce()` on Oracle checks the `output_field`
 of arguments and crashes. We should be able to fix this but prioritize
 `PositiveIntegerField`, e.g.
 {{{#!diff
 diff --git a/django/db/models/expressions.py
 b/django/db/models/expressions.py
 index 4ea179ecde..54edaa611c 100644
 --- a/django/db/models/expressions.py
 +++ b/django/db/models/expressions.py
 @@ -512,6 +512,23 @@ class Expression(BaseExpression, Combinable):

  _connector_combinations = [
  # Numeric operations - operands of same type.
 +{
 +# PositiveIntegerField should take precedence over IntegerField.
 +connector: [
 +(
 +fields.PositiveIntegerField,
 +fields.PositiveIntegerField,
 +fields.PositiveIntegerField,
 +),
 +]
 +for connector in (
 +Combinable.ADD,
 +Combinable.MUL,
 +Combinable.DIV,
 +Combinable.MOD,
 +Combinable.POW,
 +)
 +},
  {
  connector: [
  (fields.IntegerField, fields.IntegerField,
 fields.IntegerField),

 }}}

 Would you like to prepare a patch (via GitHub PR)? a regression test is
 required.


 Regression in 40b8a6174f001a310aa33f7880db0efeeb04d4c4 (#33397).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab6b65569-3145cab2-282e-4055-84db-03cbc9dce181-00%40eu-central-1.amazonses.com.