Code

Opened 14 months ago

Closed 13 months ago

Last modified 13 months ago

#20462 closed Bug (fixed)

Regex lookups should not fail on null/None and non-string values on SQLite

Reported by: noirbizarre Owned by: amclark7
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: sqlite, regex, lookup
Cc: timograham@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The user function provided to do the regex lookup is buggy for two cases:

  • raise an exception when the value is None (null in databse)
  • raise an exception when the value is not a string

I provided a pull request fixing these issues: https://github.com/django/django/pull/1043

Attachments (0)

Change History (12)

comment:1 Changed 14 months ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Ensuring that all databases behave the same way whenever this is possible is a good idea.

comment:2 Changed 13 months ago by Kamu

  • Triage Stage changed from Accepted to Ready for checkin

comment:3 Changed 13 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 64041f0e6e7a773574d7cde978d16305200004ea:

Fixed #20462 - Fixed sqlite regex lookups for null values and non-string fields.

comment:4 Changed 13 months ago by Tim Graham <timograham@…>

In 92c49d6f01700961698eb3d79fd393d673cee219:

Revert "Fixed #20462 - Fixed sqlite regex lookups for null values and non-string fields."

This reverts commit 64041f0e6e7a773574d7cde978d16305200004ea.

lookup.tests.LookupTests.test_regex_non_string fails under Postgres.
We should also try to rewrite the test using an existing model.

comment:5 Changed 13 months ago by timo

  • Cc timograham@… added
  • Patch needs improvement set
  • Resolution fixed deleted
  • Status changed from closed to new
  • Triage Stage changed from Ready for checkin to Accepted

comment:6 Changed 13 months ago by amclark7

  • Owner changed from nobody to amclark7
  • Status changed from new to assigned

comment:7 Changed 13 months ago by amclark7

  • Patch needs improvement unset

Added a pull request for fixing this issue: https://github.com/django/django/pull/1306

This is only a minor change compared to the original patch provided by noirbizarre.

comment:8 Changed 13 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 273dc550a4eccdad958541f456332312b3369504:

Fixed #20462 -- null/non-string regex lookups are now consistent

Thanks to noirbizarre for the report and initial patch.

comment:9 Changed 13 months ago by timo

  • Resolution fixed deleted
  • Status changed from closed to new

As pointed out in IRC by loic84, the str() cast added in django/db/backends/sqlite3/base.py should probably be something like force_text to properly handle unicode.

comment:11 Changed 13 months ago by Loic Bistuer <loic.bistuer@…>

  • Resolution set to fixed
  • Status changed from new to closed

In a9ea7d8c708c8265cccc17f23c62b2268d9e94f8:

Fixed #20462 - Replaced the str() cast introduced in 273dc55 by force_text()

comment:12 Changed 13 months ago by Tim Graham <timograham@…>

In b6a87f5c93efa5192433be1e45fc4e79d54efdc7:

Merge pull request #1308 from loic/ticket20462

Fixed #20462 - Replaced the str() cast introduced in 273dc55 by force_text()

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.