PR 9583 - Ticket 28643

59 views
Skip to first unread message

Matthew Pava

unread,
Jan 18, 2018, 1:06:29 PM1/18/18
to django-d...@googlegroups.com

Hi everyone,

I’ve been working on ticket 28643 specifically adding Ord, Chr, Left, and Right to Django’s built-in database functions.  I’ve done the PR #9583.  I am a new contributor, and I’ve never really contributed code to any project through pull requests ever before.  I appreciate your guidance.  The checks have passed, and I’ve got a mess of commits that I’ve tried to squash a couple of times but seem to stick around nonetheless.

 

I also made a comment on the ticket that maybe instead of using Left, Right, and Substr functions that we instead implement slicing on fields and expressions to make things more pythonic (and deprecate Left, Right, and Substr).  Would such an implementation need a new ticket?  And then would we need to consider slicing all iterables instead of just only CharFields, TextFields, and string expressions?

 

Also, most of the functions in ticket 28643 that go across all supported Django database backends are complete, with the exception of ROUND.  SQLite does not come with many built-in math functions, though there are extensions available.  I was wondering how we should proceed with that ticket when ROUND is completed.

 

Thank you!

 

https://github.com/django/django/pull/9583

https://code.djangoproject.com/ticket/28643

 

charettes

unread,
Jan 18, 2018, 4:21:57 PM1/18/18
to Django developers (Contributions to Django itself)
Hey Matthew,

I could see adding support for F('field')[0:5] being useful and more intuitive
than using Left/Right/Substr. You should be able to assert the field is sliceable
(e.g. an instance of CharField/TextField) in the expression returned by
F.__getitem__ (e.g. Left/Right/Substr) resolve_expression method.

Maybe having a F.__getitem__ return an intermediary Expression subclass
(e.g. FieldSlice) that resolves to the correct expression based on the type of
the sliced field and the nature of the slice in resolve_expression would work
better here.

I could see this having great benefit for usage with nested data structure fields
such as contrib.postgres ArrayField (e.g F('array')[0:10]) and JSONField where
we would finally have a way to specify a key without colliding with the builtin
lookups (e.g. F('data')['exact'], F('data')['exact'][0:10]).

That might be bit too much for this ticket though and you might want to only
focus on adding Left/Right/Substr for now.

Best,
Simon

Josh Smeaton

unread,
Jan 18, 2018, 5:30:17 PM1/18/18
to Django developers (Contributions to Django itself)
I think having Left/Right is the right way to go. If there are opportunities to use slicing syntax, then they would resolve internally to those Expressions I would think. But that can be done separately, and have greater scope such as ArrayField in Simons example.

Adam Johnson

unread,
Jan 18, 2018, 6:19:11 PM1/18/18
to django-d...@googlegroups.com
+1 for adding Left/Right as plain functions right now.

Simon's example for a new future is really nice but outside the scope of the ticket.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/af172482-ffb2-4b0c-ba2f-5f16e876302c%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Adam
Reply all
Reply to author
Forward
0 new messages