Implement Basic Functionalities for the Query Module by angelip2303 · Pull Request #2 · rust-rdf/rdf.rs

angelip2303 · 2025-01-20T12:27:53Z

I am working on the basic functionalities of the query module. My idea is to write a definition of the structs that is as generic as possible, so that it can be used in any implementation of the Term trait. I am trying to respect as much as I can the original Ruby implementation, but I am having some issues with the fact that Ruby is dynamically typed. That's why we have introduced the Matcher struct, as Rust does not support Symbols (SPARQL query variables) and nil (the wildcard), and Pattern does not implement the Statement trait (term matchers can be variables).

Apart from that, let me know if I should modify this basic implementation before committing into the implementation of more complex methods.

angelip2303 · 2025-01-21T14:47:44Z

I've made a small refactor to the solution. Instead of using generics, I switched to dynamic dispatch, which I believe aligns better with the code style we've adopted in the rdf-model. I've also moved the Queryable trait to the rdf-query crate to prevent cyclic dependencies.

artob · 2025-01-22T21:41:23Z

@angelip2303 When would you expect this to be ready to review? (Since it's still in draft mode)

angelip2303 · 2025-01-22T21:42:49Z

@angelip2303 When would you expect this to be ready to review? (Since it's still in draft mode)

I believe that this will be ready before Friday, as I am currently trying to draft the basic query functionality

angelip2303 · 2025-01-23T14:54:41Z

The current implementation is almost fully functional, the only issue is that the execute method in Query does not create Solutions lazily, and that each concrete Solution stores a HeapTerm instead of a polymorphic Term. This last thing may not be an issue, but this design can be reconsidered.

artob · 2025-01-27T21:27:31Z

@race-of-sloths invite

race-of-sloths · 2025-01-27T21:28:16Z

@artob Thank you for calling!

@angelip2303 Thank you for the contribution! Join Race of Sloths by simply mentioning me in your comment/PRs description and start collecting Sloth Points through contributions to open source projects.

What is the Race of Sloths

Race of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow

For contributors:

Tag @race-of-sloths inside your pull requests
Wait for the maintainer to review and score your pull request
Check out your position in the Leaderboard
Keep weekly and monthly streaks to reach higher positions
Boast your contributions with a dynamic picture of your Profile

For maintainers:

Score pull requests that participate in the Race of Sloths and receive a reward
Engage contributors with fair scoring and fast responses so they keep their streaks
Promote the Race to the point where the Race starts promoting you
Grow the community of your contributors

Feel free to check our website for additional details!

Bot commands

For contributors
- Include a PR: @race-of-sloths include to enter the Race with your PR
For maintainers:
- Invite contributor @race-of-sloths invite to invite the contributor to participate in a race or include it, if it's already a runner.
- Assign points: @race-of-sloths score [1/2/3/5/8/13] to award points based on your assessment.
- Reject this PR: @race-of-sloths exclude to send this PR back to the drawing board.
- Exclude repo: @race-of-sloths pause to stop bot activity in this repo until @race-of-sloths unpause command is called

angelip2303 · 2025-01-31T09:07:00Z

This is ready for review 😎

imunproductive

Thanks for your work on this PR. I apologize for the delayed review. Overall the code seems good to me at first glance, but before going any further there is an immediate issue with building caused by OxrdfReader and BorshReader implementing trait Enumerable but not implementing its method grep. Could you please fix it?

imunproductive · 2025-02-17T16:51:07Z

 pub trait Enumerable:
    Countable + Iterator<Item = Result<Box<dyn Statement>, Box<dyn Error>>>
 {
+    fn grep(&self, pattern: &impl PartialEq<Box<dyn Statement>>) -> Self


grep is not implemented for OxrdfReader and BorshReader

SamuelSarle

Hello!
Generally, your implementation is good. Where it struggles is fitting with the existing Reader / Enumerable implementations.

Since in lib/rdf-query/src/lib.rs the module traits and in lib/rdf-query/src/pattern.rs the method Pattern::execute(&self, &Queryable) are non-public, I assume your intended interface for this package would be something like this:

use rdf_query::{pattern::Pattern, query::Query};
use rdf_reader::{
    // rdf_reader::providers::* isn't actually reachable because the `providers` module isn't public,
    // I modified it locally in lib/rdf-reader/lib/lib.rs.
    providers::OxrdfReader,

    Format,
    ReaderOptions,
};

fn main() {
    let file = std::fs::File::open("/path/to/data.ttl").unwrap();
    let options = ReaderOptions {
        format: Some(Format::Turtle),
    };
    let reader = OxrdfReader::new(file, options).unwrap();

    let pat = Pattern::new("<subject>", "<predicate>", "<object>", None);
    let query = Query::new(vec![pat]);

    let solutions = query.execute(&reader);

    for solution in solutions {
        for (variable, value) in solution.each_binding() {
            println!("{:?} => {:?}", variable, value);
        }
    }
}

This would be fine otherwise but the Queryable makes some assumptions that Reader / Enumerable can't promise. In essence all those types are wrapping some kind of iteration functionality but they don't—and sometimes can't—provide the sort of seek or "reset" that this snippet in the PR requires:

        let solutions: Vec<_> = self
            .patterns
            .iter()
            .flat_map(|pattern| {
                let statements = pattern.execute(queryable);
                statements.filter_map(|res| res.ok().map(|stmt| pattern.solution(stmt)))
            })
            .collect();

The flow for that is:

iter over patterns, for the first one run pattern.execute(quaryable). The quaryable reads statements from the underlying iterator until it is finished and returns None
repeat for the second pattern, the iterator is now empty and returns None.

Regarding the traits

Enumerable is a wrapper around Countable and Iterator<_>.
Reader and it's implementations like OxrdfReader implementations are a wrapper around Source which is around Enumerable, which is around Iterator, etc. etc.
Iterators are stateful and they give item for each call to Iterator::next until they are finished.
The reader implementations themselves are also wrappers around std::io:Read which only provides a method to get more bytes, has no resetting/seeking.

So all in all I can see some at least a couple methods to help with this:

Instead of executing each pattern sequentially over the Enumerable, read one item from Enumerable and match it against each pattern.
Have tighter requirements for Queryable than Enumerable, e.g. require that Queryable implementations also provide a reset method.
Read statements to memory and then match the query patterns over them. Not ideal for many reasons, there could be many more statements that fit memory, the reader could be over a network socket that doesn't "end", etc.
Perhaps tying Queryable to the readers (BorshReader/OxrdfReader) isn't the correct strategy? As far as I can tell from the RDF.rb side, execute takes # @param [RDF::Queryable] queryable # the graph or repository to query as input. I'd say not trying to implement Queryable for readers and waiting until there is a type that actually can be queried, like a repository, would be a better approach.

SamuelSarle · 2025-04-15T05:41:28Z

+impl From<&str> for Matcher {
+    fn from(name: &str) -> Self {
+        name.into()
+    }
+}


This implementation is self-recursing.

warning: function cannot return without recursing --> lib/rdf-query/src/matcher.rs:38:5 | 38 | / fn from(name: &str) -> Self { 39 | | name.into() 40 | | } | |_____^ | note: recursive call site --> lib/rdf-query/src/matcher.rs:39:9 | 39 | name.into() | ^^^^^^^^^^^ = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unconditional_recursion = note: `#[warn(clippy::unconditional_recursion)]` on by default

Suggested change

impl From<&str> for Matcher {

fn from(name: &str) -> Self {

name.into()

}

}

impl From<&str> for Matcher {

fn from(name: &str) -> Self {

Variable::from(name).into()

}

}

SamuelSarle · 2025-04-15T05:45:44Z

+
+    /// Executes the query on the given graph.
+    ///
+    /// If the query nas no patterns, it returns a single empty solution as per


Suggested change

/// If the query nas no patterns, it returns a single empty solution as per

/// If the query has no patterns, it returns a single empty solution as per

SamuelSarle · 2025-04-15T08:15:02Z

+    }
+
+    /// Returns true if the query has no patterns.
+    pub fn empty(&self) -> bool {


Optional: I think this would be good to rename to match the idiomatic format rust uses. Also you're using empty elsewhere with different semantics (Solutions::empty() -> Solutions).

Suggested change

pub fn empty(&self) -> bool {

pub fn is_empty(&self) -> bool {

Copilot

Pull Request Overview

This PR introduces basic query functionalities by defining generic structs and traits to support a SPARQL-like query system. Key changes include:

Implementing the Enumerable trait’s grep method for OxrdfReader and BorshReader with a stub (todo!()).
Adding new modules for Variable, Pattern, Query, and related query execution logic.
Updating traits in the rdf-model and rdf-query crates for query processing support.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
lib/rdf-reader/src/providers/oxrdf.rs	Adds a stub for the Enumerable trait’s grep method implementation.
lib/rdf-query/src/query.rs	Adds query execution logic with documentation for empty queries.
lib/rdf-query/src/pattern.rs	Implements pattern matching and solution extraction logic.
lib/rdf-query/src/matcher.rs	Introduces Matcher to support variable and term matching.
lib/rdf-model/src/traits/enumerable.rs	Extends Enumerable with the grep function signature.
lib/rdf-borsh/src/borsh_reader.rs	Adds a grep method stub to support Enumerable for BorshReader.
Other files (variable.rs, solution.rs, graph_pattern.rs, etc.)	Provide additional support for query building and execution.

Copilot · 2025-05-07T23:06:08Z

+    where
+        Self: Sized,
+    {
+        todo!()


The grep method is left unimplemented using todo!(), which could lead to runtime panics if invoked. It is advisable to either implement the functionality or clearly mark this stub as non-production ready.

Suggested change

todo!()

let filtered_statements: Vec<Box<dyn Statement>> = self

.filter_map(|result| result.ok()) // Ignore errors

.filter(|statement| pattern == statement)

.collect();

let reader = std::io::Cursor::new(filtered_statements);

let parser = RdfParser::from_format(self.format.try_into().expect("format must be compatible"))

.for_reader(reader);

Self {

format: self.format,

parser,

}

Copilot · 2025-05-07T23:06:08Z

+    where
+        Self: Sized,
+    {
+        todo!()


The grep method is left as a stub using todo!(), which may cause runtime issues when called. Consider implementing the method or providing an explicit indication that it is intentionally unimplemented.

Suggested change

todo!()

// Filter the quads that match the provided pattern

let filtered_quads: Vec<_> = self

.decompressor

.filter_map(|quad| {

if pattern == &Box::new(quad) {

Some(quad)

} else {

None

}

})

.collect();

// Create a new BorshReader with the filtered quads

BorshReader {

decompressor: self.decompressor.clone(), // Assuming decompressor is clonable

term_dict: self.term_dict.clone(),

quad_count: filtered_quads.len(),

read_count: 0,

}

Copilot · 2025-05-07T23:06:08Z

+
+    /// Executes the query on the given graph.
+    ///
+    /// If the query nas no patterns, it returns a single empty solution as per


Typo in the documentation comment: 'nas' should be 'has'.

Suggested change

/// If the query nas no patterns, it returns a single empty solution as per

/// If the query has no patterns, it returns a single empty solution as per

angelip2303 added 6 commits January 19, 2025 14:04

initial version with the structs defined

19bac93

implement statement for pattern

ce1ecfe

definition of variables to be used in triple patterns

6ff72bf

definition of triple pattern matchers; e.g. its building blocks

74dae99

definition of the triple patterns

4c1d8f1

definition of the solution of the query and the iterator over them

e604f89

angelip2303 added the enhancement New feature or request label Jan 20, 2025

angelip2303 self-assigned this Jan 20, 2025

angelip2303 requested a review from artob as a code owner January 20, 2025 12:27

artob requested review from SamuelSarle, gkellogg and labra January 20, 2025 14:14

angelip2303 requested review from SamuelSarle and removed request for SamuelSarle January 20, 2025 15:02

angelip2303 added 3 commits January 21, 2025 15:43

moving queryable to rdf-query

431ab2b

removing the necessity of having generics, using dynamic dispatch

50a3863

query execution

f4b6c4a

angelip2303 marked this pull request as draft January 21, 2025 14:47

angelip2303 added 6 commits January 23, 2025 15:50

enumerable now has the grep method

85edf20

matcher and variable can be built from a str

36281f1

query now depends on Enumerable

23ea2d3

solution manages the binding of a variable with an statement

32ec649

query execute returns solutions

4239b49

value is simplified

3465168

angelip2303 added 2 commits January 23, 2025 16:19

small simplification of the Solution creation

7f871e9

remove some TODOs

9b6c9da

angelip2303 marked this pull request as ready for review January 23, 2025 15:30

angelip2303 added 2 commits January 24, 2025 10:29

simple implementation of the query method

4ddfeed

minor improvements of the pattern struct

20aed7e

artob requested a review from imunproductive February 14, 2025 20:50

artob assigned imunproductive and unassigned angelip2303 Feb 14, 2025

imunproductive suggested changes Feb 17, 2025

View reviewed changes

minor fix so that the project can be built

abe38b3

SamuelSarle requested changes Apr 16, 2025

View reviewed changes

artob requested a review from Copilot May 7, 2025 23:05

Copilot AI reviewed May 7, 2025

View reviewed changes

	/// If the query nas no patterns, it returns a single empty solution as per
	/// If the query has no patterns, it returns a single empty solution as per

	pub fn empty(&self) -> bool {
	pub fn is_empty(&self) -> bool {

-        todo!()
+        let filtered_statements: Vec<Box<dyn Statement>> = self
+            .filter_map(|result| result.ok()) // Ignore errors
+            .filter(|statement| pattern == statement)
+            .collect();
+        let reader = std::io::Cursor::new(filtered_statements);
+        let parser = RdfParser::from_format(self.format.try_into().expect("format must be compatible"))
+            .for_reader(reader);
+        Self {
+            format: self.format,
+            parser,
+        }

-        todo!()
+        // Filter the quads that match the provided pattern
+        let filtered_quads: Vec<_> = self
+            .decompressor
+            .filter_map(|quad| {
+                if pattern == &Box::new(quad) {
+                    Some(quad)
+                } else {
+                    None
+                }
+            })
+            .collect();
+        // Create a new BorshReader with the filtered quads
+        BorshReader {
+            decompressor: self.decompressor.clone(), // Assuming decompressor is clonable
+            term_dict: self.term_dict.clone(),
+            quad_count: filtered_quads.len(),
+            read_count: 0,
+        }

Conversation

angelip2303 commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angelip2303 commented Jan 21, 2025

Uh oh!

artob commented Jan 22, 2025

Uh oh!

angelip2303 commented Jan 22, 2025

Uh oh!

angelip2303 commented Jan 23, 2025

Uh oh!

artob commented Jan 27, 2025

Uh oh!

race-of-sloths commented Jan 27, 2025

Uh oh!

angelip2303 commented Jan 31, 2025

Uh oh!

imunproductive left a comment

Choose a reason for hiding this comment

Uh oh!

imunproductive Feb 17, 2025 • 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!

SamuelSarle 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!

SamuelSarle Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

SamuelSarle Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

SamuelSarle Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

angelip2303 commented Jan 20, 2025 •

edited

Loading

imunproductive Feb 17, 2025 •

edited

Loading

SamuelSarle left a comment •

edited

Loading