Code

Opened 4 years ago

Closed 11 months ago

Last modified 11 months ago

#14786 closed Bug (fixed)

get_db_prep_lookup call get_prep_value twice for each value if prepared == False

Reported by: homm Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: sprintdec2010 fields lookup
Cc: jonash Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by lrekucki)

In db.models.fields.Field get_db_prep_lookup() check if value is prepared and prepare it:

def get_db_prep_lookup(self, lookup_type, value, connection, prepared=False):
    "Returns field's value prepared for database lookup."
    if not prepared:
        value = self.get_prep_lookup(lookup_type, value)

get_prep_lookup() call get_prep_value() for every value.

But look next:

elif lookup_type in ('exact', 'gt', 'gte', 'lt', 'lte'):
    return [self.get_db_prep_value(value, connection=connection, prepared=prepared)]
elif lookup_type in ('range', 'in'):
    return [self.get_db_prep_value(v, connection=connection, prepared=prepared) for v in value]

Prepared flag not changed and get_db_prep_value() call get_prep_value() through get_db_prep_value() again!

I think get_db_prep_lookup() should call get_db_prep_value() always with prepared == True.

elif lookup_type in ('exact', 'gt', 'gte', 'lt', 'lte'):
    return [self.get_db_prep_value(value, connection=connection, prepared=True)]
elif lookup_type in ('range', 'in'):
    return [self.get_db_prep_value(v, connection=connection, prepared=True) for v in value]

This bug is still unnoticed because in standard ORM get_db_prep_lookup() always calls with prepared == True.

Attachments (2)

14786_fix_with_tests.diff (2.8 KB) - added by Aramgutang 4 years ago.
Fix with a regression test.
ticket14786.diff (2.4 KB) - added by lrekucki 2 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by jonash

  • Cc jonash added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Changed 4 years ago by Aramgutang

Fix with a regression test.

comment:2 Changed 4 years ago by Aramgutang

  • Has patch set
  • Keywords sprintdec2010 fields lookup added
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.2 to SVN

comment:3 Changed 3 years ago by anonymous

  • Severity set to Normal
  • Type set to Bug

comment:4 Changed 2 years ago by lrekucki

  • Description modified (diff)
  • Easy pickings unset
  • Patch needs improvement set
  • UI/UX unset

Patch doesn't apply cleanly, will try to rebase.

Changed 2 years ago by lrekucki

comment:5 Changed 2 years ago by lrekucki

  • Patch needs improvement unset

comment:6 Changed 11 months ago by Tim Graham <timograham@…>

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

In f19a3669b87641d7882491c3590d3c1c79756197:

Fixed #14786 -- Fixed get_db_prep_lookup calling get_prep_value twice if prepared is False.

Thanks homm for the report and Aramgutang and lrekucki for work on
the patch.

comment:7 Changed 11 months ago by Tim Graham <timograham@…>

In 10d15f79e5e2ca7b733e2bf1860e1778c3a712dc:

[1.6.x] Fixed #14786 -- Fixed get_db_prep_lookup calling get_prep_value twice if prepared is False.

Thanks homm for the report and Aramgutang and lrekucki for work on
the patch.

Backport of f19a3669b8 from master

comment:8 Changed 11 months ago by Simon Charette <charette.s@…>

In 36bbe3b7c52d459ed4862bbad1b44541a58cda00:

Altered test introduced in f19a3669b8 for the sake of readability. refs #14786

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.