Skip to content

Fix compacting property-based indexes. #528

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

Merged
merged 5 commits into from
May 20, 2021
Merged

Fix compacting property-based indexes. #528

merged 5 commits into from
May 20, 2021

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented May 15, 2021

The value of @index on a property map can be a Compact IRI in addition to an IRI. The Compaction Algorithm is updated to first expand this value before re-compacting it. Added additional language to the Compaction Algorithm step 12.8.9.6.1 to first expand container key before compacting it.

Also fixed an emergent issue with WebIDL reference for LoadDocumentCallback/url.

Adds two new compaction tests.

Fixes #514.

CC/ @vcharpenay


Preview | Diff

@gkellogg gkellogg requested review from davidlehn and pchampin May 15, 2021 19:02
<li>Reinitialize <var>container key</var> by <a>IRI compacting</a>
<var>index key</var>.</li>
<li id="alg-compact-12_8_9_6_1">Reinitialize <var>container key</var> by <a>IRI compacting</a>
<var>index key</var> after first <a>IRI expanding</a> it.</li>
Copy link
Member Author

Choose a reason for hiding this comment

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

An alternative to consider, which is probably more correct, would be to do the expansion of the @index value when the context is processed, as the value may expand differently than it would when compacting. It would still be a localized change, but not to the Compaction algorithm.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did try this, but remembering the original compacted term is necessary for expansion. The most correct solution would be to save the expanded version of this in the term definition, and use that in the Compaction algorithm instead of the original version, but that would be more disruptive to the rest of the document. This solution will solve any issues other than those specifically constructed to defeat it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class-3 Class-3 change spec:bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No roundtrip with property-based data indexing
3 participants