Implement JSON array filtering functions #1493

Jinxiao0302 · 2022-08-02T05:40:53Z

Related to #1410

Technical details
Added a DBFunction for getting JSON array length;
To be done: Add comparaing functions that nest array length function.

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the master branch of the repository
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no
visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

dmos62

Regarding your question about how to use the ArrayLength to compose a filter like json array length is less than (I presume that that's a valid approximation of your goal), create a DBFunctionPacked subclass inside db.types.custom.json_array. DBFunctionPacked (and the whole of hints.mathesar_filter mechanism, really) is there to bypass the lack of composition in our current UI filtering system.

dmos62 · 2022-08-02T10:53:18Z

db/functions/base.py

+
+    @staticmethod
+    def to_sa_expression(value):
+        return func.json_array_length(value)


Actually, since this DBFunction depends on the custom type json_array which has its own namespace (db.types.custom.json_array), it makes sense to put this DBFunction in there too (some other namespaces under db.types.custom already do this). You're actually already half-way there, since you've added these modules to the list of modules to search for DBFunctions in (db.functions.known_db_functions._modules_to_search_in).

dmos62 · 2022-08-02T11:18:45Z

db/functions/hints.py

+json_array = _make_hint("json_array")
+
+
+json_object = _make_hint("json_object")


You'll want to tell Mathesar how these hints relate to actual types. Look at db/types/hintsets.py. Note this line:

_add_to_db_type_hintsets(categories.JSON_TYPES, (hints.json,))

This says that all database types defined as categories.JSON_TYPES are associated with the hints (hints.json,).

You'll probably want to make changes to this. And, to the related categories in db.types.categories as well, since currently we only have categories.JSON_TYPES, which is a catch-all category for everything JSON, and you probably want something more granular.

Note how the categories sets define a sort of hierarchy, where for example time-related types are made up of point-in-time as well as duration types, those in turn are made up of other more granular categories, etc.

By the way, if this wasn't clear before (probably not), the way Mathesar finds what hints are associated with what UI type, is it takes the intersection of a UI type's DB types' hints (a UI type is effectively a set of DB types, and each of those have a set of hints, as seen in db/types/hintsets.py); and, the way it checks whether a filter is applyable to a column of a given UI type, is that it checks if the hints of the first parameter of that filter are satisfied by the hints of the UI type. "Satisfied" here is intentionally ambiguous, since we want the freedom to evolve how this works, but at the moment this mostly means that the first param hint-set must be a subset of the UI type hint-set.

Oof, sorry for the wall of text. Thumbs up for braving through 👍👍, feel free to reach out.

dmos62 · 2022-08-02T11:25:40Z

db/functions/base.py

+        hints.returns(hints.comparable),
+        hints.parameter_count(1),
+        hints.parameter(0, hints.json_array),
+        hints.mathesar_filter,


This isn't actually a filter (doesn't return a boolean; not meant to be used directly by the UI). You'll probably use this function to compose a DBFunctionPacked-based filter and you'll give that a mathesar_filter hint, but giving this function here a mathesar_filter will result in unexpected behaviour.

Thanks for your very helpful explanation! @dmos62 , I added a DBFunctionPacked function using ArrayLength in the db/types/custom/json_array and also moved the ArrayLength into that file. Also, I added the json array into categories.

codecov-commenter · 2022-08-02T22:42:12Z

Codecov Report

Merging #1493 (263552c) into master (bff8f2d) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1493      +/-   ##
==========================================
+ Coverage   92.27%   92.30%   +0.03%     
==========================================
  Files         139      139              
  Lines        6060     6084      +24     
==========================================
+ Hits         5592     5616      +24     
  Misses        468      468

