Skip to content

A number/string/boolean metric's SQL expression is replaced by COUNT(*) #4157

Description

@os-zhuang

Found while making the aggregate vocabularies lockstep (#4153). Left unfixed there on purpose — it is a behavioural change in an analytics SQL path, not a derivation cleanup.

The defect

NativeSQLStrategy.resolveMeasureSql wraps a measure's SQL according to its type. After #4153 that is a table lookup with a fallback:

constwrap=AGGREGATE_SQL[measure.type];returnwrap ? wrap(col) : `COUNT(*)`;

AGGREGATE_SQL covers the six aggregates. But AggregationMetricType (packages/spec/src/data/analytics.zod.ts:21) — the vocabulary Metric.type is validated against — has nine members:

'count','sum','avg','min','max','count_distinct','number',// Custom SQL expression returning a number'string',// Custom SQL expression returning a string'boolean'// Custom SQL expression returning a boolean

For those last three the measure's sqlis the whole expression — a ratio, a CASE, a window function. Nothing should be wrapped around it. Instead the expression is discarded and the query returns COUNT(*): a row count where the author declared a computed value. No error, no log.

Reachability

Not reachable from a dataset. dataset-compiler.ts's aggregateToMetricType only ever returns an AggregationFunction member, and throws for a measure with no aggregate — so a compiled dataset's metrics are always one of the six.

It needs a hand-authored Cube reaching NativeSQLStrategy with a type: 'number' measure. Whether any exists today is the first thing to establish; the spec explicitly invites them ("Custom SQL expression returning a number"), so the surface is advertised.

Suggested fix

Return col for number/string/boolean — the expression as authored, unwrapped — and reserve the fallback for a genuinely unknown type, where a throw is more honest than a plausible-looking number (the argument #3948 settled for the drivers).

The care needed is in aggregation context: a custom expression used as a measure in a GROUP BY query must itself be aggregate-shaped, or the SQL is invalid. So the fix probably wants a test matrix over grouped/ungrouped and the three types, which is exactly why it did not belong in #4153.

Worth checking at the same time whether the !measure early return (return 'COUNT(*)' for a member that resolves to no measure at all) should also be a throw. Same silent-plausible-answer shape.

Refs #4153, objectstack-ai/objectui#2945, #3948

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions