Repository navigation
Conversation
… datasets
An empty graph pattern such as GRAPH ?g { } becomes OpDatasetNames in
quad form. Until now only QueryEngineTDB could execute it: the general
OpExecutor threw ARQNotImplemented, and OpExecutorTDB2 cast the dataset
to DatasetGraphTDB, which fails with a ClassCastException when the
query runs against DatasetGraphSwitchable or any other wrapper.
OpExecutor now evaluates OpDatasetNames itself. A concrete graph name
passes the input bindings through if that graph exists in the dataset,
and yields nothing otherwise. A variable is bound to each graph name in
the dataset, or checked against the dataset if an earlier part of the
query already bound it.
OpExecutorTDB2 unwraps the dataset with TDBInternal.getDatasetGraphTDB
and falls back to the general implementation when the dataset is not
TDB2 storage. SolverLibTDB.graphNames keeps the incoming bindings
instead of replacing them, so GRAPH ?g { } composes with the rest of
the query the same way on TDB2 as elsewhere.
Tests cover both executors, bound and unbound graph variables, and
wrapped datasets.
Generated-by: Claude Code, reviewed and tested by the author.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
|
This will take a long time to review properly. Remove the fiddling around with a QueryEngineRegistry and the example works, and it works if a DatasetGraphWrapper is added. TDB2 execution does not go direct to the quad engine. This is the problem with AI. It solves a problem; the question is whether it is solving the problem. |
Author
No rush on my side. I was able to fix the CCE, but know how this can cascade |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub issue resolved #4362
Pull request Description:
GRAPH ?g { }andGRAPH <iri> { }becomeOpDatasetNameswhen a query is turned into quad form. Today that only executes throughQueryEngineTDB. The generalOpExecutorthrowsARQNotImplemented, andOpExecutorTDB2casts the execution dataset toDatasetGraphTDB, which fails with aClassCastExceptionwhen the query runs against aDatasetGraphSwitchableor another wrapper.A runnable example of the failure, as asked for on the issue. It routes the query through the general quad engine, which is what happens for a wrapped dataset:
On main this throws
ClassCastException: DatasetGraphSwitchable cannot be cast to DatasetGraphTDB. WithDatasetGraphFactory.createTxnMem()in place of TDB2 it throwsARQNotImplemented: execute/OpDatasetNames. With this change both print( ?g = <http://example/g> ).Changes:
OpExecutornow evaluatesOpDatasetNames. A concrete graph name passes the input bindings through when the graph exists and yields nothing otherwise. A variable is bound to each graph name in the dataset, or checked against the dataset if an earlier part of the query already bound it.OpExecutorTDB2unwraps the execution dataset withTDBInternal.getDatasetGraphTDBand falls back to the general implementation when the dataset is not TDB2 storage (for exampleDatasetGraphWrapperorDatasetGraphFilteredView). This mirrors howQueryEngineFactoryTDBalready unwraps before building its engine.SolverLibTDB.graphNameskeeps the incoming bindings instead of replacing them, and checks an already-bound graph variable against the quads table. Before,{ ?x :p ?s . GRAPH ?g { } }on TDB2 dropped?sfrom the results.Tests:
TestOpDatasetNames(added toTS_Engine): empty dataset, unbound and pre-bound graph variables, a literal bound to the graph variable, concrete graph names, wrapped and filtered datasets, and preservation of unrelated input bindings.Test_SPARQL_TDB: the same cases on TDB2 through the quad engine (switchable, wrapped and filtered datasets), plus a binding-preservation test throughQueryEngineTDB.ARQNotImplemented,ClassCastException, or lost bindings).Verified with
mvn test -pl jena-arq,jena-tdb2 -am -Dtest=TS_Engine,TS_Store,TS_SolverTDB(294 tests, 0 failures) on Java 21, and a fullmvn installbuild.This change was written with help from an AI coding assistant. I reviewed and tested it and take responsibility for it.
By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.
See the Apache Jena "Contributing" guide.