Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#27834 closed New feature (fixed)

Add the STRPOS database function

Reported by: Baptiste Mispelon Owned by: Brad Melin
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: expressions
Cc: josh.smeaton@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django currently ships with a dozen or so database functions [1].

I recently had the need for what postgres calls STRPOS(string, substring) to find the position of a string inside a substring and was surprised to find it was not included in the list of builtins.

I'm not sure if there's an official criteria for deciding which functions get implemented in core, but I did some quick research and found that this STRPOS() functionality at least seems to be present in all 4 officially supported databases:

  • Postgres: STRPOS(string, substring) [2]
  • Sqlite: INSTR(string, substring) [3]
  • MySQL: LOCATE(substring, string) [4]
  • Oracle: INSTR(string, substring) [5]

The names and order of arguments are somewhat inconsistent but the behavior and return value of the function seems well-defined: it returns a positive integer corresponding to the (1-indexed) position of the substring inside the string; if the substring is not found, 0 is returned.

I'm marking this as "easy pickings" because other than the inconsistent naming and argument order, the actual implementation of the Func shouldn't be too complex.

[1] https://docs.djangoproject.com/en/dev/ref/models/database-functions/
[2] https://www.postgresql.org/docs/current/static/functions-string.html
[3] https://www.sqlite.org/lang_corefunc.html#instr
[4] https://dev.mysql.com/doc/refman/5.7/en/string-functions.html#function_locate
[5] https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions068.htm

Change History (15)

comment:1 by David Adler, 8 years ago

Triage Stage: UnreviewedAccepted

This seems valid to me maybe it could be called something like Indexstr to be in keeping with 'asdf'.index('s')?

comment:2 by Brad Melin, 8 years ago

Owner: changed from nobody to Brad Melin
Status: newassigned

comment:3 by Josh Smeaton, 8 years ago

Cc: josh.smeaton@… added
Keywords: expressions added
Version: 1.10master

I'm not sure if there's an official criteria for deciding which functions get implemented in core

Nope, there's no official criteria. As you noted there's support in all 4 official backends, and it's a commonly used function, which makes this a good candidate for inclusion. For anyone reading this in the future, I'm wary about including obscure functions that don't enjoy support for all (or most) backends, and would probably like to see a library spring up to support those use cases.

Thanks for doing the investigation Baptiste.

comment:4 by vikram, 8 years ago

Hello,

I have some experience using django for developing web apps/rest api's. I want to get to know about the internal of django, how the thing works.
I had read the contributing docs and completed with setup of django code.
Can you guide me how can I kickstart implementing Add the STRPOS database function feature

Thanks

comment:5 by Tim Graham, 8 years ago

Brad has already assigned the ticket three days ago. If there's no activity or update from after after some weeks it's fine to reassign, otherwise it's not polite to reassign the ticket so quickly. In choosing a ticket, try to find something that you some idea of how to begin.

comment:6 by Josh Smeaton, 8 years ago

Vikram, as Tim already mentioned this particular ticket has been picked up by someone else. That doesn't mean you can't hunt around for another database function to implement that meets the criteria of "widely used" and "works on mysql, postgres, oracle, and sqlite". Be sure to open a ticket if you find such a one.

Can you guide me how can I kickstart implementing X Database Function

First you should check out the docs for this existing functions: https://docs.djangoproject.com/en/1.10/ref/models/database-functions/
Then you should read up on the Expressions API: https://docs.djangoproject.com/en/1.10/ref/models/expressions/

You can have a look at how some of the current functions are implemented here: https://github.com/django/django/blob/1f7ca858664491589ba400419a491dd0a9af5dff/django/db/models/functions/base.py

Read the various parts of the expressions API as you're looking into how the other functions are implemented. Generally get familiar with the following files:

django/db/models/functions/*
django/db/models/expressions.py

They're all reasonably self contained, and don't venture too far into the rest of the ORM.

Once you've got some code to make a function work, then you'd write some tests in https://github.com/django/django/blob/1f7ca858664491589ba400419a491dd0a9af5dff/tests/db_functions/tests.py
and write some docs in https://github.com/django/django/blob/1f7ca858664491589ba400419a491dd0a9af5dff/docs/ref/models/database-functions.txt

For testing, you can use https://github.com/django/django-box to run tests on all databases except for Oracle.

Of course, this assumes you've read the previous contributing to django documentation. If you haven't, you should definitely start there.

comment:7 by Brad Melin, 8 years ago

Would this patch be considered non-trivial? I.e. should I discuss it in django-developers?

comment:8 by Josh Smeaton, 8 years ago

No, no need. Just make a start, open a PR, and we'll review there. If you need some help feel free to reply here or #django-dev on IRC

comment:9 by Brad Melin, 8 years ago

Has patch: set
Needs documentation: set
Version 0, edited 8 years ago by Brad Melin (next)

comment:10 by Brad Melin, 8 years ago

Needs documentation: unset

Turns out that MySQL also has the INSTR[1] function that uses the same argument order as the other backends. Using that instead.

[1] https://dev.mysql.com/doc/refman/5.7/en/string-functions.html#function_instr

Last edited 8 years ago by Brad Melin (previous) (diff)

comment:11 by Mariusz Felisiak, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Tim Graham, 8 years ago

Easy pickings: unset
Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

comment:13 by Brad Melin, 8 years ago

Patch needs improvement: unset

comment:14 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In b625907a:

Fixed #27834 -- Added StrIndex database function.

comment:15 by Tim Graham <timograham@…>, 8 years ago

In 7f8a924:

Refs #27834 -- Removed Value wrapping from StrIndex's substring param.

Note: See TracTickets for help on using tickets.
Back to Top