Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/lowered-operator-key-clobber.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
---
'@objectstack/driver-memory': patch
'@objectstack/driver-mongodb': patch
---

Stop a field operator whose lowering reuses another operator's key from silently clobbering it.

Both document-shaped drivers translated a field constraint by writing every lowered key into one object literal. Several authorable operators do not lower to a key of their own name — `$null` writes `$eq`/`$ne`, `$between` writes `$gte` plus `$lte`/`$lt`, `$lte` on a bare calendar day writes `$lt`, and MongoDB's `$contains`/`$startsWith`/`$endsWith`/`$icontains` all write `$regex` — so two constraints on one field landed on one key and the second assignment won. One constraint disappeared with no error and no trace in the emitted query, and which one disappeared was decided by the author's key order. On a row-level-security read scope, a dropped constraint is a widened one.

A lowered write whose key is free now merges inline as before; a write whose key is already taken becomes its own `$and` branch, where both constraints survive. Which write keeps the inline slot is decided by the spec's declared operator order rather than by the author's key order, so one predicate emits the same query however it is spelled. `driver-memory`'s analytics (cube) face carried a wider form of the same defect — its `$match` was keyed by field path, so a second predicate on a member replaced the first entirely, for every operator pair — and is promoted the same way.

Filters with no contested key are unchanged.
33 changes: 32 additions & 1 deletion packages/drivers/driver-memory/src/memory-analytics.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -675,6 +675,11 @@ export class MemoryAnalyticsService implements IAnalyticsService {
const normalizedFilters = this.normalizeFilters(query);
if (normalizedFilters.length > 0) {
const matchStage: Record<string, any> = {};
/**
* [#13524] Predicates for a member that already has one — see the
* promotion below the loop for why they cannot be assigned.
*/
const contested: Record<string, any>[] = [];
for (const filter of normalizedFilters) {
const fieldPath = this.resolveFieldPath(cube, filter.member);
// [#5374] The operator decides the WHOLE predicate, not just its name —
Expand All@@ -687,15 +692,41 @@ export class MemoryAnalyticsService implements IAnalyticsService {
// FROM: a boolean reaches mingo as a boolean and `null` as `null`, so a
// predicate over `is_active` or `closed_at` selects the same rows
// `find()` selects instead of none / all of them.
matchStage[fieldPath] = this.mongoPredicateBuilder(filter.operator)({
const predicate = this.mongoPredicateBuilder(filter.operator)({
comparands: this.comparandsFor(cube, filter.member, filter.values),
raw: filter.values,
substring: (value) => this.driver.filterSubstringPattern(value),
// [#6520] `$icontains`' fold, from the spec's shared definition rather
// than from the driver's Unicode-folding `filterSubstringPattern`.
asciiSubstring: (value) => new RegExp(asciiCaseInsensitiveRegexSource(String(value))),
});
// [#13524] This was `matchStage[fieldPath] = …`, and the assignment was
// a WHOLESALE clobber — the widest member of this card's class. The two
// document-shaped translators lose a constraint only when two operators
// happen to lower onto the SAME key; here the stage is keyed by field
// path alone, so the second predicate on a member replaced the first
// ENTIRELY, for every operator pair. And `flattenFilterCondition` folds
// `$and` into this same flat list, so `{$and: [{name: {$contains:'a'}},
// {name: {$ne:'b'}}]}` — two separate nodes, not one operator map —
// lost a constraint too. Measured on a three-row fixture:
// `{name: {$contains:'a', $ne:'b'}}` aggregated ['1','3'] and its
// key-swapped twin ['1'], while the reference matcher said ['1'].
//
// Same rule as the translators: free member merges inline, a taken one
// becomes its own `$and` branch of the SAME `$match`, where both
// predicates survive. No ranking is needed here — unlike a contested
// operator key, nothing is overwritten, so which predicate sits inline
// changes the document's shape but never its answer.
if (Object.prototype.hasOwnProperty.call(matchStage, fieldPath)) {
contested.push({ [fieldPath]: predicate });
} else {
matchStage[fieldPath] = predicate;
}
}
// A field path can never BE `$and` — `resolveFieldPath` resolves cube
// members, and `flattenFilterCondition` refuses `$or` / `$not` and folds
// `$and` away before this runs — so this cannot collide with a member.
if (contested.length > 0) matchStage.$and = contested;
if (Object.keys(matchStage).length > 0) {
pipeline.push({ $match: matchStage });
}
Expand Down
Loading
Loading