Flag	Coverage Δ
pytest-backend	`92.30% <100.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
db/functions/hints.py	`95.00% <100.00%> (+0.17%)`	⬆️
db/functions/known_db_functions.py	`100.00% <100.00%> (ø)`
db/types/categories.py	`100.00% <100.00%> (ø)`
db/types/custom/json_array.py	`95.91% <100.00%> (+2.36%)`	⬆️
db/types/hintsets.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

mathemancer

Hey @Jinxiao0302 , as noted through other channels I'd like you to add some tests before we merge this PR. Thank you, it's looking good so far!

…to json-fix update branch

pavish

The changes look good to me. As @mathemancer mentioned, we can merge this in once we have tests. I'm approving from my end.

mathemancer

Okay, I think this looks good. Let's get it merged!

Jinxiao0302 added 2 commits August 2, 2022 01:28

create DBFunction to get json array lengths

feddbf3

create DBFunction to get json array length

053c4b9

Jinxiao0302 requested review from mathemancer and dmos62 August 2, 2022 05:46

dmos62 reviewed Aug 2, 2022

View reviewed changes

Jinxiao0302 added 2 commits August 2, 2022 18:25

add dbfunction pack for json array length equals

4425a72

revert a mischange

1d4bc7e

Jinxiao0302 requested a review from dmos62 August 3, 2022 17:57

Jinxiao0302 added 3 commits August 4, 2022 09:01

fix identifier naming problem

7d6522d

fix code formatting issues

998fadc

Merge branch 'master' into json-fix

19d21c6

Jinxiao0302 marked this pull request as ready for review August 4, 2022 20:08

Jinxiao0302 requested review from a team and rajatvijay and removed request for a team August 4, 2022 20:08

kgodey assigned mathemancer Aug 8, 2022

kgodey requested review from pavish and removed request for rajatvijay August 8, 2022 15:38

kgodey assigned pavish Aug 8, 2022

kgodey added the pr-status: review A PR awaiting review label Aug 8, 2022

mathemancer requested changes Aug 9, 2022

View reviewed changes

Jinxiao0302 added 3 commits August 9, 2022 19:15

add testings for json array filter functions

cdb1dbb

Merge branch 'json-fix' of https://github.com/Jinxiao0302/mathesar in…

d52361b

…to json-fix update branch

fix coding style issues

d393993

Jinxiao0302 requested a review from a team August 9, 2022 23:24

Delete 1

ac420a7

pavish approved these changes Aug 10, 2022

View reviewed changes

Merge branch 'master' into json-fix

263552c

mathemancer approved these changes Aug 11, 2022

View reviewed changes

mathemancer enabled auto-merge August 11, 2022 02:49

mathemancer merged commit 6557591 into mathesar-foundation:master Aug 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement JSON array filtering functions #1493

Implement JSON array filtering functions #1493

Jinxiao0302 commented Aug 2, 2022

dmos62 left a comment

dmos62 Aug 2, 2022 •

edited

dmos62 Aug 2, 2022 •

edited

dmos62 Aug 2, 2022 •

edited

Jinxiao0302 Aug 2, 2022

codecov-commenter commented Aug 2, 2022 •

edited

mathemancer left a comment

pavish left a comment

mathemancer left a comment

		json_array = _make_hint("json_array")


		json_object = _make_hint("json_object")

Implement JSON array filtering functions #1493

Implement JSON array filtering functions #1493

Conversation

Jinxiao0302 commented Aug 2, 2022

Checklist

Developer Certificate of Origin

dmos62 left a comment

Choose a reason for hiding this comment

dmos62 Aug 2, 2022 • edited

Choose a reason for hiding this comment

dmos62 Aug 2, 2022 • edited

Choose a reason for hiding this comment

dmos62 Aug 2, 2022 • edited

Choose a reason for hiding this comment

Jinxiao0302 Aug 2, 2022

Choose a reason for hiding this comment

codecov-commenter commented Aug 2, 2022 • edited

Codecov Report

mathemancer left a comment

Choose a reason for hiding this comment

pavish left a comment

Choose a reason for hiding this comment

mathemancer left a comment

Choose a reason for hiding this comment

dmos62 Aug 2, 2022 •

edited

dmos62 Aug 2, 2022 •

edited

dmos62 Aug 2, 2022 •

edited

codecov-commenter commented Aug 2, 2022 •

edited