search

SWI-Prolog / packages-semweb

The SWI-Prolog RDF store
28 stars 14 forks source link

FIXED: Some instantiations of rdf/3 give incorrect results #78 #95

Closed likelion closed 4 years ago

JanWielemaker commented 4 years ago

Thanks. I think it can be simpler though. I propose https://github.com/SWI-Prolog/packages-semweb/commit/4e3002daf8b85932bd5304a400c066e3b0a776f3

Not that this gives problems if one would like to use true or false as a URI, but that is already ambiguous (and not valid RDF anyway). The rational is that if the first argument is an atom it is a URI and all the complicated matching is not needed.

Do you agree this fixes the issue too?

likelion commented 4 years ago

Do you agree this fixes the issue too?

I believe this fixes the issue too! Thanks!

likelion commented 4 years ago

However, my PR also properly handles true/false, and saves some cycles, when processing ground URI objects while preserving existing semantics. One thing I noticed, literal_class/2 applies only to cases Literal@Lang although the documentation states that it should classify typed literal as well, so I optimized it out in pre_object(Val^^Type, ... clause for now.

I have no strong preference w.r.t. which of our fixes to apply, you decide.

likelion commented 4 years ago

If you decide to go for your fix with post_object/2, I suggest to move the third clause of pre_object/2:

pre_object(Atom, URI) :-
    atom(Atom),
    \+ boolean(Atom),
    !,
    URI = Atom.

to the first place, as atom(Atom) excludes both literal_condition/2 and literal_class/2 thus saving cycles when checking ground URI objects.

JanWielemaker commented 4 years ago

Convinced. Thanks!