Implement cast expressions correctly #1751
Conversation
There was a problem hiding this comment.
I have mostly three comments
- Tests missing
- The failure of the
xsd:stringtest case is not yours, but a bug in the parser - We have to analyze and then decide on the behavior for
xsd:double(3E4)andxsd::boolean(3)andxsd:boolean(3.3)(the first being relevant for this PR)
| return Id::makeFromBool(std::get<int64_t>(value) != 0); | ||
| } else if (std::holds_alternative<double>(value)) { | ||
| return Id::makeFromBool(std::get<double>(value) != 0); | ||
| } else { |
There was a problem hiding this comment.
I will have to check again whether that is the correct/intended behavior.
| using ToDouble = NARY<1, FV<ToNumericImpl<double>, ToNumericValueGetter>>; | ||
| using ToDecimal = | ||
| NARY<1, FV<ToNumericImpl<double, false>, ToNumericValueGetter>>; |
There was a problem hiding this comment.
Have you verified, that this is indeed a subtle difference between double and decimal?
Even if, I am still not sure whether it currently is a good idea to treat them differently in this single function, when we implement them the same in all other cases.
There was a problem hiding this comment.
I compared the SPARQL grammar for decimal and yes, this is the difference:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1751 +/- ##
==========================================
+ Coverage 89.97% 89.99% +0.01%
==========================================
Files 395 395
Lines 37667 37743 +76
Branches 4236 4250 +14
==========================================
+ Hits 33891 33965 +74
- Misses 2478 2479 +1
- Partials 1298 1299 +1 ☔ View full report in Codecov by Sentry. |
test/SparqlExpressionTest.cpp
Outdated
| // These test cases should technically not result in undefined, but | ||
| // they are parsed into actual float values, so the corresponding | ||
| // SPARQL compliance test cases pass regardless. | ||
| lit("0E1", absl::StrCat("^^<", XSD_PREFIX.second, "float>")), | ||
| lit("1E0", absl::StrCat("^^<", XSD_PREFIX.second, "float>")), | ||
| lit("1.25", absl::StrCat("^^<", XSD_PREFIX.second, "float>")), | ||
| lit("-7.875", absl::StrCat("^^<", XSD_PREFIX.second, "float>"))}), |
There was a problem hiding this comment.
I don't understand the comment. Do you mean "the compliance tests suggest, that no floating point value (even if it is exactly zero or "exactly" one can be converted to xsd::boolean directly?
Conformance check passed ✅Test Status Changes 📊
|
|
Add the cast expressions
xsd:boolean(?something)andxsd:string(?something).Also add
xsd:floatwhich just does the same asxsd:doubleas QLever currently doesn't distinguish between these operations.Also support exponential notation like "1E04" for the
xsd:doublecast.