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

Fixed exact-match to numeric literal #106

Closed
wants to merge 1 commit into from

Conversation

gsvarovsky
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Owner

jacoscaz commented Oct 12, 2020

@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 added a commit that referenced this pull request Oct 18, 2020
…5), drops the search API (closes #104), fixes exact match to numeric literals (closes #106)
@jacoscaz
Copy link
Owner

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.

@jacoscaz jacoscaz closed this Oct 18, 2020
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.

None yet

2 participants