search

vasanthv / jsonbox

HTTP-based JSON storage.
https://jsonbox.io
MIT License
2.49k stars 171 forks source link

Add support for json-query syntax filtering #34

Closed harlev closed 4 years ago

harlev commented 5 years ago

Added an optional filtering using json-query. This should allow deep array filtering like the one requested in https://github.com/vasanthv/jsonbox/issues/30 .

It is important to note (also mentioned in the readme) this is an optional extra filtering, and will not affect any of the existing behavior if not explicitly added by the user.

Using the filter is by adding the jq url parameter. The supported syntax is detailed in the json-query page

harlev commented 5 years ago

There is an option of returning more types of data from such queries, like returning the actual parent of the items matching the filter. This will create a behavior more similar to the results returned by the current implementation. @vasanthv I'm looking for your guidance of where you would like to take this

vasanthv commented 5 years ago

@harlev I actually liked this json-query implementation, but the thing that stops me from merging it is, users can give complex queries on a 1000 records and running more queries like that can hold the node.js thread. So I am in a dilemma. Please advise if I am wrong.

harlev commented 5 years ago

@vasanthv Yes. Technically it is possible to run a complex query on all the records in a box. I have no indication on how resource intensive this could be. Similar issue if running on just a few (or even just one) records, with a very long array inside it (limited by the size of record supported).

We could create some limit on the number of records at the point we apply the json-query. It may solve typical cases, but will not be a complete protection for abuse (as noted in example above).

vasanthv commented 5 years ago

@harlev the limit is already set to 1000 records per fetch.

harlev commented 5 years ago

@vasanthv my question was if we want to further limit the record number, only for the cases json-query is applied

vasanthv commented 5 years ago

@harlev 1000 seems ok to me. If someone abused then we can act to reduce the number? Any thoughts?

harlev commented 5 years ago

@vasanthv In this case, we should be able to merge this PR as is.

vasanthv commented 4 years ago

I am not convinced on this approach as this can change the output format. I would still want to maintain the output consistent. Closing this now, as I am making new changes for version 2