RDFLib / rdflib

RDFLib is a Python library for working with RDF, a simple yet powerful language for representing information.
https://rdflib.readthedocs.org
BSD 3-Clause "New" or "Revised" License
2.15k stars 555 forks source link

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

Closed mgberg closed 2 months ago

mgberg commented 2 months ago

Summary of changes

Disclaimer: I am not super familiar with the SPARQL algebra/translation code in RDFLib, so I'm not confident this is the best solution. Also I'm not 100% confident this will solve all nested filter issues.

This fixes an issue which allows nested filters to work as discussed in #709. Previously, a graph pattern could be passed to rdflib.plugins.sparql.algebra::translateGroupGraphPattern multiple times. If this occurs, the graph pattern would get overwritten with an empty pattern, causing the nested filters to appear to be dropped in the query plan. This modifies translateGroupGraphPattern to:

  1. Mark translated graph patterns as translated, and
  2. Simply return the provided graph pattern if it already has been translated.

Additionally, this includes two new test cases compiled by @ajnelson-nist that verifies that known nested filter issues work (at least those discussed in #709).

Checklist

ajnelson-nist commented 2 months ago

Thank you for nudging this forward, @mgberg .

coveralls commented 2 months ago

Coverage Status

coverage: 91.041% (+0.001%) from 91.04% when pulling 958ffb469c6ae003337e7611c3c4edca2e64b100 on mgberg:issue-709 into a7865e9b1f542310c45884a62feaec9340011944 on RDFLib:main.

nicholascar commented 2 months ago

Hi @ajnelson-nist & @mgberg,

Thanks for this and similar PRs. We are about to unblock this long list of auto-PRs from dependabot etc. and will merge this in after doing that. Will also make a 7.10 release. ETA: by August.

ashleysommer commented 2 months ago

Merge blockages should be unblocked now. I've merged latest main branch into this PR, and hopefully everything passes.