Scope blank nodes to each federated source #624

rubensworks · 2020-03-16T14:52:24Z

When federating over multiple sources,
the outcoming blank nodes coming from each source
will receive a distinct blank node,
so that they can not be joined across different sources,
even if they have the same blank node label.

A reverse translation also takes place for incoming queries with blank nodes,
so that these blank nodes will only match if they come from that source.

Blank nodes coming from sources will receive a .skolemized field
containing a named node.
This named node can be queried again as an IRI, and this will
be interpreted by Comunica as a blank node corresponding to the proper
source, assuming that the array of sources remains the same.

Closes #355
Required for LDflex/Query-Solid#34

While the implementation is quite trivial, the impact is quite major, so this deserves a detailed review.

When federating over multiple sources, the outcoming blank nodes coming from each source will receive a distinct blank node, so that they can not be joined across different sources, even if they have the same blank node label. A reverse translation also takes place for incoming queries with blank nodes, so that these blank nodes will only match if they come from that source. Blank nodes coming from sources will receive a .skolemized field containing a named node. This named node can be queried again as an IRI, and this will be interpreted by Comunica as a blank node corresponding to the proper source, assuming that the array of sources remains the same. Closes #355 Required for LDflex/Query-Solid#34

CLAassistant · 2020-03-16T14:52:53Z

All committers have signed the CLA.

coveralls · 2020-03-16T15:03:51Z

Pull Request Test Coverage Report for Build 2163

37 of 37 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 2142:	0.0%
Covered Lines:	4116
Relevant Lines:	4116

💛 - Coveralls

rubensworks · 2020-03-16T15:26:13Z

Note to self: test BlankNodeSkolemizable#equals

packages/actor-rdf-resolve-quad-pattern-federated/lib/BlankNodeSkolemizable.ts

