feat(cognitarium)!: add query expression filters #616
Conversation
WalkthroughThe changes introduce a more flexible and robust query handling system by modifying how Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QueryEngine
participant PlanBuilder
participant ExpressionEvaluator
User->>QueryEngine: Execute Query with Optional WhereClause
QueryEngine->>PlanBuilder: Build Query Plan
PlanBuilder->>ExpressionEvaluator: Evaluate Expressions
ExpressionEvaluator-->>PlanBuilder: Return Results
PlanBuilder-->>QueryEngine: Return Query Plan
QueryEngine-->>User: Return Query Results
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
Sounds good 👍 |
adopt a more specific naming for the type of clauses and allow to combine multiple ones, simplifying the addition of new ones
amimart
force-pushed
the
feat/cognitarium-expr-eval
branch
2 times, most recently
from
2c882ca to
87b53fa
Compare
amimart
force-pushed
the
feat/cognitarium-expr-eval
branch
from
87b53fa to
e179988
Compare
amimart
force-pushed
the
feat/cognitarium-expr-eval
branch
from
a374a14 to
73df19c
Compare
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (3)
contracts/axone-cognitarium/src/querier/mod.rs (1)
2-2: Consider documenting the newexpressionmodule.The addition of the
expressionmodule suggests new functionality. Consider adding documentation to clarify its purpose and usage.contracts/axone-cognitarium/src/state/test_util.rs (1)
1-3: Consider adding documentation forInMemoryNamespaceSolver.Adding documentation will help clarify the purpose and usage of this in-memory solver for future developers.
contracts/axone-cognitarium/src/msg.rs (1)
469-482: Enhance documentation forWhereClause.The
WhereClauseenum now supportsBgp,LateralJoin, andFilter. Ensure that the documentation clearly explains each variant's purpose and usage, especially the newFiltervariant.Consider adding examples to illustrate how each variant can be used in queries.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- contracts/axone-cognitarium/src/contract.rs (45 hunks)
- contracts/axone-cognitarium/src/msg.rs (4 hunks)
- contracts/axone-cognitarium/src/querier/engine.rs (23 hunks)
- contracts/axone-cognitarium/src/querier/expression.rs (1 hunks)
- contracts/axone-cognitarium/src/querier/mapper.rs (3 hunks)
- contracts/axone-cognitarium/src/querier/mod.rs (1 hunks)
- contracts/axone-cognitarium/src/querier/plan.rs (6 hunks)
- contracts/axone-cognitarium/src/querier/plan_builder.rs (12 hunks)
- contracts/axone-cognitarium/src/querier/variable.rs (7 hunks)
- contracts/axone-cognitarium/src/state/mod.rs (1 hunks)
- contracts/axone-cognitarium/src/state/namespaces.rs (5 hunks)
- contracts/axone-cognitarium/src/state/test_util.rs (1 hunks)
- contracts/axone-cognitarium/src/state/triples.rs (2 hunks)
- contracts/axone-cognitarium/src/storer/engine.rs (2 hunks)
- contracts/axone-dataverse/src/contract.rs (2 hunks)
- contracts/axone-dataverse/src/registrar/registry.rs (2 hunks)
- docs/axone-cognitarium.md (8 hunks)
Additional context used
GitHub Check: codecov/patch
contracts/axone-cognitarium/src/state/test_util.rs
[warning] 35-40: contracts/axone-cognitarium/src/state/test_util.rs#L35-L40
Added lines #L35 - L40 were not covered by testscontracts/axone-cognitarium/src/querier/plan.rs
[warning] 115-118: contracts/axone-cognitarium/src/querier/plan.rs#L115-L118
Added lines #L115 - L118 were not covered by testscontracts/axone-cognitarium/src/state/namespaces.rs
[warning] 220-222: contracts/axone-cognitarium/src/state/namespaces.rs#L220-L222
Added lines #L220 - L222 were not covered by testscontracts/axone-cognitarium/src/querier/engine.rs
[warning] 163-172: contracts/axone-cognitarium/src/querier/engine.rs#L163-L172
Added lines #L163 - L172 were not covered by tests
[warning] 223-223: contracts/axone-cognitarium/src/querier/engine.rs#L223
Added line #L223 was not covered by tests
[warning] 773-773: contracts/axone-cognitarium/src/querier/engine.rs#L773
Added line #L773 was not covered by testscontracts/axone-cognitarium/src/contract.rs
[warning] 106-106: contracts/axone-cognitarium/src/contract.rs#L106
Added line #L106 was not covered by tests
[warning] 230-232: contracts/axone-cognitarium/src/contract.rs#L230-L232
Added lines #L230 - L232 were not covered by tests
[warning] 291-291: contracts/axone-cognitarium/src/contract.rs#L291
Added line #L291 was not covered by tests
Markdownlint
docs/axone-cognitarium.md
420-420: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
421-421: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
422-422: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
525-525: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
526-526: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
550-550: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
551-551: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
552-552: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
571-571: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
572-572: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
599-599: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
600-600: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
601-601: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
644-644: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
645-645: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
646-646: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
741-741: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
924-924: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
Additional comments not posted (40)
contracts/axone-cognitarium/src/querier/mod.rs (1)
9-9: Ensure theplanmodule export is necessary.The
planmodule is now publicly exported. Verify that this export is required and aligns with the intended architecture.contracts/axone-cognitarium/src/state/mod.rs (1)
11-14: Good practice: Conditional compilation for testing utilities.The use of
#[cfg(test)]for thetest_utilmodule is a good practice, ensuring it only compiles during testing.contracts/axone-cognitarium/src/querier/mapper.rs (3)
Line range hint
9-20: Simplification and renaming approved.The changes to
literal_as_objectsimplify the function by removing thestoragedependency and align with the newNamespaceSolvernaming convention.
Line range hint
26-39: Simplification and renaming approved.The changes to
iri_as_nodesimplify the function by removing thestoragedependency and align with the newNamespaceSolvernaming convention.
43-48: New function addition approved.The
iri_as_stringfunction provides a useful utility for convertingIRItypes into strings. The implementation is straightforward and aligns with existing patterns.contracts/axone-dataverse/src/registrar/registry.rs (1)
39-47: Enhanced query flexibility approved.The use of
WhereClause::Bgpallows for more complex graph patterns, aligning with the PR objectives of enhancing querying capabilities. The changes maintain the intended functionality.contracts/axone-cognitarium/src/querier/plan.rs (4)
24-29: New method addition approved.The
empty_planmethod provides a straightforward way to create an emptyQueryPlan, enhancing usability. The implementation is simple and aligns with the intended functionality.
77-78: New variant addition approved.The
Filtervariant inQueryNodeenhances query capabilities by allowing filtering based on anExpression. This aligns with the PR objectives of improving query precision.
88-91: New method addition approved.The
noopmethod provides a way to create a no-operation node, improving the flexibility of constructing query nodes. The implementation is straightforward and aligns with the intended functionality.
115-118: Modification forFiltervariant approved.The changes to
lookup_bound_variablesensure that theFiltervariant is correctly handled, maintaining the integrity of variable management. The implementation is consistent with existing patterns.Tools
GitHub Check: codecov/patch
[warning] 115-118: contracts/axone-cognitarium/src/querier/plan.rs#L115-L118
Added lines #L115 - L118 were not covered by testscontracts/axone-cognitarium/src/state/triples.rs (1)
154-155: LGTM! VerifyNamespaceSolverimplementation.The change to use
NamespaceSolverenhances type safety and clarity. Ensure that theNamespaceSolvertrait is correctly implemented for all necessary structures.Run the following script to verify the
NamespaceSolvertrait implementation:contracts/axone-cognitarium/src/storer/engine.rs (1)
283-286: LGTM!The refactoring of the
node_sizefunction improves readability and reduces code complexity while maintaining functionality.contracts/axone-cognitarium/src/state/namespaces.rs (1)
44-47: LGTM!The renaming of
NamespaceResolvertoNamespaceQuerierand the introduction ofNamespaceSolverenhance modularity and clarity in namespace management.contracts/axone-cognitarium/src/querier/variable.rs (3)
Line range hint
54-99: LGTM! Theas_valuemethod changes enhance flexibility.The update to accept a mutable reference to a
NamespaceSolvertrait object improves the method's extensibility.
101-124: LGTM! Theas_termmethod is well-implemented.The method effectively converts
ResolvedVariableinstances intoTermobjects using theNamespaceSolver.
177-187: LGTM! TheHasBoundVariablestrait adds useful abstraction.The trait provides a structured approach for handling variable lookups, enhancing the module's functionality.
contracts/axone-dataverse/src/contract.rs (2)
145-146: LGTM! The import forWhereClauseis necessary.The addition of the
WhereClauseimport reflects its new usage in query construction.
306-314: LGTM! The test case update improves clarity.The use of
WhereClause::Bgpallows for more structured query conditions in the test.contracts/axone-cognitarium/src/querier/expression.rs (3)
10-70: LGTM! TheExpressionenum andevaluatemethod are well-implemented.The enum provides a comprehensive set of logical operations, and the
evaluatemethod effectively handles different expression types.
100-153: LGTM! TheTermenum and its methods are well-designed.The enum provides necessary conversions and comparisons, and the methods are implemented correctly.
155-512: LGTM! The test cases are comprehensive and thorough.The tests cover various scenarios, ensuring the correctness of the
ExpressionandTermimplementations.contracts/axone-cognitarium/src/msg.rs (2)
76-76: Ensure backward compatibility with optionalWhereClause.The
r#wherefield in bothDeleteDataandDescribeQueryhas been changed toOption<WhereClause>. Ensure that any existing code handling these structures can accommodate the possibility of aNonevalue.Consider adding tests to verify the behavior when
r#whereisNone.Also applies to: 428-428
485-514: Verify logical operations inExpression.The
Expressionenum supports various logical operations. Ensure that these operations are correctly implemented and tested, especially complex combinations ofAnd,Or, andNot.Consider adding tests to cover edge cases and complex logical expressions.
contracts/axone-cognitarium/src/querier/plan_builder.rs (2)
14-14: Initializens_resolvercorrectly inPlanBuilder.The
ns_resolverfield is initialized usingNamespaceResolver::new. Ensure that the initialization correctly handles the providedstorageandns_cache.Consider adding tests to verify the behavior of
PlanBuilderwith different namespace configurations.
68-87: Ensure comprehensive testing ofbuild_node.The
build_nodemethod handles differentWhereClausevariants. Ensure that each variant is correctly processed and that error handling, especially for unbound variables, is robust.Consider adding tests to cover each
WhereClausevariant and potential error scenarios.contracts/axone-cognitarium/src/querier/engine.rs (3)
22-22: Ensurens_cacheis correctly utilized inQueryEngine.The
ns_cachefield is introduced to manage namespaces. Ensure that it is correctly initialized and used throughout theQueryEngine.Consider adding tests to verify the behavior of
QueryEnginewith different namespace configurations.
163-172: Add test coverage forFilterIterator.The
FilterIteratoris a new addition that implements filtering logic. Ensure that it is thoroughly tested, especially for edge cases and complex expressions.Consider adding tests to cover the filtering logic and potential error scenarios.
Tools
GitHub Check: codecov/patch
[warning] 163-172: contracts/axone-cognitarium/src/querier/engine.rs#L163-L172
Added lines #L163 - L172 were not covered by tests
773-773: Add test coverage forliteral_as_object.The
literal_as_objectfunction is used inbuild_object_template. Ensure that it is correctly implemented and tested, especially for different literal types.Consider adding tests to cover various literal conversions and potential error scenarios.
Tools
GitHub Check: codecov/patch
[warning] 773-773: contracts/axone-cognitarium/src/querier/engine.rs#L773
Added line #L773 was not covered by testsdocs/axone-cognitarium.md (6)
423-423: Verify thedelete_data.whereparameter description.Ensure that the
WhereClausetype and its usage are accurately described in the documentation.
552-552: Verify thewhereparameter inconstructquery description.Ensure that the
WhereClausetype and its usage in theconstructquery are accurately described in the documentation.Tools
Markdownlint
552-552: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
573-573: Verify thewhereparameter indescribequery description.Ensure that the
WhereClausetype and its usage in thedescribequery are accurately described in the documentation.
910-910: Verify consistency ofWhereConditiontoWhereClauserenaming.Ensure that the renaming is consistently applied throughout the documentation.
519-527: Verify the newBgpsection description.Ensure that the
Bgpconstruct and its usage are accurately described in the new section.Tools
Markdownlint
525-525: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
526-526: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
Line range hint
575-647: Verify the new sections forExpression,Filter, andLateralJoin.Ensure that the constructs and their usage are accurately described in the new sections.
Tools
Markdownlint
571-571: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
572-572: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
599-599: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
600-600: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
601-601: null
Spaces inside emphasis markers(MD037, no-space-in-emphasis)
contracts/axone-cognitarium/src/contract.rs (6)
96-107: Verify the correctness of theexecutefunction changes.Ensure that the changes to handle an optional
WhereClauseare correctly implemented and consistent with the PR objectives.Tools
GitHub Check: codecov/patch
[warning] 106-106: contracts/axone-cognitarium/src/contract.rs#L106
Added line #L106 was not covered by tests
101-107: Verify the logic of thedeletefunction.Ensure that the logic for handling the
deletevector andr#whereparameter is correctly implemented.Tools
GitHub Check: codecov/patch
[warning] 106-106: contracts/axone-cognitarium/src/contract.rs#L106
Added line #L106 was not covered by tests
223-233: Verify the correctness of thedescribefunction changes.Ensure that the usage of
WhereClausefor query construction is correctly implemented.Tools
GitHub Check: codecov/patch
[warning] 230-232: contracts/axone-cognitarium/src/contract.rs#L230-L232
Added lines #L230 - L232 were not covered by tests
282-292: Verify the correctness of theconstructfunction changes.Ensure that the usage of
WhereClausefor query construction is correctly implemented.Tools
GitHub Check: codecov/patch
[warning] 291-291: contracts/axone-cognitarium/src/contract.rs#L291
Added line #L291 was not covered by tests
203-203: Verify the correctness of theselectfunction changes.Ensure that the usage of
WhereClausefor query construction is correctly implemented.
Line range hint
354-363: Verify the correctness of themap_select_solutionsfunction.Ensure that the function correctly maps query solutions to the expected response format.
Closes #614
Description
Here is an API evolution to integrate expression filtering capabilities in the cognitarium querying mechanism. The changes are breaking.
The idea is to enhance the
WhereClausetype by renaming the oldSimplebyBgpfor basic graph patterns and adding a newFilterclause allowing to filter another clause based on expressions, this would allow to filter with specific variable values.Here is an example using this to filter on a range of a
heightvalue:{ "filter": { "expr": { "and": [ { "greater_or_equal": [ {"variable": "height"}, {"literal": "1000"} ] }, { "less_or_equal": [ {"variable": "height"}, {"literal": "2000"} ] } ] }, "inner": { "bgp": [ { "subject": {"variable": "subject"}, "predicate": {"named": {"full": "height"}}, "object": {"variable": "height"} } ] } } }In this example the BGP will provide the resolution of the
heightvariable and the filter will then limit the possibilities for those between 1000 and 2000.@ccamel @bdeneux I'd appreciate your insights on this design before going further in the implementation :)
Details
The
WhereClause::Filtertakes an inner clause and filtering matching the providedExpression. An expression internally resolves as a term, which can currently be either aStringor abool. A term can be built from a variable so it can be constructed from object literals for example, the construction of the resulting string is opinionated and impact the behaviour of comparison operators, so I'll let you juge the proposed implementation.This implementation is very simplistic, it'd make sense in the future to provide more types of
Termfor a fine grained control on expression operators...Misc
A
WhereClause::LateralJoinclause has also been added to allow to join two clauses as a for loop, this was needed to make theDescribemessage to work with the refactoring.The namespace resolution mechanism has been reworked by providing trait abstraction to ease tests and integration with the new filtering capabilities, it brings a lot of additional changes sorry for that..
Summary by CodeRabbit
New Features
WhereClausein query operations.Expressiontype for complex logical operations in queries.Filtervariant in query plans to enable conditional filtering of results.Bug Fixes
WhereClauseis provided.Documentation
WhereClause,Bgp, andExpression.Chores