Fix bug preventing nested FILTER statements from working (#709) (#2822)

* test: Start tests for nested FILTER statements (#709) This patch adds a single test, self-authored, recreating an issue I encountered when attempting to use a query with a nested `FILTER NOT EXISTS` statement. This test is known to currently fail, but is expected to pass as written with a corrected implementation. References: * #709 Signed-off-by: Alex Nelson <[email protected]> * test: Add nested FILTER statement test (#709) @mgberg wrote the graph and query for this test last year. This patch puts his work, with his permission, into the new test. This test is known to currently fail, but is expected to pass as written with a corrected implementation. References: * #709 (comment) Co-authored-by: Matt Goldberg <[email protected]> Signed-off-by: Alex Nelson <[email protected]> * sparql algebra: Prevent graph patterns from being translated more than once to enable nested filters to work * Fix lint errors in test_nested_filters.py --------- Signed-off-by: Alex Nelson <[email protected]> Co-authored-by: Alex Nelson <[email protected]> Co-authored-by: Ashley Sommer <[email protected]>
RDFLib · Jul 24, 2024 · 6c5a783 · 6c5a783
1 parent 4cb7d5e
commit 6c5a783
Show file tree

Hide file tree

Showing 2 changed files with 356 additions and 0 deletions.
diff --git a/rdflib/plugins/sparql/algebra.py b/rdflib/plugins/sparql/algebra.py
@@ -328,6 +328,11 @@ def translateGroupGraphPattern(graphPattern: CompValue) -> CompValue:
     http://www.w3.org/TR/sparql11-query/#convertGraphPattern
     """
 
+    if graphPattern.translated:
+        # This occurs if it is attempted to translate a group graph pattern twice,
+        # which occurs with nested (NOT) EXISTS filters. Simply return the already
+        # translated pattern instead.
+        return graphPattern
     if graphPattern.name == "SubSelect":
         # The first output from translate cannot be None for a subselect query
         # as it can only be None for certain DESCRIBE queries.
@@ -384,6 +389,9 @@ def translateGroupGraphPattern(graphPattern: CompValue) -> CompValue:
     if filters:
         G = Filter(expr=filters, p=G)
 
+    # Mark this graph pattern as translated
+    G.translated = True
+
     return G
 
 

diff --git a/test/test_sparql/test_nested_filters.py b/test/test_sparql/test_nested_filters.py
@@ -0,0 +1,348 @@
+#!/usr/bin/env python3
+
+# Portions of this script contributed by NIST are governed by the
+# following license:
+#
+# This software was developed at the National Institute of Standards
+# and Technology by employees of the Federal Government in the course
+# of their official duties. Pursuant to title 17 Section 105 of the
+# United States Code this software is not subject to copyright
+# protection and is in the public domain. NIST assumes no
+# responsibility whatsoever for its use by other parties, and makes
+# no guarantees, expressed or implied, about its quality,
+# reliability, or any other characteristic.
+#
+# We would appreciate acknowledgement if the software is used.
+
+from __future__ import annotations
+
+import logging
+from typing import Set, Tuple
+
+from rdflib import Graph, URIRef
+from rdflib.query import ResultRow
+
+
+def test_nested_filter_outer_binding_propagation() -> None:
+    expected: Set[URIRef] = {
+        URIRef("http://example.org/Superclass"),
+    }
+    computed: Set[URIRef] = set()
+    graph_data = """\
+@prefix ex: <http://example.org/> .
+@prefix owl: <http://www.w3.org/2002/07/owl#> .
+@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
+
+ex:Superclass
+  a owl:Class ;
+.
+ex:Subclass1
+  a owl:Class ;
+  rdfs:subClassOf ex:Superclass ;
+.
+ex:Subclass2
+  a owl:Class ;
+  rdfs:subClassOf ex:Superclass ;
+  owl:deprecated true ;
+.
+"""
+    query = """\
+SELECT ?class
+WHERE {
+  ?class a owl:Class .
+  FILTER EXISTS {
+    ?subclass rdfs:subClassOf ?class .
+    FILTER NOT EXISTS { ?subclass owl:deprecated true }
+  }
+}
+"""
+    graph = Graph()
+    graph.parse(data=graph_data)
+    for result in graph.query(query):
+        assert isinstance(result, ResultRow)
+        assert isinstance(result[0], URIRef)
+        computed.add(result[0])
+    assert expected == computed
+
+
+def test_nested_filter_outermost_binding_propagation() -> None:
+    """
+    This test implements a query that requires functionality of nested FILTER NOT EXISTS query components.
+
+    It encodes a single ground truth positive query result, a tuple where:
+    * The first member is a HistoricAction,
+    * The second member is a wholly redundant HistoricRecord in consideration of latter HistoricRecords that cover all non-HistoricRecord inputs to the Action, and
+    * The third member is the superseding record.
+    """
+    expected: Set[Tuple[URIRef, URIRef, URIRef]] = {
+        (
+            URIRef("http://example.org/kb/action-1-2"),
+            URIRef("http://example.org/kb/record-123-1"),
+            URIRef("http://example.org/kb/record-1-2"),
+        )
+    }
+    computed: Set[Tuple[URIRef, URIRef, URIRef]] = set()
+
+    historic_ontology_graph_data = """\
+@prefix case-investigation: <https://ontology.caseontology.org/case/investigation/> .
+@prefix ex: <http://example.org/ontology/> .
+@prefix kb: <http://example.org/kb/> .
+@prefix owl: <http://www.w3.org/2002/07/owl#> .
+@prefix prov: <http://www.w3.org/ns/prov#> .
+@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
+@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
+@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .
+
+<http://example.org/ontology>
+  a owl:Ontology ;
+  rdfs:comment "This example ontology represents a history-analyzing application, where notes of things' handling are created and accompany the things as they are used in actions.  For the sake of demonstration, classes and properties implemented here are simplifications of other ontologies' classes and properties.  Otherwise, this ontology is narrowly similar to an application of the CASE and PROV-O ontologies."@en ;
+  rdfs:seeAlso <https://github.com/casework/CASE-Implementation-PROV-O> ;
+  .
+
+# Begin ontology (TBox).
+
+ex:HistoricThing
+  a owl:Class ;
+  rdfs:subClassOf owl:Thing ;
+  rdfs:comment "A thing generated by some HistoricAction with an accompanying HistoricRecord, and is the input to other HistoricActions.  When a HistoricThing is the input to a HistoricAction, a new HistoricRecord should be emitted by the HistoricAction."@en ;
+  rdfs:seeAlso prov:Entity ;
+  .
+
+ex:HistoricRecord
+  a owl:Class ;
+  rdfs:subClassOf ex:HistoricThing ;
+  rdfs:comment
+    "An example class analagous to PROV-O's Collection and CASE's ProvenanceRecord."@en ,
+    "Only the latest HistoricRecord for an object should be an input to a HistoricAction."@en
+    ;
+  rdfs:seeAlso
+    case-investigation:ProvenanceRecord ,
+    prov:Collection
+    ;
+  .
+
+ex:HistoricAction
+  a owl:Class ;
+  rdfs:subClassOf owl:Thing ;
+  rdfs:comment "An example class analagous to PROV-O's Activity and CASE's InvestigativeAction."@en ;
+  rdfs:seeAlso
+    case-investigation:InvestigativeAction ,
+    prov:Activity
+    ;
+  owl:disjointWith ex:HistoricThing ;
+  .
+
+ex:hadMember
+  a owl:ObjectProperty ;
+  rdfs:domain ex:HistoricRecord ;
+  rdfs:range ex:HistoricThing ;
+  rdfs:seeAlso prov:hadMember ;
+  .
+
+ex:generated
+  a owl:ObjectProperty ;
+  rdfs:domain ex:HistoricAction ;
+  rdfs:range ex:HistoricThing ;
+  rdfs:seeAlso prov:wasGeneratedBy ;
+  .
+
+ex:used
+  a owl:ObjectProperty ;
+  rdfs:domain ex:HistoricAction ;
+  rdfs:range ex:HistoricThing ;
+  rdfs:seeAlso prov:used ;
+  .
+
+ex:wasDerivedFrom
+  a owl:ObjectProperty ;
+  rdfs:domain owl:Thing ;
+  rdfs:range owl:Thing ;
+  rdfs:seeAlso prov:wasDerivedFrom ;
+  .
+
+# Begin knowledge base (ABox).
+
+kb:record-123-1
+  a ex:HistoricRecord ;
+  rdfs:comment "This is a first record of having handled thing-1, thing-2, and thing-3."@en ;
+  ex:hadMember
+    kb:thing-1 ,
+    kb:thing-2
+    ;
+  .
+
+kb:record-1-2
+  a ex:HistoricRecord ;
+  rdfs:comment "This is a second record of having handled thing-1."@en ;
+  ex:hadMember kb:thing-1 ;
+  ex:wasDerivedFrom kb:record-123-1 ;
+  .
+
+kb:record-2-2
+  a ex:HistoricRecord ;
+  rdfs:comment "This is a second record of having handled thing-2."@en ;
+  ex:hadMember kb:thing-2 ;
+  ex:wasDerivedFrom kb:record-123-1 ;
+  .
+
+kb:record-4-1
+  a ex:HistoricRecord ;
+  rdfs:comment "This is a first record of having handled thing-4.  thing-4 is independent in history of thing-1 and thing-2."@en ;
+  ex:hadMember kb:thing-4 ;
+  .
+
+kb:thing-1
+  a ex:HistoricThing ;
+  .
+
+kb:thing-2
+  a ex:HistoricThing ;
+  .
+
+kb:thing-3
+  a ex:HistoricThing ;
+  .
+
+kb:thing-4
+  a ex:HistoricThing ;
+  .
+
+kb:action-123-0
+  a ex:HistoricAction ;
+  rdfs:comment "Generate things 1, 2, and 3."@en ;
+  ex:generated
+    kb:record-123-1 ,
+    kb:thing-1 ,
+    kb:thing-2 ,
+    kb:thing-3
+    .
+
+kb:action-4-0
+  a ex:HistoricAction ;
+  rdfs:comment "Generate thing 4."@en ;
+  ex:generated
+    kb:record-4-1 ,
+    kb:thing-4
+    .
+
+kb:action-1-1
+  a ex:HistoricAction ;
+  rdfs:comment "Handle thing-1."@en ;
+  ex:used
+    kb:record-123-1 ,
+    kb:thing-1
+    ;
+  ex:generated kb:record-1-2 ;
+  .
+
+kb:action-2-1
+  a ex:HistoricAction ;
+  rdfs:comment "Handle thing-2."@en ;
+  ex:used
+    kb:record-123-1 ,
+    kb:thing-2
+    ;
+  ex:generated kb:record-2-2 ;
+  .
+
+kb:action-1-2
+  a ex:HistoricAction ;
+  rdfs:comment "This node SHOULD be found by the query.  record-123-1 is wholly redundant with record-1-2 with respect to the collective whole of action inputs."@en ;
+  ex:used
+    kb:record-123-1 ,
+    kb:record-1-2 ,
+    kb:thing-1
+    ;
+  .
+
+kb:action-12-2
+  a ex:HistoricAction ;
+  rdfs:comment "This node SHOULD NOT be found by the query.  record-123-1 is partially, but not wholly, redundant with record-1-2, due to to thing-2 having record-123-1 as its only accompanying historic record."@en ;
+  ex:used
+    kb:record-123-1 ,
+    kb:record-1-2 ,
+    kb:thing-1 ,
+    kb:thing-2
+    ;
+  .
+
+kb:action-123-2
+  a ex:HistoricAction ;
+  rdfs:comment "This node SHOULD NOT be found by the query.  record-123-1 is partially, but not wholly, redundant with record-1-2 and record-2-2, due to thing-3 having record-123-1 as its only accompanying historic record."@en ;
+  ex:used
+    kb:record-123-1 ,
+    kb:record-1-2 ,
+    kb:record-2-2 ,
+    kb:thing-1 ,
+    kb:thing-2 ,
+    kb:thing-3
+    ;
+  .
+
+kb:action-1234-2
+  a ex:HistoricAction ;
+  rdfs:comment "This node SHOULD NOT be found by the query.  record-123-1 is partially, but not wholly, redundant with record-1-2 and record-2-2, due to thing-3 having record-123-1 as its only accompanying historic record.  thing-4 also has no shared history with thing-1, -2, or -3."@en ;
+  ex:used
+    kb:record-123-1 ,
+    kb:record-1-2 ,
+    kb:record-2-2 ,
+    kb:record-4-1 ,
+    kb:thing-1 ,
+    kb:thing-2 ,
+    kb:thing-3 ,
+    kb:thing-4
+    ;
+  .
+"""
+
+    # See 'TEST OBJECTIVE' annotation.
+    query = """\
+PREFIX ex: <http://example.org/ontology/>
+SELECT ?nAction ?nRedundantRecord ?nSupersedingRecord
+WHERE {
+  ?nAction
+    ex:used
+      ?nThing1 ,
+      ?nRedundantRecord ,
+      ?nSupersedingRecord
+      ;
+    .
+  ?nRedundantRecord
+    a ex:HistoricRecord ;
+    ex:hadMember ?nThing1 ;
+    .
+  ?nSupersedingRecord
+    a ex:HistoricRecord ;
+    ex:wasDerivedFrom+ ?nRedundantRecord ;
+    ex:hadMember ?nThing1 ;
+    .
+  FILTER NOT EXISTS {
+    ?nAction ex:used ?nThing2 .
+    ?nRedundantRecord ex:hadMember ?nThing2 .
+    FILTER ( ?nThing1 != ?nThing2 )
+    FILTER NOT EXISTS {
+      ####
+      #
+      # TEST OBJECTIVE:
+      # nThing2 must be passed from the outermost context.
+      #
+      ####
+      ?nSupersedingRecord ex:hadMember ?nThing2 .
+    }
+  }
+}
+"""
+
+    graph = Graph()
+    graph.parse(data=historic_ontology_graph_data)
+    logging.debug(len(graph))
+
+    for result in graph.query(query):
+        assert isinstance(result, ResultRow)
+        assert isinstance(result[0], URIRef)
+        assert isinstance(result[1], URIRef)
+        assert isinstance(result[2], URIRef)
+
+        computed.add((result[0], result[1], result[2]))
+
+    assert expected == computed