@@ -0,0 +1,22 @@
+import * as RDF from "rdf-js";
+
+export class BlankNodeSkolemizable implements RDF.BlankNode {


packages/actor-rdf-resolve-quad-pattern-federated/lib/BlankNodeSkolemizable.ts

+  }
+
+  public equals(other: RDF.Term | null | undefined): boolean {
+    return other && other.termType === 'BlankNode' && other.value === this.value;


packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+      const match = /^urn:comunica_skolem:source_([0-9]+):(.+)$/.exec(term.value);
+      if (match) {
+        // We had a skolemized term
+        if (parseInt(match[1], 10) === sourceId) {


packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+   */
+  public static deskolemizeTerm(term: RDF.Term, sourceId: number): RDF.Term | false {
+    if (term.termType === 'NamedNode' || term.termType === 'BlankNode') {
+      const match = /^urn:comunica_skolem:source_([0-9]+):(.+)$/.exec(term.value);


packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+  public static skolemizeTerm(term: RDF.Term, sourceId: number): RDF.Term | BlankNodeSkolemizable {
+    if (term.termType === 'BlankNode') {
+      const skolemized = `urn:comunica_skolem:source_${sourceId}:${term.value}`;
+      return new BlankNodeSkolemizable(skolemized, DataFactory.namedNode(skolemized));


packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+   * @param term Any RDF term.
+   * @param sourceId A numerical source identifier.
+   */
+  public static deskolemizeTerm(term: RDF.Term, sourceId: number): RDF.Term | false {


RubenVerborgh · 2020-03-17T19:37:46Z

packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+  public getSourceId(source: IDataSource): number {
+    let sourceId = this.sourceIds.get(source);
+    if (sourceId === undefined) {
+      sourceId = this.sourceIds.size;


Actually, you might want to give sources the opportunity to optionally define their own ID. Then, if a source is hidden behind a cache or something, it would still give the same ID.

packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

-        FederatedQuadSource.nullToVariable(graph, 'g'),
-      );
+      return new PromiseProxyIterator(async () => {
+        // Deskolemize terms, so we send the original blank nodes to each source.


packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

-      );
+      return new PromiseProxyIterator(async () => {
+        // Deskolemize terms, so we send the original blank nodes to each source.
+        const s = FederatedQuadSource.deskolemizeTerm(FederatedQuadSource.nullToVariable(subject, 's'), sourceId);


Remove regex, parseInt, and optimize return types for V8

rubensworks · 2020-03-24T11:06:08Z

@RubenVerborgh Made some changes based on your comments.
Could you have another look at it?
It's important that we get this part right :-)

RubenVerborgh

Great stuff!

RubenVerborgh · 2020-03-26T14:21:25Z

packages/actor-rdf-resolve-quad-pattern-federated/lib/BlankNodeScoped.ts

+  }
+
+  public equals(other: RDF.Term | null | undefined): boolean {
+    return other && other.termType === 'BlankNode' && other.value === this.value;


strictly !!other && …, but that will do

Right...
I should really enable those strict null checks in TS, because that would have detected this problem at compile time.

RubenVerborgh · 2020-03-26T14:24:23Z

packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+        const termSourceId = term.value.substr(FederatedQuadSource.SKOLEM_PREFIX.length, colonSeparator - FederatedQuadSource.SKOLEM_PREFIX.length);
+        // We had a skolemized term
+        if (termSourceId === sourceId) {
+          // It can from the correct source


RubenVerborgh · 2020-03-26T14:25:54Z

packages/actor-rdf-resolve-quad-pattern-federated/lib/FederatedQuadSource.ts

+        // Deskolemize terms, so we send the original blank nodes to each source.
+        // Note that some sources may not match bnodes by label. SPARQL endpoints for example consider them variables.
+        const s = !subject   ? DataFactory.variable('vs') : FederatedQuadSource.deskolemizeTerm(subject,   sourceId);
+        const p = !predicate ? DataFactory.variable('vp') : FederatedQuadSource.deskolemizeTerm(predicate, sourceId);


Predicates shouldn't be blanks I guess, but better to be safe than sorry.

True. By keeping it generic, we can in the future have support for generalized RDF should that ever be needed.

rubensworks added the feature ➕ label Mar 16, 2020

rubensworks requested review from RubenVerborgh and joachimvh March 16, 2020 14:52

rubensworks self-assigned this Mar 16, 2020

RubenVerborgh reviewed Mar 17, 2020

View reviewed changes

rubensworks added 5 commits March 24, 2020 11:15

Rename BlankNodeSkolemizable to BlankNodeScoped

1322bab

Make label of BlankNodeScoped more human-readable

94af4f1

Optimize deskolemization

d3af1e8

Remove regex, parseInt, and optimize return types for V8

Add note on matching blank nodes in sources

1f556eb

Simplify handling of falsy terms and deskolemization

51e4a6d

Add unit tests for BlankNodeScoped

68ca460

rubensworks force-pushed the feature/skolemization branch from 1c60d7d to 68ca460 Compare March 24, 2020 12:43

RubenVerborgh approved these changes Mar 26, 2020

View reviewed changes

Resolve final comments by RV

47c80ef

rubensworks merged commit cb7b9b2 into master Mar 26, 2020

rubensworks deleted the feature/skolemization branch March 26, 2020 14:35

justinwb mentioned this pull request Mar 27, 2020

Allow blank nodes to be reused across expressions LDflex/Query-Solid#34

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scope blank nodes to each federated source #624

Scope blank nodes to each federated source #624

rubensworks commented Mar 16, 2020

CLAassistant commented Mar 16, 2020 •

edited

Loading

coveralls commented Mar 16, 2020 •

edited

Loading

rubensworks commented Mar 16, 2020

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

RubenVerborgh Mar 17, 2020

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

rubensworks commented Mar 24, 2020 •

edited

Loading

RubenVerborgh left a comment

RubenVerborgh Mar 26, 2020

rubensworks Mar 26, 2020

RubenVerborgh Mar 26, 2020

RubenVerborgh Mar 26, 2020

rubensworks Mar 26, 2020

		@@ -0,0 +1,22 @@
		import * as RDF from "rdf-js";

		export class BlankNodeSkolemizable implements RDF.BlankNode {

Scope blank nodes to each federated source #624

Scope blank nodes to each federated source #624

Conversation

rubensworks commented Mar 16, 2020

CLAassistant commented Mar 16, 2020 • edited Loading

coveralls commented Mar 16, 2020 • edited Loading

Pull Request Test Coverage Report for Build 2163

💛 - Coveralls

rubensworks commented Mar 16, 2020

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

RubenVerborgh Mar 17, 2020

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

rubensworks commented Mar 24, 2020 • edited Loading

RubenVerborgh left a comment

Choose a reason for hiding this comment

RubenVerborgh Mar 26, 2020

Choose a reason for hiding this comment

rubensworks Mar 26, 2020

Choose a reason for hiding this comment

RubenVerborgh Mar 26, 2020

Choose a reason for hiding this comment

RubenVerborgh Mar 26, 2020

Choose a reason for hiding this comment

rubensworks Mar 26, 2020

Choose a reason for hiding this comment

CLAassistant commented Mar 16, 2020 •

edited

Loading

coveralls commented Mar 16, 2020 •

edited

Loading

rubensworks commented Mar 24, 2020 •

edited

Loading