Skip to content

Commit b313fde

Browse files
huangyiireneclaude
andauthored
fix(driver-memory): render the pipeline dump's RegExp operand instead of dropping it to {} (#7853) (#7869)
`AnalyticsResult.sql` from `generateSqlFromPipeline()` dumped each mingo stage with a bare `JSON.stringify`. A `RegExp` has no own enumerable properties, so every pattern operand rendered as `{}` — `{"name":{"$regex":{}}}` — and the one field an author debugging an in-memory chart is looking for was the one the dump dropped. Measured across the twelve operators this face declares: exactly three carry a pattern (`$contains`, `$icontains`, `$notContains` inside `$not`) and all three were affected. They now render the pattern's own literal syntax, `/source/flags` — chosen over the mongo-shaped `{$regex, $options}` because the `RegExp` sits AT the `$regex` key, so a value replacer producing the pair renders the doubled `{"$regex":{"$regex":"et","$options":""}}`, a shape no mongo query has, and flattening it would make the dump disagree with the pipeline it dumps. No executed behaviour changes: the dump is explicitly not SQL, and `query()`'s rows, `generateSql()`'s SQL, and the other nine operators' dumps are unchanged. The existing dump check asserted structure (`$not` wraps a `$regex`) and passed throughout — it is strengthened here to assert the pattern TEXT for each affected operator, since `{"$regex":{}}` is non-empty and satisfies any presence-only assertion. Claude-Session: https://claude.ai/code/session_01UzMdsWW2aZwGCEhfDLo1sT Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0410522 commit b313fde

3 files changed

Lines changed: 156 additions & 6 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@objectstack/driver-memory": patch
3+
---
4+
5+
fix(driver-memory): the analytics pipeline dump shows its RegExp pattern instead of `{}` (#7853)
6+
7+
`MemoryAnalyticsService.query()` returns `AnalyticsResult.sql` — a stage-by-stage
8+
dump of the mingo pipeline it actually executed, and the only thing an author
9+
debugging an in-memory chart is given. It dumped each stage with a bare
10+
`JSON.stringify`, and a `RegExp` has **no own enumerable properties**, so every
11+
pattern operand rendered as `{}`:
12+
13+
```
14+
-- MongoDB Aggregation Pipeline on table: deal
15+
/* Stage 1: $match */ {"name":{"$regex":{}}}
16+
```
17+
18+
The `$match` stage was reported as constraining `name` by an empty object. The
19+
one field the reader came for is the one the dump dropped. Measured across the
20+
twelve operators this face declares, exactly three carry a pattern and all three
21+
were affected: `$contains`, `$icontains`, and `$notContains` (nested inside
22+
`$not`). The same three now render:
23+
24+
```
25+
/* Stage 1: $match */ {"name":{"$regex":"/et/"}}
26+
/* Stage 1: $match */ {"name":{"$regex":"/[Bb][Ee][Tt]/"}}
27+
/* Stage 1: $match */ {"name":{"$not":{"$regex":"/et/"}}}
28+
```
29+
30+
**No executed behaviour changes.** This dump is explicitly not SQL — its own
31+
header says `-- MongoDB Aggregation Pipeline on table: …` — so it is a
32+
transparency surface, not a runnable one, and the rows `query()` returns and the
33+
SQL `generateSql()` emits are byte-identical before and after. The other nine
34+
operators' dumps are unchanged.
35+
36+
**Why the pattern's own literal syntax** (`/source/flags`) and not the
37+
mongo-shaped `{"$regex":"…","$options":"…"}` the rest of the dump speaks: the
38+
`RegExp` sits AT the `$regex` key, so a value replacer producing the mongo pair
39+
renders the doubled `{"$regex":{"$regex":"et","$options":""}}` — a shape no mongo
40+
query has. Flattening it to the real spelling would mean rewriting the parent
41+
object, making the dump disagree with the pipeline it claims to dump, since what
42+
mingo executes at that key is a JS `RegExp`. The literal form is also the only
43+
one-token rendering that keeps the FLAGS, which matter here: `$icontains`' fold
44+
lives in the pattern source (#6520) while `$contains` is case-exact (#7723).

packages/drivers/driver-memory/src/memory-analytics.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,62 @@ function sizeDistinctSet(values: readonly unknown[]): number {
480480
return new Set(values.filter((v) => v !== null && v !== undefined)).size;
481481
}
482482

483+
/**
484+
* [#7853] A `JSON.stringify` replacer that renders a `RegExp` operand instead of
485+
* dropping it — the one value type the pipeline dump carries that
486+
* `JSON.stringify` erases.
487+
*
488+
* ## What was lost
489+
*
490+
* A `RegExp` has no own enumerable properties, so `JSON.stringify` renders it as
491+
* `{}`. Three operators put one into the `$match` stage — `contains` and
492+
* `icontains` as `{$regex: …}`, `notContains` as `{$not: {$regex: …}}` (measured:
493+
* those three and no others, out of the twelve this face declares) — so
494+
* `{name: {$contains: 'Industries'}}` dumped as
495+
*
496+
* ```
497+
* /* Stage 1: $match *\/ {"name":{"$regex":{}}}
498+
* ```
499+
*
500+
* The one field an author debugging a chart is looking for is the one the dump
501+
* dropped. This is lost information on a transparency surface, not the #5333
502+
* class: the dump is explicitly NOT SQL (its header says so) and `{}` reads as
503+
* "something is missing here" rather than as a working predicate, which is why
504+
* it is graded below #7117 rather than beside it.
505+
*
506+
* ## Why the pattern's own literal syntax and not `{"$regex":"…","$options":"…"}`
507+
*
508+
* The mongo-shaped form is what the rest of the dump speaks, and it was the
509+
* first candidate. It cannot be reached from a value replacer, and the reason is
510+
* structural rather than cosmetic: the `RegExp` sits AT the `$regex` key, so
511+
* replacing it with `{$regex, $options}` renders the doubled
512+
* `{"name":{"$regex":{"$regex":"Industries","$options":""}}}` — a shape no mongo
513+
* query has. Flattening it into the real mongo spelling means rewriting the
514+
* PARENT object, which would make the dump disagree with the pipeline it claims
515+
* to be dumping: what mingo executes is a JS `RegExp` object at that key, not a
516+
* source/options pair. Trading a degenerate rendering for a plausible-but-wrong
517+
* one is the #5333 direction, and this card is explicitly not that.
518+
*
519+
* So the value is rendered as the JS literal it is, `/source/flags`, which is
520+
* also the only one-token form that keeps the FLAGS. Flags are not decoration
521+
* here: `$icontains`' fold lives in the pattern SOURCE (#6520) while `$contains`
522+
* is case-EXACT (#7723, #4706 Q2 = A), so a rendering that dropped `i` would
523+
* recreate a smaller copy of this same information loss on the one axis those
524+
* two operators differ.
525+
*
526+
* ## What it deliberately does not touch
527+
*
528+
* Every other value on this path already renders faithfully, measured rather
529+
* than assumed: a `Date` comparand is canonicalized to an ISO string by
530+
* {@link MemoryAnalyticsService.comparandsFor} before it reaches here, and
531+
* `toJSON` runs BEFORE a replacer in any case, so dates are unchanged. A
532+
* `BigInt` comparand does throw — but out of mingo's own `Query.compile` during
533+
* EXECUTION, before this dump is ever built, so no replacer here reaches it.
534+
*/
535+
function pipelineDumpReplacer(_key: string, value: unknown): unknown {
536+
return value instanceof RegExp ? `/${value.source}/${value.flags}` : value;
537+
}
538+
483539
/**
484540
* Configuration for MemoryAnalyticsService
485541
*/
@@ -1280,9 +1336,13 @@ export class MemoryAnalyticsService implements IAnalyticsService {
12801336
private generateSqlFromPipeline(table: string, pipeline: Record<string, any>[]): string {
12811337
// Simplified SQL generation for debugging
12821338
// This is a basic representation of the aggregation pipeline
1339+
//
1340+
// [#7853] The replacer is what keeps a `RegExp` operand from rendering as
1341+
// `{}` — see {@link pipelineDumpReplacer} for why the pattern's own literal
1342+
// syntax and not the mongo-shaped `{$regex, $options}`.
12831343
const stages = pipeline.map((stage, idx) => {
12841344
const op = Object.keys(stage)[0];
1285-
return `/* Stage ${idx + 1}: ${op} */ ${JSON.stringify(stage[op])}`;
1345+
return `/* Stage ${idx + 1}: ${op} */ ${JSON.stringify(stage[op], pipelineDumpReplacer)}`;
12861346
}).join('\n');
12871347

12881348
return `-- MongoDB Aggregation Pipeline on table: ${table}\n${stages}`;

packages/drivers/driver-memory/src/memory-driver-filter-logic-conformance.test.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -696,17 +696,63 @@ describe('[#5374] operator semantics — the analytics face against the live que
696696
* absent and the rows happen to line up", and the emitted `$match` is the
697697
* artifact the issue actually diagnosed.
698698
*/
699-
it('the emitted $match wraps the negation around a pattern instead of a bare scalar', async () => {
699+
const matchStageOf = async (where: FilterCondition): Promise<string> => {
700700
const { sql } = await service.query({
701701
cube: COMPARAND_TABLE,
702702
measures: [`${COMPARAND_TABLE}.count`],
703-
where: { name: { $notContains: 'et' } } as FilterCondition,
703+
where,
704704
});
705-
const matchStage = /\/\* Stage 1: \$match \*\/ (.*)/.exec(sql ?? '')?.[1] ?? '';
706-
// `JSON.stringify` renders a RegExp as `{}`, so assert on the STRUCTURE the
707-
// pipeline carries rather than on that rendering.
705+
return /\/\* Stage 1: \$match \*\/ (.*)/.exec(sql ?? '')?.[1] ?? '';
706+
};
707+
708+
it('the emitted $match wraps the negation around a pattern instead of a bare scalar', async () => {
709+
const matchStage = await matchStageOf({ name: { $notContains: 'et' } } as FilterCondition);
708710
expect(matchStage).not.toBe('{"name":{"$not":"et"}}');
709711
expect(matchStage).toContain('"$not"');
710712
expect(matchStage).toContain('"$regex"');
711713
});
714+
715+
/**
716+
* [#7853] The dump's CONTENT for the pattern operators, not merely its shape.
717+
*
718+
* The assertion above pins the structure `$not` wraps, and it passed for as
719+
* long as the pattern itself was missing: `JSON.stringify` renders a `RegExp`
720+
* as `{}` (no own enumerable properties), so `{name: {$contains: 'et'}}`
721+
* dumped as `{"name":{"$regex":{}}}` — non-empty, structurally correct, and
722+
* silent about the one field an author reading this dump came for. That is
723+
* why the cases below assert the pattern TEXT: a `toBeDefined()` or a
724+
* `toContain('$regex')` cannot tell the two states apart.
725+
*
726+
* The rendering is the pattern's own literal syntax, `/source/flags`. No
727+
* operator this face declares carries a RegExp FLAG today — measured:
728+
* `$icontains`' fold is compiled into the pattern source (#6520) and
729+
* `$contains` is case-exact (#7723) — so the flags segment renders empty
730+
* here; it is in the form because a fold that ever moved into a flag would
731+
* otherwise vanish exactly the way the source did.
732+
*/
733+
const PATTERN_DUMP_CASES: Array<[string, FilterCondition, string]> = [
734+
// The pattern, plainly, for the operator the issue measured.
735+
['$contains', { name: { $contains: 'et' } } as FilterCondition, '{"name":{"$regex":"/et/"}}'],
736+
// The ASCII fold is IN the source, so the dump shows the folded character
737+
// classes rather than an `i` — this is what #6520 compiled, made visible.
738+
['$icontains', { name: { $icontains: 'BET' } } as FilterCondition, '{"name":{"$regex":"/[Bb][Ee][Tt]/"}}'],
739+
// The negation still wraps a pattern, and now the pattern is legible.
740+
['$notContains', { name: { $notContains: 'et' } } as FilterCondition, '{"name":{"$not":{"$regex":"/et/"}}}'],
741+
// A comparand carrying a regex metacharacter shows its ESCAPE. `a.p` is a
742+
// literal here, not "any character between a and p" (#5567's direction), and
743+
// the dump is the only place an author can see which of the two ran.
744+
['$contains with a metacharacter', { name: { $contains: 'a.p' } } as FilterCondition, '{"name":{"$regex":"/a\\\\.p/"}}'],
745+
];
746+
747+
for (const [label, where, expected] of PATTERN_DUMP_CASES) {
748+
it(`the pipeline dump renders ${label}'s pattern instead of dropping it to {}`, async () => {
749+
const matchStage = await matchStageOf(where);
750+
expect(
751+
matchStage,
752+
`${label}: the dump lost its RegExp operand — this is the \`{"$regex":{}}\` state, ` +
753+
'which is non-empty and passes every assertion that only checks for presence',
754+
).not.toContain('{}');
755+
expect(matchStage).toBe(expected);
756+
});
757+
}
712758
});

0 commit comments

Comments
 (0)