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

xsd:string is the datatype by default and becomes implicit in SPARQL results srx srj #76

Merged
merged 1 commit into from
Feb 5, 2023

Conversation

BorderCloud
Copy link
Contributor

xsd:string seems to become the datatype by default and so, it becomes implicit in sparql results.

Here, I propose to modify the SPARQL results of tests in order to be in coformity with the SPARQL results generate today by the classic SPARQL services.

Copy link
Member

@gkellogg gkellogg left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

@gkellogg gkellogg requested a review from afs May 14, 2021 18:25
@afs
Copy link
Contributor

afs commented May 15, 2021

There is at least one major triple store that provides RDF 1.0, not RDF 1.1.

There are probably RDF1.0 data around on the web.

These tests would work in a SPARQL 1.1/RDF 1.0 system as well because the data files align with the result files. jsonres tests also involve ORDER BY so there may be other effects.

While I wouldn't go out of the way to retain RDF 1.0 compatibility for new work, I don't think making a change when there isn't a need (bug or necessary change) helps overall.

SPARQL client software still has to deal with explicit xsd:string to be compliant so these do check something.

Minor: the original EARL reports were with RDF 1.0.

@gkellogg
Copy link
Member

@BorderCloud based on @afs's response, I've marked this as "wont fix". If you agree, you can close the PR.

@afs
Copy link
Contributor

afs commented May 16, 2021

It does ask the question what is this for - this community could decide that this repo is only about "now", or should reflect the history. There are good arguments for either.

@gkellogg
Copy link
Member

At some point, the test suite, if not the SPARQL specs, need to fully align with RDF 1.1, particularly with regards to plain literals and xsd:string handling.

the point of this repo is to curate these tests with regard to the current specs, but if a SPARQL CG would like to move the specs forwards, we could/should track the changes here. It wouldn’t make sense to fork the test suite again.

@afs
Copy link
Contributor

afs commented May 16, 2021

Agreed. The test suite here should be RDF 1.1 usable - not another fork! SPARQL 1.1 (and 1.0) is written with the anticipation that simple literals (SPARQL terminology) merge xsd:string. But it is good to test for the case of RDF 1.0 written data and not see tests as a showcase of preferred usage.

@gkellogg
Copy link
Member

The JSON-LD tests have a specVersion option to allow some tests to be made specific to either JSON-LD 1.0 or 1.1; without the option, they are valid for both versions. We could consider adding such manifest annotations. Tests which require a Plain Literal and xsd:string literal to be different cause a problem for those using RDF 1.1 semantics.

@afs
Copy link
Contributor

afs commented Jan 22, 2023

@gkellogg - I'm neutral.

The situation hasn't changed. There is at least one major triple store, OpenLink Virtuoso, that provides RDF 1.0. For them, the explicit datatype is necessary. I found at least one test that has some data with and some without explicit xsd:string and this is reflected in the test results.

In the light of RDF-star (RDF 1.2) WG starting, if the rdf-tests community wants to align to RDF 1.1 then make this change (which I have checked and it seems OK) and also #83.

To check where the community opinion is, I've raised an issue to collect votes #88 because I think we need to be completely clear as to the decision going forward. Give this at least some business-days to conclude.

If the community wishes to maintain as RDF 1.1 (and soon RDF 1.2), merge this.

If the community wishes to retain the current position (roughly - on a case-by-case basis), we should not make changes that are unnecessary and close this PR unmerged.

@TallTed
Copy link
Member

TallTed commented Jan 24, 2023

The test suite here should be RDF 1.1 usable - not another fork!

I am not sure what is meant by "not another fork".

I think having branches focused on RDF 1.0, 1.1, and 1.2 (and actually, a matrix of these with SPARQL 1.0, 1.1, and 1.2) would be quite useful, for reasons discussed in #88, #83 and related.

@afs
Copy link
Contributor

afs commented Jan 25, 2023

I've tagged the repository with sparql-mixed-rdf-version-tests so that if this is merged, there is a convenient "before" marker.

@afs afs merged commit 015c01e into w3c:main Feb 5, 2023
@afs afs removed the wontfix label Feb 5, 2023
} ,
{
"s": { "type": "uri" , "value": "http://example.org/s4" } ,
"p": { "type": "uri" , "value": "http://example.org/p4" } ,
"o": { "datatype": "http://www.w3.org/2001/XMLSchema#integer" , "type": "literal" , "value": "4" }
"o": { "type": "literal" , "value": "4" }
Copy link
Contributor

@Tpt Tpt Feb 5, 2023

Choose a reason for hiding this comment

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

This introduces the implicit rule "if a literal value is a JSON number that looks like a xsd:integer, then it should be parsed as a xsd:integer literal". Is it intended? It is not specified anywhere at my knowledge.

If we want to introduce it, it might also be nice to add more tests to cover more cases (boolean, the numbers 0.1 (xsd:decimal?) 0.1e3 (xsd:double?), 0. (??)...

Copy link
Contributor

Choose a reason for hiding this comment

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

No, that's not intended. Thank you for catching that.

I missed it because json-res isn't run by manifest-sparq11-query.

csv-tsv-res/csvtsv02.tsv is damaged as well.

manifest-all contains remote tests, which need special setup and the entailment tests.

I wonder if it would be useful to have manifest.ttl that is the "all local" part -- query, update and the result set formats.

Copy link
Contributor

Choose a reason for hiding this comment

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

A manifest.ttl with result set formats but not entailment tests seems a bit arbitrary to me but why not.

Copy link
Contributor

@afs afs Feb 6, 2023

Choose a reason for hiding this comment

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

The entailment tests includes ones with complex OWL reasoning.

The idea of manifest.ttl is a single place for the majority of triplestores.

Or manifest-results.ttl.
Or both!

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. +1 to manifest.ttl or something like manifest-with-results.ttl.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll add it to the reorg.
I'm currently minded to have manifest-results and manifest includes -query,, -update and -results.

This was referenced Feb 6, 2023
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.

5 participants