Add built-in functions by aannleax · Pull Request #781 · knowsys/nemo

aannleax · 2026-04-08T14:38:56Z

This PR adds new built-in functions. Closes #642 (I moved aggregate functions to a separate issue #782) and closes #769.

String functions

REPLACE(str, pattern, replacement[, flags]) : regex-based string replacement
langMatches(tag, range): language tag range matching per SPARQL/RFC 4647
STRTRIM(str): remove leading and trailing whitespace
STRTRIMSTART(str): remove leading whitespace only
STRTRIMEND(str): remove trailing whitespace only
STRDT(lex, datatype): construct a typed literal from a lexical value and datatype IRI
Hashing functions: MD5, SHA1, SHA256, SHA384, SHA512

Date/time functions

YEAR, MONTH, DAY, HOURS, MINUTES, SECONDS: component extraction from xsd:dateTime, xsd:date, and xsd:time values
TIMEZONE: timezone as xsd:dayTimeDuration
TZ: timezone as a plain string

Nondeterministic functions

RAND(): pseudo-random double in [0, 1)
UUID() : fresh IRI-form UUID
STRUUID() : fresh string-form UUID
NOW() : current date/time as xsd:dateTime

Changes to existing functions

REGEX now accepts flags as an optional third parameter

mmarx

I've not yet reviewed everything, but I don't think this is ready to merge without a major overhaul. There's several issues both with respect to code quality and with respect to correctness:

we're pulling in chrono for handling timestamps, but then re-implement the timestamp parsing ourselves – why?
I'm not impressed with the code quality – there's functions that are defined identically in multiple files (and then inlined in other places as well), one-line functions that are only called once, redundant function calls like Some(value.to_plain_string_unchecked()) which could be a simple value.to_plain_string() instead, …
the hash functions are implemented for plain strings and language tagged strings, yet the spec says that they take plain strings and simple literals
the nondeterministic functions take an arbitrary number of arguments, even though they really should not take any arguments at all

mkroetzsch · 2026-04-09T06:23:52Z

Since "simple literals" came up in the discussion: These should not be implemented as a separate type but identified with xsd:string. This is a relic from the depth of RDF history that is vanishing as specs get updated.

mmarx · 2026-04-09T17:57:44Z

+/// An optional third parameter may provide regex flags (e.g. `"i"` for case-insensitive),
+/// corresponding to the SPARQL `regex(string, pattern [, flags])` function.


This also doesn't validate the flags, and furthermore doesn't support the q flag.

mmarx · 2026-04-09T18:01:00Z

+/// Returns a language tagged string if the first parameter has a language tag.
+/// Otherwise, return a plain string.


It would be nice if we could keep the grammar consistent and not use returns and return in the same paragraph.

This still says “returns …, otherwise return …”.

mmarx · 2026-04-09T18:03:37Z

+    #[test]
+    fn test_string_trim() {
+        let string = AnyDataValue::new_plain_string("  hello  ".to_string());
+        let result = AnyDataValue::new_plain_string("hello".to_string());
+        let actual_result = StringTrim.evaluate(string.clone());
+        assert!(actual_result.is_some());
+        assert_eq!(result, actual_result.unwrap());
+
+        let string_notstring = AnyDataValue::new_integer_from_i64(1);
+        let actual_result_notstring = StringTrim.evaluate(string_notstring);
+        assert!(actual_result_notstring.is_none());
+
+        let string_lang =
+            AnyDataValue::new_language_tagged_string("  hola  ".to_string(), "es".to_string());
+        let result_lang =
+            AnyDataValue::new_language_tagged_string("hola".to_string(), "es".to_string());
+        let actual_result_lang = StringTrim.evaluate(string_lang);
+        assert!(actual_result_lang.is_some());
+        assert_eq!(result_lang, actual_result_lang.unwrap());
+    }
+
+    #[test]
+    fn test_string_trim_start() {
+        let string = AnyDataValue::new_plain_string("  hello  ".to_string());
+        let result = AnyDataValue::new_plain_string("hello  ".to_string());
+        let actual_result = StringTrimStart.evaluate(string.clone());
+        assert!(actual_result.is_some());
+        assert_eq!(result, actual_result.unwrap());
+    }
+
+    #[test]
+    fn test_string_trim_end() {
+        let string = AnyDataValue::new_plain_string("  hello  ".to_string());
+        let result = AnyDataValue::new_plain_string("  hello".to_string());
+        let actual_result = StringTrimEnd.evaluate(string.clone());
+        assert!(actual_result.is_some());
+        assert_eq!(result, actual_result.unwrap());
+    }
+


This is essentially testing std functionality (except, perhaps, that it returns None on non-strings, but then we only do that for trim and not for the others). Why?

This is still just an integration test disguised as a unit test checking for correct behaviour of the Rust standard library, and it's obsoleted by an actual integration test that verifies this end-to-end. There is no situation where these tests fail, but the integration tests don't.

As stated above, there might be some value verifying that these return None on non-string inputs, but we're not doing that consistently either.

mmarx · 2026-04-09T18:05:58Z

+        // With flags: case-insensitive match
+        let string_flags = AnyDataValue::new_plain_string("Hello".to_string());
+        let pattern_flags = AnyDataValue::new_plain_string("hello".to_string());
+        let flags = AnyDataValue::new_plain_string("i".to_string());
+        let result_flags = AnyDataValue::new_boolean(true);
+        let actual_result_flags = StringRegex.evaluate(&[string_flags, pattern_flags, flags]);
+        assert!(actual_result_flags.is_some());
+        assert_eq!(result_flags, actual_result_flags.unwrap());
    }


i is the most boring flag to test here. What we should have test for are the q flag (since that requires special handling), though that should rather be an integration test, backreferences in pattern (again an integration test), and that invalid flags lead to None.

The only checks that are appropriate here are for the cases where we return None. Everything else should be an integration test and only an integration test.

mmarx · 2026-04-14T13:52:50Z

We really need to push the nondeterministic functions into SPARQL queries. Consider the following program:

@import nemo :- sparql{ endpoint = <https://query.wikidata.org/sparql>, query = "SELECT ?x ?y WHERE { BIND(?x AS ?y) }"} .
@import sparql :- sparql{ endpoint = <https://query.wikidata.org/sparql>, query = "SELECT ?x WHERE { BIND(RAND() AS ?x) }"} .

foo(?x) :- nemo(RAND(), ?x) .
bar(?x) :- sparql(?x) .

bar correctly turns into a single SPARQL query, whereas foo will just produce an endless list of queries.

mmarx

This is still quite a bit from being ready. Most of the issues are just useless comments and useless “unit” tests (that are secretly integration tests, covering parts of what is already covered by proper integration tests), but there's also quite a bit of leaking implementation details (partly cross-crate) into public API docs, lying about what's in the XPath spec, a function as part of the public API that serves no purpose except breaking the invariant that every call to NOW() returns the same value, general silliness like !is_nondeterministic when is_deterministic suffices, and comments from my previous review that have been left unaddressed.

Curiously, the commit messages also seem to not match what's actually in the commits: 1bb98d3, e.g., is titled “Ask regex cache earlier”, and, while it does touch the regex cache, it absolutely does not change when it is queried (which still happens too late). It also adds a lot of documentation to other functions, as well as change the behaviour of language tag matching.

mmarx · 2026-07-31T21:18:31Z

+///
+/// Lazily initialized with the current time on first access


This comment just restates what the code already says. Get rid of it.

mmarx · 2026-07-31T21:22:27Z

+/// Capture the current time as the timestamp returned by `NOW()`.
+///
+/// This should be called once at the start of each program execution.
+/// All evaluations of `NOW()` during the execution will return this fixed value.
+///
+/// Note that the timestamp is global to the process.
+pub fn set_now_timestamp() {
+    *NOW_TIMESTAMP
+        .write()
+        .expect("no thread should panic while holding the lock") = DateTime::now().to_string();
+}


Not only do we not need this, it's now offering a public API to break the precise API contract the documentation claims (that each evaluation of NOW() returns the same value), by allowing the timestamp to change when calling set_now_timestamp multiple during reasoning.

mmarx · 2026-07-31T21:26:47Z

+        let year = if datatype == XSD_DATETIME {
+            DateTime::from_str(&lexical).ok()?.year()
+        } else if datatype == XSD_DATE {
+            Date::from_str(&lexical).ok()?.year()
+        } else {
+            return None;
+        };


These are still not match statements, despite the (still open) request below?

mmarx · 2026-07-31T21:29:57Z

+/// Returns the timestamp captured by [set_now_timestamp], so all evaluations
+/// during one program execution share the same value.


As mentioned above, this is not actually true, because the API doesn't really ensure that set_now_timestamp gets called at most once (even though it need not be called in any case to uphold this guarantee). It's also stating a non-local implementation detail as part of the public documentation of this function, which is just bound to become outdated.

mmarx · 2026-07-31T21:32:31Z

+/// Returns a value of the form `<urn:uuid:…>` containing a version 7 UUID,
+/// which is lexicographically greater than all UUIDs generated before it.


The only guarantee we want to make as part of the public API documentation is that this returns a fresh UUID. That it's a v7 UUID is an implementation detail that we might want to change at a later point, certainly we don't want to document it here.

mmarx · 2026-07-31T22:51:23Z

+/// references to capture groups that do not exist in the pattern
+/// expand to the empty string instead of being an error,
+/// and patterns that match a zero-length string are permitted
+/// (where XPath raises an error).


References to non-existing capture groups are not an error in the XPath specification (there is still a difference, though, in that XPath takes the longest possible capture group reference that exists (e.g., $23 can refer to group 2 if that exists, with the 3 taken as a literal part of the replacement value), whereas fancy_regex always takes the longest name), but if that doesn't apply, the replacement absolutely is the empty string.

mmarx · 2026-07-31T22:55:02Z

+/// Corresponds to SPARQL langMatches(language-tag, language-range).
+/// Returns `true` if `language-tag` matches `language-range` per RFC 4647 basic filtering:


Why do we suddenly use kebab-case for language-tag and language-range? Just call them tag and range.

mmarx · 2026-07-31T22:59:20Z

+/// Corresponds to SPARQL langMatches(language-tag, language-range).
+/// Returns `true` if `language-tag` matches `language-range` per RFC 4647 basic filtering:


This should also refer to BCP 47, not to RFC 4647 (which just happens to be the current version right now).

mmarx · 2026-07-31T23:07:12Z

+    #[test]
+    fn test_string_trim() {
+        let string = AnyDataValue::new_plain_string("  hello  ".to_string());
+        let result = AnyDataValue::new_plain_string("hello".to_string());
+        let actual_result = StringTrim.evaluate(string.clone());
+        assert!(actual_result.is_some());
+        assert_eq!(result, actual_result.unwrap());
+
+        let string_notstring = AnyDataValue::new_integer_from_i64(1);
+        let actual_result_notstring = StringTrim.evaluate(string_notstring);
+        assert!(actual_result_notstring.is_none());
+
+        let string_lang =
+            AnyDataValue::new_language_tagged_string("  hola  ".to_string(), "es".to_string());
+        let result_lang =
+            AnyDataValue::new_language_tagged_string("hola".to_string(), "es".to_string());
+        let actual_result_lang = StringTrim.evaluate(string_lang);
+        assert!(actual_result_lang.is_some());
+        assert_eq!(result_lang, actual_result_lang.unwrap());
+    }
+
+    #[test]
+    fn test_string_trim_start() {
+        let string = AnyDataValue::new_plain_string("  hello  ".to_string());
+        let result = AnyDataValue::new_plain_string("hello  ".to_string());
+        let actual_result = StringTrimStart.evaluate(string.clone());
+        assert!(actual_result.is_some());
+        assert_eq!(result, actual_result.unwrap());
+    }
+
+    #[test]
+    fn test_string_trim_end() {
+        let string = AnyDataValue::new_plain_string("  hello  ".to_string());
+        let result = AnyDataValue::new_plain_string("  hello".to_string());
+        let actual_result = StringTrimEnd.evaluate(string.clone());
+        assert!(actual_result.is_some());
+        assert_eq!(result, actual_result.unwrap());
+    }
+


This is still just an integration test disguised as a unit test checking for correct behaviour of the Rust standard library, and it's obsoleted by an actual integration test that verifies this end-to-end. There is no situation where these tests fail, but the integration tests don't.

As stated above, there might be some value verifying that these return None on non-string inputs, but we're not doing that consistently either.

mmarx · 2026-07-31T23:10:08Z

+        // With flags: case-insensitive match
+        let string_flags = AnyDataValue::new_plain_string("Hello".to_string());
+        let pattern_flags = AnyDataValue::new_plain_string("hello".to_string());
+        let flags = AnyDataValue::new_plain_string("i".to_string());
+        let result_flags = AnyDataValue::new_boolean(true);
+        let actual_result_flags = StringRegex.evaluate(&[string_flags, pattern_flags, flags]);
+        assert!(actual_result_flags.is_some());
+        assert_eq!(result_flags, actual_result_flags.unwrap());
    }


The only checks that are appropriate here are for the cases where we return None. Everything else should be an integration test and only an integration test.

mmarx · 2026-07-31T23:59:13Z

-static NOW_TIMESTAMP: RwLock<Option<String>> = RwLock::new(None);
+/// Lazily initialized with the current time on first access
+static NOW_TIMESTAMP: LazyLock<RwLock<String>> =
+    LazyLock::new(|| RwLock::new(DateTime::now().to_string()));


There's no need to ever set this after it's been set, so this should just be LazyLock<String>.

Not sure, what if we allow reasoning to be performed multiple times...

github-project-automation Bot added this to nemo Apr 8, 2026

github-project-automation Bot moved this to Todo in nemo Apr 8, 2026

aannleax added the builtins Issue related to built-in functions label Apr 8, 2026

aannleax added this to the Release 0.10.0 milestone Apr 8, 2026

aannleax requested a review from mmarx April 8, 2026 14:40

mmarx requested changes Apr 8, 2026

View reviewed changes

github-project-automation Bot moved this from Todo to In Progress in nemo Apr 8, 2026

aannleax requested a review from mmarx April 9, 2026 08:50

mmarx requested changes Apr 9, 2026

View reviewed changes

mmarx modified the milestones: Release 0.10.0, Release 0.11.0 Apr 13, 2026

mmarx reviewed Apr 14, 2026

View reviewed changes

Comment thread nemo-physical/src/function/evaluation.rs Outdated

mmarx reviewed Apr 15, 2026

View reviewed changes

Comment thread nemo-physical/src/function/definitions/nondeterministic.rs Outdated

Comment thread nemo-physical/src/function/definitions/nondeterministic.rs Outdated

aannleax added 15 commits July 14, 2026 16:26

add replace, langMatches, and hash functions

e92e5f3

add strdt

86096f0

add datetime functions

dc6c273

cargo fmt

ee53e2a

restore functions

eb53eb3

add nondeterministic functions

fa8d328

add trim

3c10765

adjust tokio dependency to allow tests to be run on nemo-physical

21507bf

cargo fmt

f7ab3de

cargo clippy

58c1ea6

add optional flag parameter for REGEX builtin

f31b64f

add feature flag for uuid

b72391e

fix uuid on wasm

5206acb

review comments

34a7345

cargo fmt

685d83c

aannleax added 12 commits July 14, 2026 16:28

add nullary function case

5d3ff9b

Add DataValue::to_other method

b337c44

Rename function name langMatches to LANGMATCHES

cb7a2d1

Implement hashing generically

7f6e04a

remove unused chrono dependency and set uuid from v4 to v7

966d12a

remove unused dependencies; update regex to fancy_regex

6db49b6

Remove chrono

cb09d33

Remove chrono and chnage regex to fancy-regex

d12beb7

adjust implementation of new built-in functions

6b52799

Recompute non-deterministic functions on every row

6a7bb5c

Set now timestamp during initialization

7b14484

Update test cases for new built-ins

3a551ea

aannleax force-pushed the feature/add-builtins branch from e4a29f9 to 3a551ea Compare July 14, 2026 14:32

aannleax added 9 commits July 14, 2026 20:17

Enable oxsdatatypes js feature for wasm

08ba3a3

Move uuid

4d696b4

Lazily initilialize NOW_TIMESTAMP

f80f813

Fix nondeterministic functions being recomputed multiple times

1daf671

Fewer allocations when hashing

a07e9a4

Ask regex cache earlier

1bb98d3

Remove logging from dictionary len method

1975949

Remove RAND test

4645c5b

remove some unit tests that were also validated as integration tests

b1cee22

mmarx requested changes Jul 31, 2026

View reviewed changes

mmarx reviewed Jul 31, 2026

View reviewed changes

		/// An optional third parameter may provide regex flags (e.g. `"i"` for case-insensitive),
		/// corresponding to the SPARQL `regex(string, pattern [, flags])` function.

		/// Returns a language tagged string if the first parameter has a language tag.
		/// Otherwise, return a plain string.

		///
		/// Lazily initialized with the current time on first access

		/// Returns the timestamp captured by [set_now_timestamp], so all evaluations
		/// during one program execution share the same value.

		/// Returns a value of the form `<urn:uuid:…>` containing a version 7 UUID,
		/// which is lexicographically greater than all UUIDs generated before it.

		/// Corresponds to SPARQL langMatches(language-tag, language-range).
		/// Returns `true` if `language-tag` matches `language-range` per RFC 4647 basic filtering:

Uh oh!

Conversation

aannleax commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmarx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkroetzsch commented Apr 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mmarx commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmarx left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aannleax commented Apr 8, 2026 •

edited

Loading

mmarx commented Apr 14, 2026 •

edited

Loading

mmarx left a comment •

edited

Loading