Regression: Queries with HAVING clause throw AssertionError in Var because parent is already set

[jetztgradnet]

Current Behavior

When parsing the following query with RDF4J 4.1.1 (worked with 4.1.0 and 4.0.0) I get an AssertionError:

SELECT ?this
WHERE {
	?this <http://example.org/nestedProperty> ?value .
}
GROUP BY ?this
HAVING ( ( count(?value)  < 2 ) && ( count(?value)  != 0 ) )

Removing the HAVING clauses makes it parse, so the issue must be somewhere in there. I assume one of the recent optimizations are missing one of the Expr.clone() calls or so?

Stacktrace:

java.lang.AssertionError: Unexpected assertion
	at com.metaphacts.rdf4j.RDF4JTest.testVar(RDF4JTest.java:20)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:93)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:40)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:529)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:756)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:452)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
Caused by: java.lang.AssertionError
	at org.eclipse.rdf4j.query.algebra.Var.setParentNode(Var.java:112)
	at org.eclipse.rdf4j.query.algebra.BinaryValueOperator.setLeftArg(BinaryValueOperator.java:70)
	at org.eclipse.rdf4j.query.algebra.BinaryValueOperator.replaceChildNode(BinaryValueOperator.java:103)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder$AggregateOperatorReplacer.meetAggregate(TupleExprBuilder.java:2616)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder$AggregateOperatorReplacer.meet(TupleExprBuilder.java:2581)
	at org.eclipse.rdf4j.query.algebra.Count.visit(Count.java:29)
	at org.eclipse.rdf4j.query.algebra.BinaryValueOperator.visitChildren(BinaryValueOperator.java:96)
	at org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor.meetNode(AbstractQueryModelVisitor.java:576)
	at org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor.meetBinaryValueOperator(AbstractQueryModelVisitor.java:545)
	at org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor.meet(AbstractQueryModelVisitor.java:158)
	at org.eclipse.rdf4j.query.algebra.Compare.visit(Compare.java:89)
	at org.eclipse.rdf4j.query.algebra.BinaryValueOperator.visitChildren(BinaryValueOperator.java:97)
	at org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor.meetNode(AbstractQueryModelVisitor.java:576)
	at org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor.meetBinaryValueOperator(AbstractQueryModelVisitor.java:545)
	at org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor.meetOther(AbstractQueryModelVisitor.java:520)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder.processHavingClause(TupleExprBuilder.java:444)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder.visit(TupleExprBuilder.java:374)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder.visit(TupleExprBuilder.java:237)
	at org.eclipse.rdf4j.query.parser.sparql.ast.ASTSelectQuery.jjtAccept(ASTSelectQuery.java:27)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder.visit(TupleExprBuilder.java:346)
	at org.eclipse.rdf4j.query.parser.sparql.TupleExprBuilder.visit(TupleExprBuilder.java:237)
	at org.eclipse.rdf4j.query.parser.sparql.ast.ASTQueryContainer.jjtAccept(ASTQueryContainer.java:29)
	at org.eclipse.rdf4j.query.parser.sparql.SPARQLParser.buildQueryModel(SPARQLParser.java:246)
	at org.eclipse.rdf4j.query.parser.sparql.SPARQLParser.parseQuery(SPARQLParser.java:206)
	at org.aksw.rdfunit.validate.integration.RDF4JTest.testVar(RDF4JTest.java:17)
	... 26 more

Expected Behavior

The query can be parsed without errors.

Steps To Reproduce

This unit test reproduces the behavior:

Removing the HAVING clauses makes it pass, so the issue must be somewhere in there.

package com.metaphacts.rdf4j;

import org.eclipse.rdf4j.query.parser.sparql.SPARQLParser;
import org.junit.Test;

public class RDF4JTest {
	@Test
	public void testVar() throws Exception {
		String queryString = "SELECT ?this\n"
				+ "WHERE {\n"
				+ "		?this <http://example.org/nestedProperty> ?value .\n"
				+ "}\n"
				+ "GROUP BY ?this\n"
				+ "HAVING ( ( count(?value)  < 2 ) && ( count(?value)  != 0 ) )";
		SPARQLParser parser = new SPARQLParser();
		try {
			parser.parseQuery(queryString, null);
		}
		catch (AssertionError e) {
			throw new AssertionError("Unexpected assertion", e);
		}
	}
}

Version

4.1.1

Are you interested in contributing a solution yourself?

No response

Anything else?

No response

[hmottestad]

Thanks for finding this.

It might not be a bug per se. I added the assertions to help detect reused Var objects since this has been the root cause of some bugs that were particularly hard to debug.

[jetztgradnet]

Maybe it's not a bug, but it makes it unusable for us. Or we need to disable assertions, which might also be an option, but that requirs and extra configuration step and will need to be done by anybody using HAVING clauses with 4.1.1...

Ideally we'd have a 4.1.2 soon which avoids triggering this assert. Any chance for that? We have a release of our software planned for end of this month, so getting an update a bit before then would be great!

[hmottestad]

I'll see what I can do :)

In the mean time you should be able to disable just that assertion without affecting other assertions. https://stackoverflow.com/questions/28901375/how-do-i-disable-java-assertions-for-a-junit-test-in-the-code

Assertions aren't enabled by default in java, so if your product is a standalone application then it shouldn't be affected I would hope.

[hmottestad]

@jetztgradnet Could you take a look at my PR?

[jetztgradnet]

@hmottestad That was quick! I will have a look together with @aschwarte10.

[jetztgradnet]

Quick feedback regarding disabling asserts:

adding this to my unit tests makes them pass again:

// ...
import org.eclipse.rdf4j.query.algebra.Var;
// ...

	@Before
	public void disableAssertions() {
		Var.class.getClassLoader().setClassAssertionStatus(Var.class.getName(), false);
	}

I will now test this PR (building it locally and replacing some of my jars)

[jetztgradnet]

With 4.1.2 this is fixed now.
I found a related issue though in 4.1.2 which might be caused by further optimizations (or was there also previously, not sure): #4189