Skip to content

Commit 95b7278

Browse files
committed
fix(service-analytics): the compile leg's words for a count_distinct refusal, and the census pin the narrowed table moves (#20808)
Claude-Session: https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY Co-authored-by: Claude <noreply@anthropic.com>
1 parent d2a6058 commit 95b7278

3 files changed

Lines changed: 62 additions & 13 deletions

File tree

‎.changeset/20808-json-stored-group-distinct-refused.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"@objectstack/objectql": minor
33
"@objectstack/spec": minor
44
"@objectstack/lint": patch
5+
"@objectstack/service-analytics": patch
56
---
67

78
fix(objectql,spec)!: a `groupBy` on a multi-value field and a `count_distinct` on a JSON-stored field are refused with `INVALID_FIELD` / 400 at the engine's `aggregate`, on every driver, and the aggregate × field-type table stops accepting `count_distinct` over the JSON-stored types
@@ -28,3 +29,5 @@ Clause-②: no (narrowing)
2829
**Unchanged.** A `groupBy` or `count_distinct` on a scalar-stored field, a single-value `select` or `lookup` included; `count` over any field; the `having`, filter and sort positions; and an undeclared name, which the REST door answers `INVALID_FIELD` as unknown before the engine is reached.
2930

3031
`@objectstack/lint`: the dataset-measure refusal's hint no longer says `count_distinct` accepts every type.
32+
33+
`@objectstack/service-analytics`: the dataset compile leg's refusal of a `count_distinct` measure over a JSON-stored field says why it diverges (the drivers compare the values for equality three ways) and prescribes `count`, or a scalar field for the part being counted; its other refusals no longer say `count_distinct` accepts every type.

‎packages/services/service-analytics/src/__tests__/aggregate-nontemporal-measure-refusal.test.ts‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,29 +157,31 @@ describe('#16099 — the pairs this leg refuses are the TABLE\'s, not this packa
157157
expect(isAggregateCompatibleWithFieldType('avg', 'percent')).toBe(true);
158158
});
159159

