Inverse property path with optional ? does not work in some cases

[damyan-ognyanov]

There is an issue with the Inverse property paths in conjunction with ? optional modifier when grouping with brackets (forced precedense) are used.

To reproduce load following data:

insert data {
    <urn:1> <urn:prop> <urn:object> .
    <urn:2> <urn:prop> <urn:mid:1> .
    <urn:mid:1> <urn:prop> <urn:object> .
    <urn:3> <urn:prop> <urn:mid:2> .
    <urn:mid:2> <urn:prop> <urn:mid:3> .
    <urn:mid:3> <urn:prop> <urn:object> .
}

This query works as expected, returning 4 results:

select * where { 
    <urn:object> ^(<urn:prop>?) ?o .
}

This query, that uses the default operator precedense, also works correctly returning 4 results

select * where { 
    <urn:object> ^<urn:prop>? ?o .
} 

This query returns correctly 3 results:

select * where { 
    <urn:object> (^<urn:prop>) ?o .
}

but when we add ? optional modifier, it is expected to return the above 3 results plus the so called self result but instead, it only return one result (self):

select * where { 
    <urn:object> (^<urn:prop>)? ?o .
}

which is unexpected.

the produced query model in the wrong case is not inversed (urn:object is on subject position and the var on object position, e.g. these are not swapped according to the ^ inverse modifier used.

Tried to find a solution/quick fix but the code around the transaltion of property paths is a bit of a mess. For simple inverse paths like the one used in the above queries the code could be patched somehow but if the path is more complex and involves either alternatives or other segments that are also inversed the number of swaps could not be deduced after the recursive step finishes converting the nested segments so it is hard to decide wheter to swap the subj/obj positions or not.

Hope this description is clear enough ...

[abrokenjester]

Tried to find a solution/quick fix but the code around the transaltion of property paths is a bit of a mess.

I quite agree - we keep monkey-patching the code, but it really needs a good overhaul.

For simple inverse paths like the one used in the above queries the code could be patched somehow but if the path is more complex and involves either alternatives or other segments that are also inversed the number of swaps could not be deduced after the recursive step finishes converting the nested segments so it is hard to decide wheter to swap the subj/obj positions or not.

I think we should look at writing a dedicated class for handling of path expressions. Or at the very least get rid of the awful if-else constructions in visit(PathSequence) and handle things properly.

[VladimirAlexiev]

Original issue: https://ontotext.atlassian.net/browse/GDB-4650, where we have a more complex case of collecting VIAF names with a more complex path like this
(^foaf:focus?)/(schema:name|skos:prefLabel|skos:altLabel|schema:alternateName)

[abrokenjester]

I'm doing a little spiking on this in my spare time, to see if I can come up with a refactor that makes sense.