quadstorejs / quadstore

A LevelDB-backed graph database for JS runtimes (Node.js, Deno, browsers, ...) supporting SPARQL queries and the RDF/JS interface.
https://github.com/quadstorejs/quadstore
MIT License
203 stars 14 forks source link

Fixed exact-match to numeric literal #106

Closed gsvarovsky closed 4 years ago

gsvarovsky commented 4 years ago

I came across this problem in testing #91. Without the fix in serialization.ts, the new test fails.

SPARQL test results are:

test-rdf:sparql10: 219 / 438 tests succeeded

test-rdf:sparql11: 97 / 271 tests succeeded

I'll merge this change into #91 too if OK.

jacoscaz commented 4 years ago

This is an interesting issue that probably requires more research. The reason why I use the rangeBoundary parameter is that I sometimes want to check for equality only on the string representation of the numerical value:

See https://github.com/beautifulinteractions/node-quadstore/blob/d31766221ea9163e85296c6ad80367e04ed12825/lib/rdf/serialization.ts#L80-L83 :

https://github.com/beautifulinteractions/node-quadstore/blob/d31766221ea9163e85296c6ad80367e04ed12825/lib/rdf/fpstring.ts#L27-L31

What this allows us to do is to compare numerical values across different numerical datatypes, which is very important in our primary use case. Dropping that parameter prevents us from being able to do so. This said, there clearly is an issue somewhere that needs to be addressed; I will look at this as soon as possible and get back to you.

jacoscaz commented 4 years ago

@gsvarovsky I might have found something but I won't have time to test it properly until Thursday or Friday. If you have the spare cycles, could you test the following change?

src/lib/rdf/serialization.ts, line 164:

      case 'Literal':
        if (rangeBoundary) {
          const value = importLiteralTerm(term, rangeBoundary);
          return { gte: value, lte: value };
        }
        return importLiteralTerm(term, rangeBoundary);
      default:
jacoscaz commented 4 years ago

Closing this PR as I've manually brought in the fix and the test into my current working branch, which is about to become the new master.