160-
it('⭐ the refused set, enumerated by the card that enforced each part — 155 pairs, 0 left over', () => {
160+
it('⭐ the refused set, enumerated by the card that enforced each part — 165 pairs, 0 left over', () => {
161161
// The arithmetic the PR bodies show, asserted rather than narrated, so a
162162
// row moving upstream moves this count instead of leaving a stale claim.
163163
// ⚠️ [#17560] The `minmaxString` / `selecting` split this case used to carry
164164
// is retired with the reading behind it: the 42 string pairs were counted
165165
// apart because they were "ruled to be AMENDED" under #17513, and decision
166166
// batch #127 found no ruling behind that and declined to amend the table.
167167
// The 42 and the 32 are one population again, enforced in one pass.
168-
let refusedByTable = 0, temporal = 0, deriving = 0, selecting = 0;
168+
let refusedByTable = 0, temporal = 0, deriving = 0, selecting = 0, distinct = 0;
169169
for (const a of Object.keys(AGGREGATE_FIELD_TYPE_COMPATIBILITY)) {
170170
for (const ft of FieldType.options) {
171171
if (isAggregateCompatibleWithFieldType(a, ft)) continue;
172172
refusedByTable++;
173173
if (a === 'min' || a === 'max') { selecting++; continue; }
174+
if (a === 'count_distinct') { distinct++; continue; }
174175
if (TEMPORAL_SOURCE_FIELD_TYPES.has(ft)) { temporal++; continue; }
175176
deriving++;
176177
}
177178
}
178-
expect(refusedByTable).toBe(155);
179+
expect(refusedByTable).toBe(165);
179180
expect(temporal).toBe(6); // commit 357f4992b's — `sum`/`avg` over the temporal class
180181
expect(deriving).toBe(75); // #16099's — `sum`/`avg` over everything else
181182
expect(selecting).toBe(74); // #17560's — `min`/`max`, 42 string + 32 non-string
182-
expect(temporal + deriving + selecting).toBe(refusedByTable);
183+
expect(distinct).toBe(10); // #20808's — `count_distinct` over the JSON-stored types
184+
expect(temporal + deriving + selecting + distinct).toBe(refusedByTable);
183185
// ⭐ And nothing is left declared-but-unenforced: one door judges all six.
184186
expect(Object.keys(AGGREGATE_FIELD_TYPE_COMPATIBILITY).length).toBe(6);
185187
});
@@ -326,7 +328,7 @@ describe('#16099 — the controls: every pair the table accepts still compiles',
326328
expect(err.message).not.toContain('derives a NUMBER');
327329
});
328330

329-
it('`count` / `count_distinct` accept every type — they read no arithmetic off the value', async () => {
331+
it('`count` / `count_distinct` over a scalar-stored field compile — they read no arithmetic off the value', async () => {
330332
for (const aggregate of ['count', 'count_distinct'] as const) {
331333
const { go, sqls } = run(aggregate, 'note');
332334
const result: any = await go();
@@ -335,6 +337,30 @@ describe('#16099 — the controls: every pair the table accepts still compiles',
335337
}
336338
});
337339

340+
// [#20808] The table's `count_distinct` row stops accepting the JSON-stored
341+
// types, and this door judges every row: the refusal says why a distinct
342+
// count diverges there (EQUALITY, not arithmetic or order) and never says
343+
// `count_distinct` accepts every type.
344+
it('[#20808] `count_distinct` over a JSON-stored field → DATASET_INVALID / 400 in the distinct words, before any SQL', async () => {
345+
for (const [field, declared] of [['payload', 'json'], ['embedding', 'vector'], ['tags_list', 'multiselect']] as const) {
346+
expect(isAggregateCompatibleWithFieldType('count_distinct', declared), declared).toBe(false);
347+
const { go, sqls } = run('count_distinct', field);
348+
const err = await refusalOf(go);
349+
expect({ code: err.code, status: err.status }, declared).toEqual({ code: 'DATASET_INVALID', status: 400 });
350+
expect(err.message, declared).toContain(`declares as \`${declared}\``);
351+
expect(err.message, declared).toContain('COMPARES the stored values for equality');
352+
expect(err.message, declared).not.toContain('derives a NUMBER');
353+
expect(err.message, declared).not.toMatch(/count_distinct` accept every type/);
354+
expect(err.message, declared).toContain('`count` counts the rows');
355+
expect(sqls.length, declared).toBe(0);
356+
}
357+
// `count` over the same field compiles — it compares nothing.
358+
const { go, sqls } = run('count', 'payload');
359+
const result: any = await go();
360+
expect(result.rows.length).toBe(1);
361+
expect(sqls.length).toBe(1);
362+
});
363+
338364
it('the three cannot-answer tiers still do not block — unchanged by the widened scope', async () => {
339365
// 1. a field the hook cannot resolve
340366
const unknown = run('sum', 'no_such_field');

‎packages/services/service-analytics/src/dataset-compiler.ts‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -215,11 +215,22 @@ function aggregateToMetricType(m: DatasetMeasure): Metric['type'] {
215215
* the ORDER — collation-dependent for text on every backend, and absent
216216
* altogether where the storage form has no ordering operator (`min(jsonb)`
217217
* does not exist on PostgreSQL).
218+
* - [#20808] `count_distinct` COMPARES the stored values for equality, and
219+
* reaches this sentence only over a JSON-stored field (the table's
220+
* `count_distinct` row refuses the structured-JSON and multi-option types):
221+
* the in-memory driver counted equal documents apart, SQLite compared the
222+
* serialized text, and PostgreSQL has no equality operator for `json`.
218223
*
219-
* `count` / `count_distinct` accept every type and never reach this sentence.
224+
* `count` accepts every type and never reaches this sentence.
220225
*/
221226
const DIVERGENCE_BY_AGGREGATE = (aggregate: string, fieldType: string): string =>
222-
aggregate === 'min' || aggregate === 'max'
227+
aggregate === 'count_distinct'
228+
? `"${aggregate}" COMPARES the stored values for equality, so over a \`${fieldType}\` column `
229+
+ 'the answer is decided by how each backend compares a JSON-stored value rather than by '
230+
+ 'the data — one counts every row apart, one compares the serialized text, another has no '
231+
+ 'equality for the type and fails at query time — and one dataset would mean two things '
232+
+ 'on two deployments. '
233+
: aggregate === 'min' || aggregate === 'max'
223234
? `"${aggregate}" SELECTS one of the stored values, so over a \`${fieldType}\` column the `
224235
+ 'answer is decided by the ORDER the SQL dialect happens to impose rather than by the '
225236
+ 'data — string order is collation-dependent, and some storage forms have no ordering '
@@ -244,19 +255,28 @@ const DIVERGENCE_BY_AGGREGATE = (aggregate: string, fieldType: string): string =
244255
* wants. [#17560] The selecting sentence is the third: an author who wrote
245256
* `min` over a text column wanted a FIRST ROW, and a sort delivers that in one
246257
* declared order instead of asking each backend for its own smallest value.
258+
* [#20808] The distinct sentence is the fourth: `count_distinct` over a
259+
* JSON-stored field has no value every backend compares alike, so the author
260+
* counts rows, or counts distinct a scalar field holding the part they meant.
261+
* The other two non-temporal sentences no longer say `count_distinct` accepts
262+
* every type: since #20808 it accepts every type but the JSON-stored ones.
247263
*/
248264
const REMEDY_BY_SOURCE_CLASS = (fieldType: string, aggregate: string): string =>
249265
TEMPORAL_SOURCE_FIELD_TYPES.has(fieldType)
250266
? 'For a temporal field, `min`/`max` return a real instant; a DURATION has to be '
251267
+ 'stored as a number (a computed "days open" field) and aggregated as one.'
268+
: aggregate === 'count_distinct'
269+
? 'For a JSON-stored field, `count` counts the rows; a distinct count has to be taken '
270+
+ 'over a field that stores one scalar value, so store the part you count in a field '
271+
+ 'of its own and `count_distinct` that field.'
252272
: aggregate === 'min' || aggregate === 'max'
253-
? 'For a field with no backend-independent order, `count`/`count_distinct` accept every '
254-
+ 'type because they read neither arithmetic nor order off the value; a "first" or '
255-
+ '"last" record is a SORT on the record list, which orders once in a declared '
273+
? 'For a field with no backend-independent order, `count` accepts every type because it '
274+
+ 'reads no value, and `count_distinct` every type but the JSON-stored ones; a "first" '
275+
+ 'or "last" record is a SORT on the record list, which orders once in a declared '
256276
+ 'direction, not an aggregate that asks every backend for its own smallest value.'
257-
: 'For a non-numeric field, `count`/`count_distinct` accept every type because they '
258-
+ 'read no arithmetic off the value; a quantity that should be added up has to be '
259-
+ 'stored as a numeric field and aggregated as one.';
277+
: 'For a non-numeric field, `count` accepts every type because it reads no value, and '
278+
+ '`count_distinct` every type but the JSON-stored ones; a quantity that should be '
279+
+ 'added up has to be stored as a numeric field and aggregated as one.';
260280

261281
/**
262282
* [#16737 / #16099 / #17560] Refuse a measure whose AGGREGATE cannot meaningfully

0 commit comments

Comments
 (0)