You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ThoughtSpot converter: an absent aggregation is treated as NONE in both directions, but ThoughtSpot's default is SUM #467
Found by an independent adversarial review, and reproduced. Pre-existing in the merged converter (#364).
The converter treats a missing aggregation key as NONE. ThoughtSpot documents the default as SUM. The premise is wrong in both directions, and the two halves compound.
Read side — convert_metric uses properties.get("aggregation", "NONE")
emits a metric with THOUGHTSPOT expression [orders::amount] and no aggregate at all. ThoughtSpot would sum it. Exit 0, no issue for this metric, validator PASSES.
No corpus model omits aggregation, so real exports do not surface this — hand-authored and minimal TML does.
Write side — aggregation: NONE is dropped
This one loses data on a plain round trip. A formula with an explicit NONE:
{name: Ratio none, formula_id: f1, properties: {column_type: MEASURE}}
The aggregation: NONE is gone. ThoughtSpot applies its documented default, so a per-row ratio is now summed — the model reports the sum of ratios where it used to report the ratio. Exit 0 both directions; the only issue is INFO TS-EXPR-THOUGHTSPOT-ONLY. Verified independently.
The same happens to a column_id MEASURE with explicit NONE.
Cause
_build_metric in ossie_to_thoughtspot.py emits an aggregation only when the shape or the stash supplies one, or when the expression has an outer aggregate. It carries the same "absent means NONE" premise as convert_metric, so an explicit NONE and an absent key become indistinguishable.
Note for whoever fixes this
The test suite currently enforces the wrong premise. The reviewer mutated the code to treat an absent aggregation as SUM and four tests failed. One of them, test_a_bare_group_aggregate_without_one_gains_none, only asserts an absent value stays absent, which is fair round-trip fidelity — the other three encode the premise itself and would need to change with the fix.
Fixed on main by #463 (merged as 19e69e4). The PR body only carried Fixes #459, so this one did not auto-close.
Both halves the issue described are addressed:
An absent aggregation now takes ThoughtSpot's own default, SUM rather than being read as NONE (ead5cac8). Narrowed to the cases where the default is genuinely live — a raw column and a scalar formula. Where a formula cross-references other formulas the default is declined rather than guessed, and reported as TS-METRIC-AGGREGATION-DEFAULT-UNRESOLVED, because composing sum(...) around [formula_a] / [formula_b] double-aggregates: it imports cleanly and answers wrongly.
An explicit aggregation: NONE is preserved across a round trip (9d9feee7), stashed so the reverse direction can tell "the user set NONE" from "nothing was set", which the conventional path cannot otherwise distinguish.
One known follow-up, raised by @jbonofre in review of #463 and not covered here: with the SUM default in place, a non-numeric column marked MEASURE — a varchar or date, or a string-returning formula — would come out as sum ( [t::col] ), which is not a valid metric. A datatype check before applying the default would avoid it. I will pick that up once the converters have settled in apache/ossie-converters, so it lands in whichever repo owns the code.
Activity
fix(thoughtspot): preserve an explicit `aggregation: NONE` across a r…
fix(thoughtspot): an absent aggregation is ThoughtSpot's default, SUM…
fix(thoughtspot): preserve an explicit `aggregation: NONE` across a r…
fix(thoughtspot): an absent aggregation is ThoughtSpot's default, SUM…
fix(thoughtspot): portable expressions — unqualify a field's, quote w…
djwaldo commentedon Oct 8, 2026
Fixed on
mainby #463 (merged as 19e69e4). The PR body only carriedFixes #459, so this one did not auto-close.Both halves the issue described are addressed:
aggregationnow takes ThoughtSpot's own default, SUM rather than being read as NONE (ead5cac8). Narrowed to the cases where the default is genuinely live — a raw column and a scalar formula. Where a formula cross-references other formulas the default is declined rather than guessed, and reported asTS-METRIC-AGGREGATION-DEFAULT-UNRESOLVED, because composingsum(...)around[formula_a] / [formula_b]double-aggregates: it imports cleanly and answers wrongly.aggregation: NONEis preserved across a round trip (9d9feee7), stashed so the reverse direction can tell "the user set NONE" from "nothing was set", which the conventional path cannot otherwise distinguish.One known follow-up, raised by @jbonofre in review of #463 and not covered here: with the SUM default in place, a non-numeric column marked MEASURE — a varchar or date, or a string-returning formula — would come out as
sum ( [t::col] ), which is not a valid metric. A datatype check before applying the default would avoid it. I will pick that up once the converters have settled inapache/ossie-converters, so it lands in whichever repo owns the code.