Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transaction numeric, string, datetime, and hash functions #505

Merged
merged 11 commits into from
Jul 12, 2023

Conversation

dpetran
Copy link
Contributor

@dpetran dpetran commented Jun 14, 2023

This is the easy part of #232

implements numeric, string, datetime, and hash functions

https://www.w3.org/TR/sparql11-query/#SparqlOps :17.4.3, 17.4.4, 17.4.5, 17.4.6

Also includes a fix for #522

These had existing implementations that needed to be changed to comply:

  • rand:
    before it selected a random element from a collection, now returns a random
    number between 0 and 1.
  • now:
    before it returned an integer epoch milliseconds, now returns an ISO8601 datetime string
  • subStr
    before it took a start and optionally an end arg, now it takes a start and optionally a
    length arg

These haven't been implemented:

  • md5, sha1, sha386:
    We don't have implementations for these and I didn't want to add a dependency for
    them. Figure we can add later at user request.
  • encode-for-uri:
    This seems more complex and I want to see if there's a straightforward way to get it
    without manually implementing https://www.ietf.org/rfc/rfc3986.txt
  • langMatches
    We don't currently support language tags

Outstanding work:
The datetime functions currently only work on datetime strings - not date strings, not time strings.

The functional forms (17.4.1) and functions on RDF terms (17.4.2) sections are a bit trickier. A lot of it already is implemented but the stuff that isn't requires additional context beyond just the value (subject, predicate, object).

Also, error handling is really subpar currently, I'd like to have very nice and helpful error messages when a function blows up.

@dpetran dpetran requested a review from a team June 14, 2023 16:34
[s start end]
(subs s start end))
([s start]
(subStr s start (count s)))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is confounding length and endpoint. If start is not 1, the second argument to subStr will be longer that the length remaining. In any case, the built-in subs has an arity that accomplishes this case. i think (subs s (dec start)) is what you want here.

[s substr]
(-> (str/split s (re-pattern substr))
first
str))
Copy link
Contributor

Choose a reason for hiding this comment

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

why the extra str call here? Also, I don't think this matches the spec. If substr doesn't appear in s, the spec says to return an empty literal, but this will return s.


(defn strAfter
[s substr]
(-> (str/split s (re-pattern substr))
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like this matches the spec either. First, for the same reason as strBefore above, and also because the spec says to return the string after the first occurrence of substr, and this will return the string after the last occurrence of substr

ledger @(fluree/create conn "functions" {:defaultContext [test-utils/default-str-context
{"ex" "http://example.com/"}]})
db0 (fluree/db ledger)
db1 @(fluree/stage db0 [{"id" "ex:create-predicates"
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this db1 step do? It looks like you're staging all the data you need for each test below, and we should be creating predicates on the fly as data is staged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's an artifact of how I originally wrote the test harnesses, I can move the predicate creation into the relevant test cases.

dpetran added 10 commits July 5, 2023 15:05
https://www.w3.org/TR/sparql11-query/#SparqlOps
17.4.3
17.4.4
17.4.5
17.4.6

These had existing implementations that needed to be changed to comply:
- rand:
before it selected a random element from a collection, now returns a random
number between 0 and 1.
- now:
before it returned an integer epoch milliseconds, now returns an ISO8601 datetime string
- subStr
before it took a start and optionally an end arg, now it takes a start and optionally a
length arg

These haven't been implemented:
- md5, sha1, sha386:
We don't have implementations for these and I didn't want to add a dependency for
them. Figure we can add later at user request.
- encode-for-uri:
This seems more complex and I want to see if there's a straightforward way to get it
without manually implementing https://www.ietf.org/rfc/rfc3986.txt
- langMatches
We don't currently support language tags
These are the functions that can be implemented without altering our existing query fn
structure.
@dpetran
Copy link
Contributor Author

dpetran commented Jul 5, 2023

rebased on main to remove file conflict


(defn year
[datetime-string]
(let [datetime ^LocalDateTime (datatype/coerce datetime-string const/$xsd:dateTime)]
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if we try to do this in javascript?

Also shuffled around some type hints. They didn't cause any problems where they were,
but they were misleading.

Also, `getYear` does not return what you'd expect in js, we need to rely on
`getFullYear` instead.
Copy link
Contributor

@zonotope zonotope left a comment

Choose a reason for hiding this comment

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

🌵

@dpetran dpetran merged commit 205442e into main Jul 12, 2023
5 checks passed
@dpetran dpetran deleted the feature/transaction-fns branch July 12, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants