Cross-DB JSONField ready for review

[Sage M.A.]

Hello, everyone.

As a follow-up to this message and this ticket, I have completed the implementation of a cross-DB JSONField.

I have submitted a PR, ready for review. Some folks have reviewed the PR multiple times (thanks!), but I think some more passes from new perspectives would be cool.

Regards,

Sage

[Carlton Gibson]

Hey Sage, 

Super stuff! Well done on your effort so far. (I can't say how excited I am about this feature. 💃)

Kind Regards,

Carlton

[Sage M.A.]

Hi Carlton,

Thanks! I hope it can be merged soon :D

Oh, and by the way, where should I write a setup guide for SQLite+JSON1? I don't know where and how to put it in the docs. By linking to a page in Django wiki, perhaps?

Regards,

Sage

[Adam Johnson]

Sage, I think that should go in the main docs, in the docs/ folder in the repo, no?

--
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 view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com.

--

Adam

[Sage M.A.]

Adam, that's what I thought at first. However, I don't know which section it should go under. Writing it on Model fields docs doesn't seem right. Maybe in a howto?

On Saturday, 3 August 2019 01:23:25 UTC+7, Adam Johnson wrote:

Sage, I think that should go in the main docs, in the docs/ folder in the repo, no?

On Fri, 2 Aug 2019 at 18:09, Sage M.A. <laym...@gmail.com> wrote:

Hi Carlton,

Thanks! I hope it can be merged soon :D

Oh, and by the way, where should I write a setup guide for SQLite+JSON1? I don't know where and how to put it in the docs. By linking to a page in Django wiki, perhaps?

Regards,

Sage

On Friday, 2 August 2019 22:08:17 UTC+7, Carlton Gibson wrote:

Hey Sage, 

Super stuff! Well done on your effort so far. (I can't say how excited I am about this feature. 💃)

Kind Regards,

Carlton

On Friday, 2 August 2019 13:46:46 UTC+2, Sage M.A. wrote:

Hello, everyone.

As a follow-up to this message and this ticket, I have completed the implementation of a cross-DB JSONField.

I have submitted a PR, ready for review. Some folks have reviewed the PR multiple times (thanks!), but I think some more passes from new perspectives would be cool.

Regards,

Sage

--
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-d...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com.

--

Adam

[Carlton Gibson]

Hi Sage. Perhaps draft it in a howto and we can have a look once it’s in play. C.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/bc75684b-dae2-4acb-bd4a-e24e5f9845b9%40googlegroups.com.

[Ole Laursen]

fredag den 2. august 2019 kl. 13.46.46 UTC+2 skrev Sage M.A.:

As a follow-up to this message and this ticket, I have completed the implementation of a cross-DB JSONField.

As someone who has been using the Postgres-specific JSONField extensively for dynamic, custom fields for the past couple of years, can I humbly suggest that the some more thought goes into the field lookup before the current approach is enshrined?

The simple .filter(field__foo="hello world") is actually fine, but it gets really weird if you let users define foo. What if they call foo something like "contains"? What if "foo" is actually "Foo the bar?". The JSON makes sense:

{

  "Foo the bar?": true

}

but the Django filter does not.

The current documentation says you can use __contains, but as far as I can tell, __contains, besides being difficult to understand, cannot be used for queries like __icontains. And it overwrites a built-in, otherwise useful operator.

My current wish list is:

- kill special contains and contained_by (use Q(field__foo="xxx") | Q(field__bar="yyy") instead)

- kill has_keys and has_any_keys (use Q(field__has_key="xxx") | Q(field__has_key="yyy") instead)

- add possibility of using something more robust than __ for path traversal, perhaps just a JSON extract string like JSONExtract("owner.other_pets[0].name")

This could perhaps also allow us to use the JSONField content in places where you currently can't (e.g. annotate). I'm not sure how exactly to combine it with field lookup and operators, but I'm personally okay with something like

   .filter(**{ "myfield__" + JSONExtract("owner.other_pets[0].name") + "__icontains": "baz" })

That's way better than what we have now.

The neat thing about JSONField is that it can store some of the things that are otherwise difficult to handle with traditional schemas. I think with some love and a set of more orthogonal primitives, we could query it easily from Django, too.

Ole

[schinckel]

Hi Ole,

I'm interested in what you are trying to do with JSONExtract. I have a subclass of Func called JSONBExtractPathText that I use with great success to extract parts of a JSONB object.

Also, as of Django 3.0, you can filter directly on an expression (that has an output_field of BooleanField).

Thus, you could write your first example as:

MyModel.objects.filter(JSONBExtractPathText('field', Value('Foo the bar?'), output_field=models.BooleanField())

I think you could possibly do the other stuff using either an ExpressionWrapper, or at worst a Case(When()).

(I hang out on #django on IRC if you want to discuss this in a more interactive manner).

Matt.

[Sage M.A.]

Hi Ole and Matt,

Sorry for getting back so late. I agree that having such a function would be very useful. I believe it can be done with most of the current implementation because I actually needed to compile the JSON path string from the KeyTransforms (except for PostgreSQL that uses a different syntax of its own). So, maybe I just need to add a check if it's an instance of JSONExtract and adapt accordingly.

However, I'm not really sure about killing the other lookups. I agree that it interferes with the possibility of using the lookup names as keys, but I was trying to retain all the functionalities from the original JSONField to make migration less painful.

Mariusz is going to prepare a review for the current implementation, so I think we'll be able to have a discussion about this as well.

Regards,

Sage