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

FC-1429: Don't consider any flakes for a collection when no ecount set #194

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

zonotope
Copy link
Contributor

The max subject id used by the database for a particular collection is set on
the db in the ecounts map as flakes in that collection are added to the
ledger. There's no value set in the map for that collection before any flakes in
that collection have been added. This has the effect of making the collection
have no upper limit during query processing, and flakes in any collection that
comes after the one in question are considered for analytical queries.

This patch changes the query engine so it doesn't load any flakes from
collections that don't have an ecount set because the ecount not being set means
there aren't any flakes in that collection.

The max subject id used by the database for a particular collection is set on
the db in the `ecounts` map as flakes in that collection are added to the
ledger. There's no value set in the map for that collection before any flakes in
that collection have been added. This has the effect of making the collection
have no upper limit during query processing, and flakes in any collection that
comes after the one in question are considered for analytical queries.

This patch changes the query engine so it doesn't load any flakes from
collections that don't have an ecount set because the ecount not being set means
there aren't any flakes in that collection.
@zonotope zonotope changed the title Don't consider any flakes for a collection when no ecount set FC-1429: Don't consider any flakes for a collection when no ecount set Aug 23, 2022
@zonotope zonotope requested a review from a team August 23, 2022 12:36
@@ -420,7 +420,11 @@
(let [partition (dbproto/-c-prop db :partition (last clause))
max-sid (-> db :ecount (get partition))
min-sid (flake/min-subject-id partition)
flakes (<? (query-range/index-range db :spot >= [max-sid] <= [min-sid]))
flakes (if-not max-sid
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nit-pick, but a tiny readability preference I have is to avoid negated conditionals if I can simply swap the then/else clauses. It saves me an extra brain cycle when reading later. But I'll defer to your judgment either way 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I tend to agree with you, but I explicitly picked if-not this time because the deciding factor in my mind was whether or not max-sid was null. However, this is completely arbitrary, so I changed it to an if because there should be a non-arbitrary reason to use if-not. Thanks for the suggestion.

Copy link
Contributor

@dpetran dpetran left a comment

Choose a reason for hiding this comment

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

Looks good!

@zonotope zonotope merged commit 8296cdb into maintenance/v1 Aug 23, 2022
@zonotope zonotope deleted the bug/rdf-type branch August 23, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants