Skip to content

Commit 433cddc

Browse files
GH-5310 Further distinguish between the count cases and the other
aggregate functions. Also do fix the logging issue in the ConstantOptimizer.
1 parent 00c53d3 commit 433cddc

4 files changed

Lines changed: 82 additions & 1 deletion

File tree

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/GroupIterator.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,11 @@ private List<Entry> emptySolutionSpecialCase(List<AggregatePredicateCollectorSup
322322
for (var ag : aggregates) {
323323
if (ag.agg instanceof WildCardCountAggregate) {
324324
predicates.add(ALWAYS_TRUE_BINDING_SET);
325+
} else if (ag.agg instanceof CountAggregate) {
326+
// Counts are special, because they always return a number related to the number of solutions.
327+
// which in the empty case should be 0. So we should never accept a value here.
328+
// Even in the case that the Count is of a constant value.
329+
predicates.add(ALWAYS_FALSE_VALUE);
325330
} else {
326331
predicates.add(ALWAYS_TRUE_VALUE);
327332
}
@@ -397,6 +402,7 @@ private void operate(BindingSet bs, Predicate<?> predicate, Object t) {
397402

398403
private static final Predicate<BindingSet> ALWAYS_TRUE_BINDING_SET = t -> true;
399404
private static final Predicate<Value> ALWAYS_TRUE_VALUE = t -> true;
405+
private static final Predicate<Value> ALWAYS_FALSE_VALUE = t -> false;
400406
private static final Supplier<Predicate<Value>> ALWAYS_TRUE_VALUE_SUPPLIER = () -> ALWAYS_TRUE_VALUE;
401407

402408
private AggregatePredicateCollectorSupplier<?, ?> create(GroupElem ge, ValueFactory vf)

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/optimizer/ConstantOptimizer.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.eclipse.rdf4j.query.BindingSet;
2323
import org.eclipse.rdf4j.query.Dataset;
2424
import org.eclipse.rdf4j.query.QueryEvaluationException;
25+
import org.eclipse.rdf4j.query.algebra.AggregateOperator;
2526
import org.eclipse.rdf4j.query.algebra.And;
2627
import org.eclipse.rdf4j.query.algebra.BinaryValueOperator;
2728
import org.eclipse.rdf4j.query.algebra.Bound;
@@ -223,7 +224,11 @@ protected void meetBinaryValueOperator(BinaryValueOperator binaryValueOp) {
223224
protected void meetUnaryValueOperator(UnaryValueOperator unaryValueOp) {
224225
super.meetUnaryValueOperator(unaryValueOp);
225226

226-
if (isConstant(unaryValueOp.getArg())) {
227+
ValueExpr arg = unaryValueOp.getArg();
228+
// This is not the place to optimize aggregates, as they are not
229+
// evaluated in the same way as other unary value operators.
230+
// They must be evaluated in the context of a group.
231+
if (isConstant(arg) && !(unaryValueOp instanceof AggregateOperator)) {
227232
try {
228233
Value value = strategy.precompile(unaryValueOp, context).evaluate(EmptyBindingSet.getInstance());
229234
unaryValueOp.replaceWith(new ValueConstant(value));

core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/ConstantOptimizerTest.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,62 @@ public void testAggregateOptimization() throws RDF4JException {
150150
assertEquals(1, ((Literal) bindings.getBinding("a").getValue()).intValue());
151151
}
152152

153+
@Test
154+
public void testAllAggregateOptimizations() throws RDF4JException {
155+
String query = String.join("\n",
156+
"PREFIX ex: <ex:>",
157+
"SELECT",
158+
" (MAX(1) AS ?a)",
159+
" (MIN(1) AS ?b)",
160+
" (AVG(1) AS ?c)",
161+
" (COUNT(1) AS ?d)",
162+
" (COUNT(DISTINCT 1) AS ?e)",
163+
" (COUNT(*) AS ?f)",
164+
"WHERE {",
165+
" ?x a ?z ;",
166+
" ex:someProperty ?val .",
167+
"}"
168+
);
169+
170+
ParsedQuery pq = QueryParserUtil.parseQuery(QueryLanguage.SPARQL, query, null);
171+
EvaluationStrategy strategy = new DefaultEvaluationStrategy(new EmptyTripleSource(), null);
172+
TupleExpr original = pq.getTupleExpr();
173+
174+
final AlgebraFinder finder = new AlgebraFinder();
175+
original.visit(finder);
176+
assertTrue(finder.groupElemFound);
177+
178+
// reset for re-use on optimized query
179+
finder.reset();
180+
181+
QueryBindingSet constants = new QueryBindingSet();
182+
183+
TupleExpr optimized = optimize(pq.getTupleExpr().clone(), constants, strategy);
184+
185+
optimized.visit(finder);
186+
assertThat(finder.functionCallFound).isFalse();
187+
188+
CloseableIteration<BindingSet> result = strategy.precompile(optimized)
189+
.evaluate(
190+
new EmptyBindingSet());
191+
assertNotNull(result);
192+
assertTrue(result.hasNext());
193+
194+
BindingSet bindings = result.next();
195+
assertTrue(bindings.hasBinding("a"));
196+
assertTrue(bindings.hasBinding("b"));
197+
assertTrue(bindings.hasBinding("c"));
198+
assertTrue(bindings.hasBinding("d"));
199+
assertTrue(bindings.hasBinding("e"));
200+
assertTrue(bindings.hasBinding("f"));
201+
assertEquals(1, ((Literal) bindings.getBinding("a").getValue()).intValue());
202+
assertEquals(1, ((Literal) bindings.getBinding("b").getValue()).intValue());
203+
assertEquals(1, ((Literal) bindings.getBinding("c").getValue()).intValue());
204+
assertEquals(0, ((Literal) bindings.getBinding("d").getValue()).intValue());
205+
assertEquals(0, ((Literal) bindings.getBinding("e").getValue()).intValue());
206+
assertEquals(0, ((Literal) bindings.getBinding("f").getValue()).intValue());
207+
}
208+
153209
private class AlgebraFinder extends AbstractQueryModelVisitor<RuntimeException> {
154210

155211
public boolean logicalAndfound = false;

core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/GroupIteratorTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.assertj.core.api.Assertions.assertThat;
1414
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
1515
import static org.junit.jupiter.api.Assertions.assertEquals;
16+
import static org.junit.jupiter.api.Assertions.assertTrue;
1617

1718
import java.time.Duration;
1819
import java.time.Instant;
@@ -120,6 +121,19 @@ public void testMaxEmptySet_DefaultGroup() throws QueryEvaluationException {
120121
}
121122
}
122123

124+
@Test
125+
public void testConstantCountEmptySet_DefaultGroup() throws QueryEvaluationException {
126+
Group group = new Group(EMPTY_ASSIGNMENT);
127+
group.addGroupElement(new GroupElem("count", new Count(new ValueConstant(VF.createLiteral("a")))));
128+
try (GroupIterator gi = new GroupIterator(EVALUATOR, group, EmptyBindingSet.getInstance(), CONTEXT)) {
129+
130+
assertTrue(gi.hasNext());
131+
BindingSet next = gi.next();
132+
assertEquals(1, next.size());
133+
assertEquals(0, ((Literal) next.getBinding("count").getValue()).intValue());
134+
}
135+
}
136+
123137
@Test
124138
public void testMaxSet_DefaultGroup() throws QueryEvaluationException {
125139
Group group = new Group(NONEMPTY_ASSIGNMENT);

0 commit comments

Comments
 (0)