Cross-DB JSONField ready for review

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

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

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d68c0bb6-40cc-418e-b511-60caf4b5a8ca%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

Carlton Gibson-3
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 <a href="https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;">this message and <a href="https://code.djangoproject.com/ticket/12990" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;">this ticket, I have completed the implementation of a cross-DB JSONField.
I have submitted a <a href="https://github.com/django/django/pull/11452" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F11452\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFGzZgNexFAc86_KGcdJ_UUW_Bw4w&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F11452\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFGzZgNexFAc86_KGcdJ_UUW_Bw4w&#39;;return true;">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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d8c946e2-9111-488a-9869-c6b4e555fac3%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

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

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 <a href="https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;">this message and <a href="https://code.djangoproject.com/ticket/12990" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;">this ticket, I have completed the implementation of a cross-DB JSONField.
I have submitted a <a href="https://github.com/django/django/pull/11452" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F11452\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFGzZgNexFAc86_KGcdJ_UUW_Bw4w&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F11452\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFGzZgNexFAc86_KGcdJ_UUW_Bw4w&#39;;return true;">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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

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


--
Adam

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM1YBQw9Pb20h5QFjYXVJsWHvSAGbqW2_yuMtrnm6o2yyA%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

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. <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="LzMV3icnEwAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">laym...@...> 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 <a href="https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;">this message and <a href="https://code.djangoproject.com/ticket/12990" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;">this ticket, I have completed the implementation of a cross-DB JSONField.
I have submitted a <a href="https://github.com/django/django/pull/11452" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F11452\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFGzZgNexFAc86_KGcdJ_UUW_Bw4w&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fpull%2F11452\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFGzZgNexFAc86_KGcdJ_UUW_Bw4w&#39;;return true;">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 <a href="javascript:" target="_blank" gdf-obfuscated-mailto="LzMV3icnEwAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">django-d...@googlegroups.com.
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com?utm_medium=email&amp;utm_source=footer" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com.


--
Adam

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/bc75684b-dae2-4acb-bd4a-e24e5f9845b9%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

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

On Sat, 3 Aug 2019 at 03:23, Sage M.A. <[hidden email]> wrote:
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. <[hidden email]> 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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/38824b73-162f-49b1-8709-887cace1af7c%40googlegroups.com.


--
Adam

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/bc75684b-dae2-4acb-bd4a-e24e5f9845b9%40googlegroups.com.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAJwKpyTPFi1sy9vfvbfba_oe%3DsdEb7Bsne_XK9R-Sai2hTDShA%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

Ole Laursen-3
In reply to this post by Sage M.A.
fredag den 2. august 2019 kl. 13.46.46 UTC+2 skrev Sage M.A.:
As a follow-up to <a href="https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/forum/#!msg/django-developers/M4dYz7T2SUo/b5RVjJHxBQAJ&#39;;return true;">this message and <a href="https://code.djangoproject.com/ticket/12990" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fcode.djangoproject.com%2Fticket%2F12990\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEjlJhBNp2A1Jg00u4gfMlkKRMOPg&#39;;return true;">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

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/92277b8e-4453-496e-b0aa-e15b79a9d0a2%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

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.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/45fb5bf5-c1d8-44eb-973b-56914264fcc3%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Cross-DB JSONField ready for review

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

On Friday, 13 September 2019 11:09:14 UTC+7, schinckel wrote:
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.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/d3199e1e-97ed-4196-a94a-809e0eb063b0%40googlegroups.com.