Skip to content

feat: support filtering on table calculations#24

Open
jpetey75 wants to merge 2 commits into
feat/multiple-filters-same-fieldfrom
feat/table-calculation-filters
Open

feat: support filtering on table calculations#24
jpetey75 wants to merge 2 commits into
feat/multiple-filters-same-fieldfrom
feat/table-calculation-filters

Conversation

@jpetey75

Copy link
Copy Markdown
Contributor

Closes #21

⚠️ Stacked on #22 (base branch is feat/multiple-filters-same-field, not main). Review/merge #22 first, then I'll retarget this to main. The diff here is scoped to #21 only.

Problem

The SDK's filter system was dimension-only — there was no way to filter on table calculations, even though the API supports it via filters.tableCalculations.

Change

  • TableCalculation(name, sql, display_name=None) — a SQL table calculation, with the same comparison operators (==, >, between, etc.) and helpers as Dimension.
  • TableCalculationFilter — accepts a TableCalculation or a calc-name string; serializes as {target: {fieldId}, operator, values}.
  • CompositeFilter.to_dict() now routes dimension vs table-calc rules into filters.dimensions / filters.tableCalculations respectively. The tableCalculations key is omitted when unused, so existing dimension-only payloads are unchanged.
  • Query.table_calculations(...) builder method + model.query(table_calculations=...) kwarg.
  • Extracted a shared _FieldFilterMixin for the &/| operators rather than duplicating them across filter classes.
from lightdash import TableCalculation

profit_ratio = TableCalculation(name="profit_ratio", sql="${orders.profit} / ${orders.revenue}")

query = (
    model.query()
    .metrics(model.metrics.revenue, model.metrics.profit)
    .table_calculations(profit_ratio)
    .filter(profit_ratio > 0.2)
)

Verification

Shapes were checked against the authoritative lightdash/common source types rather than guessed:

  • Filters = { dimensions?, metrics?, tableCalculations? }, each a FilterGroup → confirms the filters.tableCalculations routing.
  • FilterRule = { target: {fieldId}, operator, values } (server injects id) → confirms the rule shape. A live API call returning a filter-rule parse confirmed the API accepts exactly this shape.
  • SqlTableCalculation = { name, displayName, sql } → confirms TableCalculation.to_dict().

Caveat: I could not produce a fully-green end-to-end run via the available tooling (the MCP query tool doesn't register custom SQL table calculations into the executed query). The serialization is confirmed by source + the API's filter parser; a final confidence check would be running the SDK against a live instance with an API token.

Tests

27 new tests in tests/test_table_calculations.py (calc serialization, operators, filters.tableCalculations routing, mixed dimension+calc filters, builder integration). Full unit suite green (81 passed).

🤖 Generated with Claude Code

jpetey75 and others added 2 commits June 16, 2026 18:07
Add a TableCalculation type and TableCalculationFilter so queries can filter
on calculated values. Table-calc filters serialize under
`filters.tableCalculations` (a sibling of `filters.dimensions`), matching the
Lightdash `Filters` type; the calculation itself serializes as a
`SqlTableCalculation` (`{name, displayName, sql}`).

- `TableCalculation(name, sql, display_name=None)` with the same comparison
  operators / helper methods as `Dimension`.
- `TableCalculationFilter` accepts a `TableCalculation` or a calc name string.
- `CompositeFilter.to_dict()` routes dimension vs table-calc rules into their
  respective keys; the `tableCalculations` key is omitted when unused so
  existing dimension-only payloads are byte-for-byte unchanged.
- `Query.table_calculations()` builder method + `model.query(table_calculations=...)`.
- Extracted a shared `_FieldFilterMixin` for the `&`/`|` operators instead of
  duplicating them across filter classes.

Shapes verified against lightdash/common source types and the live API's
filter-rule parser.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
End-to-end testing against a live instance surfaced a 400 CompileError:
filtering a table calculation with a numeric operator failed because an
untyped calc is treated as a "string" field by the API
("No function ... for the filter operator 'greaterThan' on ... field of type
'string'").

- `TableCalculation` gains a `type` field (defaults to "number"); `to_dict()`
  now emits it as part of the SqlTableCalculation, matching TableCalculationBase.
- Added an env-gated acceptance test (`test_query_table_calculation_filter`)
  that defines a calc, filters on it, and asserts the server applied the filter
  (every returned row satisfies the predicate and the count matches a
  client-side computation). Verified live.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jpetey75

Copy link
Copy Markdown
Contributor Author

End-to-end verification (live instance) — found & fixed a real bug

Ran the SDK against a live Lightdash instance with a real token. The first attempt failed with a 400, which source-reading alone hadn't caught:

CompileError: No function has been implemented to render SQL for the filter
operator "greaterThan" on "pv_copy" field of type "string"

Root cause: a table calculation with no declared type is treated as a string by the API, so numeric operators (>, between, …) won't compile. The filters.tableCalculations routing and rule shape were correct — the calc definition was just missing its type.

Fix (latest commit): TableCalculation now has a type field defaulting to "number" (emitted in to_dict(), matching TableCalculationBase). Override for non-numeric calcs (type="string" etc.).

After the fix — verified end-to-end: defined a calc, filtered calc > threshold, and confirmed the server returned exactly the rows a client-side filter would (every returned row satisfies the predicate; count matches). Added as a permanent env-gated acceptance test (test_query_table_calculation_filter), which passes live.

So this PR is now confirmed working end-to-end, not just shape-verified — and the earlier caveat about not being able to run E2E is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant