Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Aggregators Min and Max call Ordering file #180

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

jitsedesmet
Copy link
Member

@jitsedesmet jitsedesmet commented Jul 8, 2023

This PR aims to resolve comunica/comunica#1212.
According to the spec, Min and Max should make use of the SPARQL ORDER BY ordering definition. I assume sparqlee implements this in Ordering.ts (Both the spec and the implementation are wild to me, so please correct me if this assumption is wrong).

The issue also makes me wonder whether there are problems regarding date (and similar types) within the other aggregators, since all aggregators call the typedValue of a literal. Before #163 this was always some basic JS type (if I recall correctly), but now there are also object typedValues, these do not implement the + operator for example. The other aggregators do call the regularFunctions so should work.

@coveralls
Copy link

coveralls commented Jul 8, 2023

Coverage Status

coverage: 100.0%. remained the same when pulling de4cc48 on jitsedesmet:fix/min-of-date into 6c22f37 on comunica:master.

@jitsedesmet
Copy link
Member Author

This seems to resolve the problem.
Query:

SELECT (MIN(?of) as ?min) (MAX(?of) as ?max)
where {
    ?p ?o ?of.
}

over:

<http://example.com/apple> <http://www.w3.org/2001/XMLSchema#datePred> "2010-06-21+08:00"^^<http://www.w3.org/2001/XMLSchema#date>.
<http://example.com/pear> <http://www.w3.org/2001/XMLSchema#datePred> "2010-06-21-08:00"^^<http://www.w3.org/2001/XMLSchema#date>.

gives:

[
{"max":"\"2010-06-21-08:00\"^^http://www.w3.org/2001/XMLSchema#date","min":"\"2010-06-21+08:00\"^^http://www.w3.org/2001/XMLSchema#date"}
]

@jitsedesmet jitsedesmet marked this pull request as ready for review July 8, 2023 15:15
Copy link
Member

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

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

Other than the single comment, this looks good to me!

lib/util/Ordering.ts Outdated Show resolved Hide resolved
@rubensworks rubensworks merged commit 3bf0950 into comunica:master Aug 8, 2023
6 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agregates with xsd:date objects do not work
3 participants