Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to querying documents for sync filters #46

Open
sgwilym opened this issue Aug 14, 2020 · 4 comments
Open

Improvements to querying documents for sync filters #46

sgwilym opened this issue Aug 14, 2020 · 4 comments

Comments

@sgwilym
Copy link
Contributor

sgwilym commented Aug 14, 2020

What's the problem you want solved?

earthstar-graphql has rough support for sync filters, and this feature has been a little hairy to implement.

As of writing, earthstar-graphql's sync filters are shaped like this:

{
  pathPrefixes: string[],
  versionsByAuthors: []
}

The idea is that the peer/pub will return documents that match ANY of these rules.

However, querying documents works like this:

workspace.documents({
  pathPrefix: "/something"
  versionsByAuthor: "@test.1234"
})

And the documents returned must match ALL of the queries.

What this means is that implementing sync filters is a little bit hairy. Here's earthstar-graphql's implementation: https://github.com/earthstar-project/earthstar-graphql/blob/master/src/util.ts#L253

It calls the documents method once for each member of each property in the sync filters, and then puts all the different lists together. This method would probably get a little more unwieldy once more properties are supported.

Is there a solution you'd like to recommend?

Could there be ways to query a workspace's documents a bit more like how sync filters operate, i.e. using OR logic, and supporting lists for each property? A new method on IStorage, or a (breaking) change to documents?

@cinnamon-bun
Copy link
Member

Yeah, this is awkward. I think the IStorage query functions should accept an array of queries which would work in the same way as sync queries:

workspace.documents([
    {pathPrefix: "/about/"},
    {pathPrefix: "/wiki/"},
])

It would also still accept a single query object, or no query at all:

documents(q?: Query | Query[]) => Document[]

Matching against one query object, a document has to match EVERY property in the query. E.g. every property in a query object narrows down the search.

Overall, it would return documents that match ANY of the queries in the array. E.g. each additional query in the array broadens the search.

Would you be open to changing the GraphQL query to work this way instead of having arrays within one query object? I think this combination of narrowing and broadening will allow us to express complex queries.

@cinnamon-bun cinnamon-bun added this to the acorn milestone Aug 15, 2020
@cinnamon-bun
Copy link
Member

See also #47 (More comprehensive query options) for ideas about revamping query objects

@sgwilym
Copy link
Contributor Author

sgwilym commented Aug 17, 2020

@cinnamon-bun I like this, and I'm definitely opening to changing how filters work in earthstar-graphql. Sure it's a little more complex, but I'm not too worried as GraphQL will be able to help people understand how to make valid filters.

@cinnamon-bun
Copy link
Member

I've gotten stuck: what to do with the limit field? It makes sense to limit each query inside its own object, but what if you want an overall limit?

Maybe you just can't do that.

sgwilym pushed a commit that referenced this issue Feb 16, 2022
more organized rewrite of peer client and server
@sgwilym sgwilym removed this from the Acorn milestone May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants