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

feat(iroh-sync): Queries and "views" #1766

Merged
merged 50 commits into from
Nov 7, 2023
Merged

feat(iroh-sync): Queries and "views" #1766

merged 50 commits into from
Nov 7, 2023

Conversation

Frando
Copy link
Member

@Frando Frando commented Nov 2, 2023

Description

Implementation of #1667, extends #1701

With the following changes after discussions in Discord:

  • Remove the notion of views, instead embed the info in the query. If the high-level concept of views makes more sense for people from an API perspective, we can restore it in the client api. However under the hood the query details are different per view, this is why here I expose a single Query struct
  • Add a query builder with a typestate to only allow possible combinations
  • Add an index to the redb (fs) document store to make queries that are sorted by key, or that are filtered by key but not by author, efficient. This also is what allows to do queries for the "latest only" entry per key, without allocating the full result set.

In the process I did a refactoring of the redb store to be safer to use. Especially, I moved the ouroboros self-referencing stuff into a ranges module, and encapsulated in an inner type to keep the self-referencing compilcations scoped. Also did something I had mind for a while: Add some type-safe abstractions around the range bounds constructions that are used when selecting on redb tables. All this turned out quite nice, IMO.

Also contains the changes from #1772 :

  • Renames get_one to get_exact
  • Add flag to get_exact whether to include empty entries or not
  • Add get_one to iroh::client::Doc to get a single entry with the same query mechanisms as get_many

Open questions and tasks:

  • Naming review of the query builder methods
  • Integration of the query parameters in the console

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates if relevant.
  • Tests if relevant.

@dignifiedquire
Copy link
Contributor

I think you need to rebase 😅 lots of conflicts

@Frando
Copy link
Member Author

Frando commented Nov 2, 2023

I think you need to rebase 😅 lots of conflicts

yes will do, started hacking away on #1701


// Records by key
// Key: (NamespaceId, Key, AuthorId)
// Value: same as in RECORDS_TABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be duplicating the full value here? seems like we could should store a reference to the records table (not sure how to and if that is actually better..)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not sure myself. It's one extra lookup vs. 156 bytes (value size) per entry.

In the most recent commit, I changed it to not duplicate the value but instead do the extra lookup.

@Frando Frando mentioned this pull request Nov 3, 2023
@dignifiedquire dignifiedquire added this to the v0.10.0 milestone Nov 6, 2023
…ntries, query single entry on client (#1772)

## Description

* Renames `get_one` to `get_exact`
* Add flag to `get_exact` whether to include empty entries or not
* Add `get_one` to `iroh::client::Doc` to get a single entry with the
same query mechanisms as `get_many`

Based on #1766

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- ~~[ ] Tests if relevant.~~
@Frando Frando changed the title feat(iroh-sync): Queries and "views" take 2 feat(iroh-sync): Queries and "views" Nov 6, 2023
@Frando
Copy link
Member Author

Frando commented Nov 7, 2023

I merged main and solved the conflicts from #1770.

Also added sort and desc options to doc keys console command.

This is now ready to go from my side.

@Frando Frando added this pull request to the merge queue Nov 7, 2023
Merged via the queue into main with commit 899768a Nov 7, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants