Conversation
… into DateTimeSubtraction
Overview
Conformance check passed ✅No test result changes. |
|
ullingerc
left a comment
ullingerc
left a comment
There was a problem hiding this comment.
Thank you very much. This will be a very useful addition to QLever.
And it is already quite nice. Let's iterate the details so we can get this PR on track for merge.
src/util/DateYearDuration.cpp
Outdated
| } | ||
|
|
||
| // no viable subtraction | ||
| throw std::invalid_argument("No Subtraction possible!"); |
There was a problem hiding this comment.
Which subtractions between dates are not possible?
We should implement them also for Year, ...
So ideally this throw will not be necessary any more. In any case, a SPARQL function should not throw on invalid input, but return UNDEF instead.
There was a problem hiding this comment.
I would say that Duration - Date does not make sense (probably same for Duration - Year).
Other than that Date - Date, Date - Duration, Duration - Duration and Year - Year makes sense and should be implemented.
| 60 - | ||
| (minute2 + (secondsPassed > 0.0 | ||
| ? 1 | ||
| : 0)); // we add 1 because the seconds added a minute |
There was a problem hiding this comment.
Can you explain the reason to add 1 here ?
There was a problem hiding this comment.
Here the time left until the next day is calculated. For example take the time 05:35:30 : I started with the seconds and saved the amount of seconds needed to get to the next minute (here 30s). But with this the time changed to 5:36:00 , therefore when calculating the amount of minutes needed to get to the next hour (here 06) I added 1 minute to get from 05:35 to 5:36. 0 is only added if 60.0 - seconds <= 0.0 (for example 60.0 - 60.0). The same is then done for the minutes when calculating the hoursPassed.
| updatePassedTimes(otherDate, ownDate, daysPassed, hoursPassed, | ||
| minutesPassed, secondsPassed); | ||
| } | ||
| return DateYearOrDuration(DayTimeDuration(DayTimeDuration::Type::Positive, |
There was a problem hiding this comment.
I'm not sure about this, Shouldn't the duration be negative if I subtract an later date from a earlier one like 1. Jan 2005 - 1. Jan 2025 should be -20 years?
There was a problem hiding this comment.
Maybe you can look up how other engines that implement that.
The standard draft (https://github.com/w3c/sparql-dev/blob/main/SEP/SEP-0002/sep-0002.md) relies on XPath, which doesn't specify the negative case (https://www.w3.org/TR/xpath-functions/#func-subtract-dateTimes). And the examples for the SEP0002 are also not helpful unfortunately (https://github.com/kasei/sparql-12/blob/xsd_datetime_duration/tests/xsd_functions/dateTime_subtract-01.rq)
Co-authored-by: C. Ullinger <[email protected]>
Co-authored-by: C. Ullinger <[email protected]>
Co-authored-by: C. Ullinger <[email protected]>
2 participants
TODOS: