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

Fix/subject object scans #539

Merged
merged 5 commits into from
Jul 24, 2023
Merged

Fix/subject object scans #539

merged 5 commits into from
Jul 24, 2023

Conversation

zonotope
Copy link
Contributor

This patch scans all the flakes for a subject in the spot index and filters those flakes by object in the case we know the subject and object values to match against but do not know the predicate. This ensures we consider every flake necessary. Otherwise, we could skip some flakes depending on the structure of the underlying sorted set and which flakes it branches on.

This batch depends on #536, so please review that first.

Really Fixes fluree/core#18

@zonotope zonotope added this to the 3.0.0-beta1 milestone Jul 22, 2023
@zonotope zonotope requested a review from a team July 22, 2023 16:21
@zonotope zonotope self-assigned this Jul 22, 2023
@zonotope zonotope mentioned this pull request Jul 23, 2023
Copy link
Contributor

@mpoffald mpoffald 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 to me! I'd personally like to see a test that corresponds to the original bug report as well, but the fix itself looks good.

@@ -222,3 +222,38 @@
:schema/name "Brian"}]}
subject)
"returns all results in a map keyed by alias.")))))

(deftest ^:integration subject-object-test
Copy link
Contributor

@mpoffald mpoffald Jul 24, 2023

Choose a reason for hiding this comment

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

Since this is meant to fix deletion as reported in https://github.com/fluree/core/issues/18, could we also get a corresponding test in update-test ns?

in an object pattern under the `::fn` key (if any) by also checking if a
prospective flake object is equal to the supplied `o` value if and only if the
`:spot` index is used, the `p` value is `nil`, and the `s` and `o` values are
not `nil`b. In this case, the new object value returned by this function will
Copy link
Contributor

Choose a reason for hiding this comment

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

Little typo in the second sentence of the docstring:
" not nilb. " has an extraneous "b" at the end

Base automatically changed from fix/delete-all-matches to main July 24, 2023 21:06
@zonotope zonotope merged commit b106257 into main Jul 24, 2023
5 checks passed
@zonotope zonotope deleted the fix/subject-object-scans branch July 24, 2023 21:53
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