pchampin / sophia_rs

Sophia: a Rust toolkit for RDF and Linked Data
Other
210 stars 23 forks source link

Owning a SimpleTerm #131

Closed KonradHoeffner closed 1 year ago

KonradHoeffner commented 1 year ago

I'm trying to implement triples_matching for HDT but I keep getting cannot return value referencing function parameter errors with SimpleTerm even when I clone it or call to_owned() on it. Is there any way to convert a simple term into an owned one?

   /// Only supports constant and "any" matchers.
    /// Non-constant matchers are supposed to be "any" matchers.
    fn triples_matching<'s, S, P, O>(&'s self, sm: S, pm: P, om: O) -> GTripleSource<'s, Self>
    where
        S: TermMatcher + 's,
        P: TermMatcher + 's,
        O: TermMatcher + 's,
    {
        let xso = sm.constant().map(|s| {
            let simple = SimpleTerm::from(s.as_simple().to_owned());
            let id = self.hdt.dict.string_to_id(&term_string(&simple), &IdKind::Subject);
            (simple, id)
        });
        let xpo = pm.constant().map(|p| {
            let simple = p.as_simple();
            let id = self.hdt.dict.string_to_id(&term_string(&simple), &IdKind::Predicate);
            (simple, id)
        });
        let xoo = om.constant().map(|o| {
            let simple = o.as_simple();
            let id = self.hdt.dict.string_to_id(&term_string(&simple), &IdKind::Object);
            (simple, id)
        });
        if [&xso, &xpo, &xoo].into_iter().flatten().any(|x| x.1 == 0) {
            // at least one term does not exist in the graph
            return Box::new(iter::empty());
        }
            if [&xso, &xpo, &xoo].into_iter().flatten().any(|x| x.1 == 0) {
            // at least one term does not exist in the graph
            return Box::new(iter::empty());
        }
        match (xso, xpo, xoo) {
                 (Some(s), None, None) => {
                Box::new(SubjectIter::with_pattern(&self.hdt.triples, &TripleId::new(s.1, 0, 0)).map(move |t| {
                    Ok([
                        s.0,
                        IriRef::new_unchecked(MownStr::from(
                            self.hdt.dict.id_to_string(t.predicate_id, &IdKind::Predicate).unwrap(),
                        ))
                        .into_term(),
                        IriRef::new_unchecked(MownStr::from(
                            self.hdt.dict.id_to_string(t.object_id, &IdKind::Object).unwrap(),
                        ))
                        .into_term(),
                    ])
                }))
            }
         [...]
KonradHoeffner commented 1 year ago

It seems to work with SimpleTerm::from_term(term.as_simple());, is that the intended usage?

pchampin commented 1 year ago

I'll look into it in more detail, but my first reaction is: yes, from_term is the way to go here.

pchampin commented 1 year ago

I confirm that from_term is the way to go.

Term::as_simple borrows the term for a lifetime 'a and retuns a SimpleTerm<'a>, which borrows the underlying data of the term, so can not outlive the term.

When you call SimpleTerm::from_term(t), you actually call SimpleTerm::<'static>::from_term(t), which copies the data from t to ensures that it can live as long as 'static.

Hope this helps.

pchampin commented 1 year ago

Also, regarding the comment at the top:

  /// Only supports constant and "any" matchers.
  /// Non-constant matchers are supposed to be "any" matchers.

You do realize that supporting other matchers is straightforward:

                Box::new(SubjectIter::with_pattern(&self.hdt.triples, &TripleId::new(s.1, 0, 0)).map(move |t| {
                    Ok([
                        s.0,
                        IriRef::new_unchecked(MownStr::from(
                            self.hdt.dict.id_to_string(t.predicate_id, &IdKind::Predicate).unwrap(),
                        ))
                        .into_term(),
                        IriRef::new_unchecked(MownStr::from(
                            self.hdt.dict.id_to_string(t.object_id, &IdKind::Object).unwrap(),
                        ))
                        .into_term(),
                    ])
                }) // ADD HERE:
                .filter_triples(|t| pm.matches(t.p()) && om.matches(t.o()))
                ) 

The filter_triples should be optimized away if pm and om are both Any, causing no impact on performance. That's why I got rid of all triples_with_X variants.

KonradHoeffner commented 1 year ago

Oh, I didn't realize that, thanks!