Skip to content

ColumnPredicate.test() returns the negation of "matches" — inverts by-IDs column filtering #195

@alexander-yevsyukov

Description

@alexander-yevsyukov

Summary

io.spine.server.storage.datastore.query.ColumnPredicate — a Predicate<Entity>
documented as "tests if a Datastore Entity matches the parameters defined by the query
subject"
— returns the exact opposite: a matching entity yields false, a
non-matching entity yields true.

Evidence

A minimal unit probe (StgProject, FilterAdapter.of(new DsColumnMapping()), in-memory
Entity):

  • query idString == "42", entity idString = "42" (matches) → test() = false (expected true)
  • query idString == "42", entity idString = "99" (no match) → test() = true (expected false)

Impact

DsLookupByIds (the lookup-by-IDs read path with column filters) consumes the predicate as
stream.filter(predicate)
(datastore/src/main/java/io/spine/server/storage/datastore/query/DsLookupByIds.java:97).
Because test() is inverted, the filter keeps the records that do not satisfy the column
parameters and drops those that do
— wrong results for any by-IDs query that carries a
column predicate. The class currently has 0% test coverage (found while migrating the
build to Kover and localizing :datastore gaps), which is why it went unnoticed.

Root cause & proposed fix

ColumnPredicate.testPredicate(...) ends with return !match;
(ColumnPredicate.java:100). The checkAnd / checkOr helpers already compute "entity
matches this predicate" (true = matches), and the children-recursion helpers
(checkAndChildren uses if (!matches) return true, checkOrChildren uses
if (matches) return true) are written assuming testPredicate returns "matches". The stray
! inverts the whole result.

Fix: change line 100 to return match;. Verified by tracing every branch — leaf AND,
leaf OR, nested AND-containing-OR, and OR-containing-AND: with return match the
documented contract holds in all of them.

Tests to add alongside the fix

A ColumnPredicateSpec (Kotlin + Kotest, same package) asserting the documented contract:
null entity → false; AND match / no-match / missing-column; OR match / no-match /
empty-params-plus-child; nested. (Drafted during the coverage work; held back because the
production fix needs its own review.)

Notes

  • The default: branch of testPredicate (unknown LogicalOperator) is unreachable from the
    public query API — io.spine.query.LogicalOperator has only AND and OR — so it is
    non-actionable for coverage.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No fields configured for Bug.

    Projects

    Status
    📋 Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions