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

check cardinality of ORDER BY Criterion in extended search #642

Open
tobiasschweizer opened this issue Oct 23, 2017 · 7 comments
Open

check cardinality of ORDER BY Criterion in extended search #642

tobiasschweizer opened this issue Oct 23, 2017 · 7 comments
Assignees
Labels
bug something isn't working enhancement improve existing code or new feature

Comments

@tobiasschweizer
Copy link
Contributor

If the ORDER BY criterion is a property whose occurrence/cardinality is not 1, then the extended search logic does not work correctly.

The ORDER BY criterion has to be included in a GROUP BY statement, returning more than one row if property occurs more than once

-> only accept a value as an ORDER BY criterion if the property has a cardinality of exactly 1.

@tobiasschweizer tobiasschweizer added bug something isn't working enhancement improve existing code or new feature labels Oct 23, 2017
@tobiasschweizer tobiasschweizer self-assigned this Oct 23, 2017
@tobiasschweizer
Copy link
Contributor Author

also the statement binding the ORDER BY criterion must not be contained in an OPTIONAL or UNION. This makes sure that it is actually bound.

@tobiasschweizer
Copy link
Contributor Author

@benjamingeer Would you like to do that in the type checker?

@tobiasschweizer
Copy link
Contributor Author

tobiasschweizer commented Feb 5, 2018

@SepidehAlassi I think we would have to make beol:creationDate required (occurrence 1) to be able to support sorting of letters by date.

In case we do not have a date, we could put the whole century as a date. Do you think that would be ok?

@tobiasschweizer
Copy link
Contributor Author

On the other hand, if an optional property is not inside an OPTIONAL or a UNION, the logic works fine. Resources, where the property is missing, are not returned though.

@benjamingeer
Copy link

If the ORDER BY criterion is a property whose occurrence/cardinality is not 1, then the extended search logic does not work correctly.

Why not?

Would you like to do that in the type checker?

The current type inspector doesn't have access to ontologies. I've been meaning to add another type inspector that would run as a second pass, querying ontologies to infer types, but that's non-trivial and I can't do it right away.

Does the search responder even have enough information to connect the ORDER BY variable with the property that points to it, and to the class that has a cardinality for that property? Looking at NonTriplestoreSpecificConstructToSelectTransformer.getOrderBy, it looks to me like we don't actually have that information. We just know the type of the variable itself, we have no idea what property or class it's associated with.

@benjamingeer
Copy link

Also, if you want the 'type' of a variable to include the cardinality of the property that points to the variable within a given class, then we will need to redesign what type inspection means. Currently, the type of a variable is just an IRI (a NonPropertyTypeInfo).

@tobiasschweizer
Copy link
Contributor Author

tobiasschweizer commented Feb 5, 2018

If the ORDER BY criterion is a property whose occurrence/cardinality is not 1, then the extended search logic does not work correctly.
Why not?

Because we expect the order by variable to be bound in the response. Otherwise an error occurs.
Also if you have multiple occurrence of the same property, the same resource is repeated.

Actually this also means that you can only use order by when you are indicating a resource class. A property for itself has no cardinality.

I think this could be solved in the responder (we could get additional information from the ontology responder).

I've been meaning to add another type inspector that would run as a second pass, querying ontologies to infer types, but that's non-trivial and I can't do it right away.

Yes, I understand that this will be complex. If I find a fix for the problem mentioned above in the responder, this won't be urgent.

@subotic subotic added this to the Backlog milestone Feb 7, 2020
@irinaschubert irinaschubert removed this from the Backlog milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working enhancement improve existing code or new feature
Projects
None yet
Development

No branches or pull requests

4 participants