[Django] #29222: Substr on NULL values returns incorrect results with pattern lookups.
Django
-------------------------------------+-------------------------------------
Reporter: felixxm | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords: Oracle
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
feb683c4c2c5ecfb61e4cb490c3e357450c0c0e8 revealed an unrelated issue on
Oracle. `SUBSTR(NULL, x, y)` returns `NULL` on Oracle which can be
concatenate with other strings, hence if we use it with pattern lookups
then all rows match an query, e.g.
- `__startswith=Substr(sth, x, y)` -> `LIKE SUBSTR(sth, x, y) || '%'` ->
`LIKE '%'`,
- `__endswith=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y)` ->
`LIKE '%'`,
- `__contains=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y) || '%'
` -> `LIKE '%%'`,
which is unexpected.
--
Ticket URL: <https://code.djangoproject.com/ticket/29222>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: felixxm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Oracle | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: felixxm (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: felixxm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> feb683c4c2c5ecfb61e4cb490c3e357450c0c0e8 revealed an unrelated issue on
> Oracle. `SUBSTR(NULL, x, y)` returns `NULL` on Oracle which can be
> concatenate with other strings, hence if we use it with pattern lookups
> then all rows match an query, e.g.
>
> - `__startswith=Substr(sth, x, y)` -> `LIKE SUBSTR(sth, x, y) || '%'` ->
> `LIKE '%'`,
> - `__endswith=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y)` ->
> `LIKE '%'`,
> - `__contains=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y) || '%'
> ` -> `LIKE '%%'`,
>
> which is unexpected.
New description:
feb683c4c2c5ecfb61e4cb490c3e357450c0c0e8 revealed an unrelated issue on
Oracle. `SUBSTR(NULL, x, y)` returns `NULL` on Oracle which can be
concatenate with other strings, hence if we use it with pattern lookups
then all rows match an query, e.g.
- `__startswith=Substr(sth, x, y)` -> `LIKE SUBSTR(sth, x, y) || '%'` ->
`LIKE '%'`,
- `__endswith=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y)` ->
`LIKE '%'`,
- `__contains=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y) || '%'
` -> `LIKE '%%'`,
which is unexpected.
''"Although Oracle treats zero-length character strings as nulls,
concatenating a zero-length character string with another operand always
results in the other operand, so null can result only from the
concatenation of two null strings. However, this may not continue to be
true in future versions of Oracle Database. To concatenate an expression
that might be null, use the NVL function to explicitly convert the
expression to a zero-length string."'' (see
[https://docs.oracle.com/database/121/SQLRF/operators003.htm#SQLRF51158
documentation]).
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: felixxm | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Oracle | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => felixxm
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Oracle | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: Mariusz Felisiak => (none)
* status: assigned => new
Old description:
> feb683c4c2c5ecfb61e4cb490c3e357450c0c0e8 revealed an unrelated issue on
> Oracle. `SUBSTR(NULL, x, y)` returns `NULL` on Oracle which can be
> concatenate with other strings, hence if we use it with pattern lookups
> then all rows match an query, e.g.
>
> - `__startswith=Substr(sth, x, y)` -> `LIKE SUBSTR(sth, x, y) || '%'` ->
> `LIKE '%'`,
> - `__endswith=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y)` ->
> `LIKE '%'`,
> - `__contains=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y) || '%'
> ` -> `LIKE '%%'`,
>
> which is unexpected.
>
> ''"Although Oracle treats zero-length character strings as nulls,
> concatenating a zero-length character string with another operand always
> results in the other operand, so null can result only from the
> concatenation of two null strings. However, this may not continue to be
> true in future versions of Oracle Database. To concatenate an expression
> that might be null, use the NVL function to explicitly convert the
> expression to a zero-length string."'' (see
> [https://docs.oracle.com/database/121/SQLRF/operators003.htm#SQLRF51158
> documentation]).
New description:
feb683c4c2c5ecfb61e4cb490c3e357450c0c0e8 revealed an unrelated issue on
Oracle. `SUBSTR(NULL, x, y)` returns `NULL` on Oracle which can be
concatenate with other strings, hence if we use it with pattern lookups
then all rows match an query, e.g.
- `__startswith=Substr(sth, x, y)` -> `LIKE SUBSTR(sth, x, y) || '%'` ->
`LIKE '%'`,
- `__endswith=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y)` ->
`LIKE '%'`,
- `__contains=Substr(sth, x, y)` -> `LIKE '%' || SUBSTR(sth, x, y) || '%'
` -> `LIKE '%%'`,
which is unexpected.
''"Although Oracle treats zero-length character strings as nulls,
concatenating a zero-length character string with another operand always
results in the other operand, so null can result only from the
concatenation of two null strings. However, this may not continue to be
true in future versions of Oracle Database. To concatenate an expression
that might be null, use the NVL function to explicitly convert the
expression to a zero-length string."'' (see
[https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf
/Concatenation-Operator.html#GUID-08C10738-706B-4290-B7CD-C279EBC90F7E]).
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:5>
Django
-------------------------------------+-------------------------------------
| Panditrao
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Oracle | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: (none) => Siddharth Panditrao
Hi! I'd like to work on this issue. This would be my first contribution to
Django, so please let me know if I'm on the right track.
'''What I've Found'''
When using Substr with pattern lookups like `__startswith` on Oracle, if
the Substr returns NULL, it ends up matching every row instead of none.
The problem is Oracle-specific: `NULL || '%'` becomes just `'%'` in Oracle
(universal match), but stays NULL in other databases (no match).
'''My Plan'''
I'm thinking of wrapping the expression with `NVL({}, CHR(0))` in the
Oracle backend's `_pattern_ops`. This would convert NULL to a null byte
character instead of letting it vanish during concatenation.
Something like:
{{{
Current: "startswith": "{} || '%%'"
Fixed: "startswith": "NVL({}, CHR(0)) || '%%'"
}}}
'''Testing'''
I've put together a test case on a branch that reproduces the bug - it
passes on Oracle (showing the bug) and fails on other databases
(confirming it's Oracle-only).
Does this approach make sense? Happy to submit a PR if this looks good,
just want to make sure I'm heading in the right direction before diving
deeper.
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:6>
Django
-------------------------------------+-------------------------------------
| Panditrao
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: Oracle | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I left a note to this effect [https://forum.djangoproject.com/t/fix-
oracle-bug-where-substr-null-with-pattern-lookups-matches-all-rows-
instead-of-none/43757 on the forum] but I'm afraid there isn't much we can
do here given `__startswith` and related lookups
[https://dryorm.xterm.info/ticket-29222 have historically accepted an
empty strings as their right-side-hand to match all rows].
Because `''` and `NULL` are the same thing on Oracle we have to choose; we
can't return all rows in the first case and no rows in the second like on
all other backends.
Given these lookups are meant to treat with text fields left-hand-sides in
which
[https://docs.djangoproject.com/en/6.0/ref/models/fields/#django.db.models.Field.null
we document that storing `NULL` is an anti-pattern]
> Avoid using `null` on string-based fields such as `CharField` and
`TextField`. The Django convention is to use an empty string, not `NULL`,
as the “no data” state for string-based fields.
and things have been working this way on Oracle since their inception I'd
be more inclined to wont-fix this ticket and accept it as a unfortunate
side effect of `interprets_empty_strings_as_nulls`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:7>
Django
-------------------------------------+-------------------------------------
| Panditrao
(models, ORM) |
| Unreviewed
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: => wontfix
* stage: Accepted => Unreviewed
* status: assigned => closed
Comment:
Replying to [comment:7 Simon Charette]:
oracle-bug-where-substr-null-with-pattern-lookups-matches-all-rows-
instead-of-none/43757 on the forum] but I'm afraid there isn't much we can
do here given `__startswith` and related lookups
[https://dryorm.xterm.info/ticket-29222 have historically accepted an
empty strings as their right-side-hand to match all rows].
>
> Because `''` and `NULL` are the same thing on Oracle we have to choose;
we can't return all rows in the first case and no rows in the second like
on all other backends.
>
> Given these lookups are meant to treat with text fields left-hand-sides
in which
[https://docs.djangoproject.com/en/6.0/ref/models/fields/#django.db.models.Field.null
we document that storing `NULL` is an anti-pattern]
>
> > Avoid using `null` on string-based fields such as `CharField` and
`TextField`. The Django convention is to use an empty string, not `NULL`,
as the “no data” state for string-based fields.
>
> and things have been working this way on Oracle since their inception
I'd be more inclined to wont-fix this ticket and accept it as a
things for an arguably worst default.
Yes, exactly, that's why I didn't fix this for so long.
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:8>
Django
-------------------------------------+-------------------------------------
| Panditrao
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: Oracle | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Replying to [comment:8 Mariusz Felisiak]:
> > I left a note to this effect [https://forum.djangoproject.com/t/fix-
oracle-bug-where-substr-null-with-pattern-lookups-matches-all-rows-
instead-of-none/43757 on the forum] but I'm afraid there isn't much we can
do here given `__startswith` and related lookups
[https://dryorm.xterm.info/ticket-29222 have historically accepted an
empty strings as their right-side-hand to match all rows].
> >
> > Because `''` and `NULL` are the same thing on Oracle we have to
choose; we can't return all rows in the first case and no rows in the
second like on all other backends.
> >
> > Given these lookups are meant to treat with text fields left-hand-
sides in which
[https://docs.djangoproject.com/en/6.0/ref/models/fields/#django.db.models.Field.null
we document that storing `NULL` is an anti-pattern]
> >
> > > Avoid using `null` on string-based fields such as `CharField` and
`TextField`. The Django convention is to use an empty string, not `NULL`,
as the “no data” state for string-based fields.
> >
> > and things have been working this way on Oracle since their inception
I'd be more inclined to wont-fix this ticket and accept it as a
unfortunate side effect of `interprets_empty_strings_as_nulls` than change
things for an arguably worst default.
>
> Yes, exactly, that's why I didn't fix this for so long.
the existing empty-string behavior on Oracle. I agree this should be
marked as wontfix given the trade-offs involved. Should I update the
ticket status myself, or will a maintainer handle that? And would it be
helpful if I added a test or docs clarifying this Oracle-specific
behavior?
This was my first attempt at contributing to Django, so I appreciate you
taking the time to walk through the reasoning.
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:9>
Django
-------------------------------------+-------------------------------------
| Panditrao
Type: Bug | Status: closed
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: Oracle | Triage Stage:
| Unreviewed
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Hello Siddhahrt,
> Should I update the ticket status myself, or will a maintainer handle
that?
set.
> And would it be helpful if I added a test or docs clarifying this
Oracle-specific behavior?
[https://docs.djangoproject.com/en/6.0/ref/databases/#null-and-empty-
strings this documentation]. The fact it states
> When fetching from the database, it is assumed that a `NULL` value in
one of these fields **really means the empty string**, and the data is
silently converted to reflect this assumption.
strengthen my believe we're making the right call here as we consider that
`NULL` means empty string by continuing to match all rows.
> This was my first attempt at contributing to Django, so I appreciate you
taking the time to walk through the reasoning.
Triaging issues and spending time understanding them is often more
valuable than landing code itself as it strengthen our common
understanding of the problem and allow us to
[https://github.com/django/django/pull/19793#discussion_r2637685073 build
a consistent experience] accross the diverse interfaces the ORM provides.
--
Ticket URL: <https://code.djangoproject.com/ticket/29222#comment:10>