Fix bug min/max of non-numerical type, Closes #673

comunica / sparqlee

⚙️ SPARQL expression evaluator library - Moved to @comunica/expression-evaluator

https://www.npmjs.com/package/sparqlee

14 stars 7 forks source link

Fix bug min/max of non-numerical type, Closes #673 #71

Closed stephaniech97 closed 4 years ago

stephaniech97 commented 4 years ago

https://github.com/comunica/comunica/issues/673

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 391

23 of 23 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 79.775%

Totals
Change from base Build 381:	0.2%
Covered Lines:	1068
Relevant Lines:	1288

💛 - Coveralls

rubensworks commented 4 years ago

Did you check what the intended behaviour in the SPARQL spec is? Does this PR conform to this?

stephaniech97 commented 4 years ago

https://en.wikibooks.org/wiki/SPARQL/Expressions_and_Functions#COUNT,_MIN,_MAX,_AVG_and_SUM Here was mentioned that "The functions COUNT, MIN, MAX, AVG, and SUM can only be used as Aggregate functions." So when further going into the aggregate functions "https://en.wikibooks.org/wiki/SPARQL/Aggregate_functions" they defined it in the following way "MIN, MAX: the minimum or maximum value of all elements, respectively. This works for all value types; numbers are sorted numerically, strings and other types lexically." This is why I chose to accept all types. If different types however are compared, i chose to throw an error.

rubensworks commented 4 years ago

If different types however are compared, i chose to throw an error.

The "chose" in this sentence makes me suspect that it's not spec-compliant. Can you find any mentions in the spec on what should happen when different types are compared? Maybe you can try with other SPARQL engines to see what happens there?