support day, month and year function for date by FlorianLudwig · Pull Request #1154 · RDFLib/rdflib (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation14 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Fixes #1153
Proposed Changes
- Support day, month and year sparql function on date as well as datetime
Coverage increased (+0.01%) to 75.595% when pulling 51a17fd on FlorianLudwig:support_date_type into 652d2e6 on RDFLib:master.
Coverage decreased (-0.07%) to 75.423% when pulling 7c1d49b on FlorianLudwig:support_date_type into dde9db8 on RDFLib:master.
support day, nonth and year function for date support day, month and year function for date
According to the SPARQL 1.1 Specification (https://www.w3.org/TR/sparql11-query/#func-date-time) the builtin functions are only defined on xsd:dataTime not on xsd:date. So I'm not sure if it is a good idea to extend to to xsd:date as well. If we want to extend it we might also want to extend it to xsd:gDay, xsd:gMonth, xsd:gMonthDay, xsd:gYear, and xsd:gYearMonth. While some of the builtins would result in an empty string in some cases.
@white-gecko I am no expert on the specs, I searched through it and could not find any specification on what to do if the argument is of the wrong type (date vs dateTime) - except for the string-functions, where it is explicitly said that it should result in an error if the argument is not a string. So this might be undefined behaviour?
If you would be interested in merging this I would look into supporting the other types as well and update this MR.
Note: xsd:date (and others) are not part of sparql 1.1 at all. Interestingly the first - and so far only - written enhancement proposal for 1.2. So I guess there seems to be interest in this from others as well :)
@white-gecko to be honest i have developed a dislike for switches in software as they complicate code and testing as resulting combinations do increase exponentially. So if it does not clash with the specs I prefer to not have a switch.
I would really like to simplify the code base for sparql evaluation a bit - I am running with a few local changes which gave my use case 25% performance boost - and I would like to work on it a bit more, as it is still to slow for my use case. Those where really low hanging fruit and I am sure there is more. Introducing a switch means supporting more different combinations - and probably breaking those, making it harder to move the code forward. See #1155 for an example of this - my local refactoring broke the semantic of comparing a Literal with None - keeping those would have made the code more complex for a benefit I did see (or understand).
In an informative section the standard says.
3.3 Other Term Constraints
In addition to numeric types, SPARQL supports types xsd:string, xsd:boolean and xsd:dateTime (see Operand Data Types). Section Operator Mapping describes the operators and section Function Definitions the functions that can be that can be applied to RDF terms.
Based on this I can not judge if we should support the additional types or not.
Your argument that it might be supported in SPARQL 1.2 sounds good to me.
Anyhow I ask @ashleysommer and @nicholascar for their opinion.
I am running with a few local changes which gave my use case 25% performance boost - and I would like to work on it a bit more, as it is still to slow for my use case
That sounds great! I've had a mental note for months to go in to see where SPARQL could be made faster in RDFLib.
It is the only thing that significantly slows down execution of PySHACL. When not using any SPARQL features, the SHACL validation engine executes approx 10x faster than when SPARQL features are used.
I've resolved the incidental questions, since they have answers.
Based on this I can not judge if we should support the additional types or not.
I personally use xsd:date and Python equivalents instead of dateTime wherever I can to ensure we don't get spurious time values in data.
So I approve this PR even if it is extending beyond the SPARQL 1.1 spec!
