Skip to content

OpenCypherCountOptimizationTest cannot detect the count optimization silently regressing #6280

Description

@lvca

Found while sweeping test assertions for #6260.

OpenCypherCountOptimizationTest verifies that MATCH (a:Account) RETURN COUNT(a) returns the right number. It does not, and currently cannot, verify that the count optimization is what produced it. Six plan assertions across three tests are commented out behind the same TODO:

// TODO: Verify optimization is being used by checking EXPLAIN output
// EXPLAIN integration needs to be added separately
// final ResultSet explainResult = database.query("opencypher", "EXPLAIN " + query);
// assertThat(plan).contains("TYPE COUNT OPTIMIZATION");

Two of the six are the negative cases (doesNotContain("TYPE COUNT OPTIMIZATION")), which matter just as much: they are what stops the optimization firing where it is not valid.

This is a weaker finding than #6279 - these tests do assert correct results, so they are not vacuous - but the gap is real and it is in the direction that hurts. If the push-down stopped firing tomorrow, every one of these tests would still pass, because a full type scan returns the same count as the optimized path. It just returns it slower, and slowly is not something this suite checks. Given the count push-down has already been worked on twice (#5686 uncorrelated COUNT push-down, #5715 push-down preconditions, where a LIGHTWEIGHT edge yields countType 0), a silent regression here is not hypothetical.

What to do

The TODO says EXPLAIN integration "needs to be added separately" - that is the actual blocker, and it is worth checking whether it still is. EXPLAIN works on the SQL side, so the question is only whether the openCypher engine routes it and surfaces a plan string.

  • If EXPLAIN now works for openCypher: uncomment the six assertions, confirm the plan marker string still matches what the optimizer emits, and delete the TODO.
  • If it does not: assert on something else observable rather than leaving the tests half-blind. A counter on the optimizer, or the absence of a bucket scan in the profile, both discriminate the optimized path from the full scan without needing EXPLAIN. Note the bound must not be a wall-clock one - "the optimized path is faster" is precisely the assertion shape Wall-clock assertions in the engine suite go red on the shared JVM's stop-the-world pauses, not on the behaviour they test #6260 was filed about, and StallAwareStopwatch.assertStayedUnder is the floor to build on if there is genuinely no structural signal available.

Related: #5686, #5715.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions