fix: SPARQL LOAD ... INTO GRAPH handling by aucampia · Pull Request #2554 · RDFLib/rdflib (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation10 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Summary of changes

LOAD ... INTO GRAPH stopped working correctly after the change to handling of the publicID Graph.parse parameter in RDFLib 7.0.0 (#2406).

This is because LOAD evaluation relied on publicID to select the graph name. So after #2406 data would be loaded into the default graph even if a named graph is specified.

This change adds tests for LOAD ... INTO GRAPH and fixes the load evaluation.

Checklist

• Checked that there aren't other open pull requests for
  the same change.
• Checked that all tests and type checking passes.
• Considered granting push permissions to the PR branch,
  so maintainers can fix minor issues and keep your PR up to date.

A consequence of this change is also that relative IRI lookup for graphs
loaded with LOAD ... INTO GRAPH is now relative to the source document
URI instead of the base URI of the graph being loaded into, which is
more correct I think.

I will add a test for this also tomorrow to confirm.

Also seems like windows does not like file URIs, I will look into that tomorrow also.

Also seems like windows does not like file URIs, I will look into that tomorrow also.

Okay nevermind, tests pass on windows now, I just forgot to use URIs.

coverage: 90.935% (+0.01%) from 90.925% when pulling 2ce064b on aucampia:aucampia/20230828T2212-fix-insert-into-named into dfe0c21 on RDFLib:main.

Reading the commits the changes make sense, and in line with what I observed yesterday when my LOADs ended up in wrong named graphs. Haven't tried the code yet, though.

LOAD ... INTO GRAPH stopped working correctly after the change to handling of the publicID Graph.parse parameter in RDFLib 7.0.0 (<RDFLib#2406>).

This is because LOAD evaluation relied on publicID to select the graph name. So after <RDFLib#2406> data would be loaded into the default graph even if a named graph is specified.

This change adds tests for LOAD ... INTO GRAPH and fixes the load evaluation.

A consequence of this change is also that relative IRI lookup for graphs loaded with LOAD ... INTO GRAPH is now relative to the source document URI instead of the base URI of the graph being loaded into, which is more correct.

The PR has changes to the publicID that is used, which affects relative URI resolution, so that should also be tested.

A consequence of this change is also that relative IRI lookup for graphs loaded with LOAD ... INTO GRAPH is now relative to the source document URI instead of the base URI of the graph being loaded into, which is more correct I think.

I will add a test for this also tomorrow to confirm.

Done now.

I will merge this tomorrow if there is no futher feedback.

Labels

ready to merge

The PR will be merged soon if no further feedback is provided.

review wanted

This indicates that the PR is ready for review