Backend-specific lookups

[Shai Berger]

Hi,

I'v been working on an old Oracle bug[1], and realized that a nice way to
solve it would be by overriding some builtin lookups with custom lookups for
Oracle. However, I had some doubts about where to place the code:

On the one hand, I could just add an "as_oracle()" to the builtin lookup
classes. But I felt that would be cheating -- Oracle is in core, but what
would 3rd-party backends do?

So I thought I would implement a custom lookup in the backend, which adds an
"as_oracle", and register that when the backend is loaded. Loic commented that
this is still problematic -- For example, I want to override "contains" on
TextField's, so I would naturally inherit the built-in Contains lookup; but if
a user has two backends which want to override "contains", they will be in
conflict -- only one of them will get to install its handler.

The solution I found was to use dynamic inheritance -- the backend inherits
not the builtin Contains, but the registered lookup for "contains" on
TextField. This way, another backend can also add its own as_vendor() method,
without any of us stepping on each other's feet.

I've posted the initial example of this as a PR[2] for comments.

This is related to some of the discussions about transforms (SQL Functions)
that happened on the mailing list a couple of months ago[3], but the topic of
backend-specific functions was not really the topic there, so I preferred to
open a new discussion.

Have fun,
Shai.


[1] https://code.djangoproject.com/ticket/11580
[2] https://github.com/django/django/pull/3544
[3] https://groups.google.com/d/msg/django-developers/HggiPzwkono/dsnx3BuXpnkJ

[Claude Paroz]

Hi Shai,

I was thinking about something very similar for the GIS backends, where this question is even more accurate, as lookup implementations are often very backend-specific.

I was experimenting with dynamic mixin registration, but that felt very hackish [1]. Anyway, a solution to this use case would be really welcome.

Claude

[1] https://gist.github.com/claudep/c234e546eab00ff9fdbf

[Josh Smeaton]

Clever. I don't mind this approach at all. Will the overriding of get_db_prep_lookup interfere with other implementations though?

[Michael Manfre]

Overriding get_db_prep_lookup seems like it would cause problems. Even if were only registered when Oracle is in use, that would make multi-db with Oracle as one of the backends problematic.

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/32571a2b-92cf-46fb-9373-a88772586349%40googlegroups.com.

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

[Shai Berger]

On Sunday 16 November 2014 00:46:43 Josh Smeaton wrote:
> Clever. I don't mind this approach at all. Will the overriding of
> get_db_prep_lookup interfere with other implementations though?
>

Yes, it probably would. It is there because the default one for the lookup
adds % signs around the value, which I didn't want; and within the evening I
couldn't find the clean way to get rid of that [in retrospect, I think I need
to invoke the grand-grand-parent -- super(BuiltinLookup, self).process_rhs()]

Shai.

[Marc Tamlyn]

The test failures on the CI would suggest that overriding `get_db_prep_lookup` is a problem!

There are changes on my ranges branch which move the % sign addition out of `Field.get_db_prep_lookup` and into the relevant lookup. This makes overriding certain lookups (e.g. contains, startswith etc) much cleaner for field types where that name makes sense but it does not mean text. You can see those here: https://github.com/mjtamlyn/django/commit/3c45e2cfecbc2c8da6e4a05c5db3367c5cabfde7

I've commented on the PR with more details.

--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/201411160106.16550.shai%40platonix.com.

[Anssi Kääriäinen]

A possible solution is to just register a custom as_vendor method in the
backend's initialization code.

You could also alter Oracle's compiler.compile() method to check for
backend specific implementation class, and use that instead of the
original one. Something like:

backend_lookup_overrides = {Contains: OracleContains}
def OracleSQLCompiler.compile(self, node):
if node.__class__ in backend_lookup_overrides:
return backend_lookup_overrides[node.__class__](node.lhs,
node.rhs).as_sql(self, self.connection)

As for get_db_prep_lookup(), I think we will need to remove it. It is
doing per-lookup stuff that is now better done in Lookup classes.

Unfortunately we need backwards compatibility for this method as it is a
documented method (https://docs.djangoproject.com/en/dev/ref/models/fields/#django.db.models.Field.get_db_prep_lookup)
I don't yet have an idea of how we can actually do the removal without
breaking user code.

- Anssi

[Shai Berger]

I just wanted to drop a line here saying "it will take me another couple of
days to deal with this".

On one hand, I want to wait with this for Marc's work to land. On the other,
there are more urgent issues even in Oracle-land (the broken Py3 tests).

And yes, I'm with Anssi on removing get_db_prep_lookup() -- the partition
between "make ready for field type" and "make ready for lookup type" that is in
the current code is weird and causes problems, which custom lookups solve.

However, note that if Anssi is correct about the backwards-compatibility
point, then Marc's work changing Contains may not be backwards-compatible.

More later,
Shai.