Metabase importer should preserve optional SQL clause filter behavior
- Dominant language
- Go
- Stars
- 730
- Forks
- 33
- Avg merge
- 11h 47m
- Merged PRs (30d)
- 18
Description
## Summary
When importing Metabase dashboards, DAC currently strips Metabase optional SQL clause markers (`[[ ... ]]`) and emits warnings like:
```text
Metabase optional SQL clause markers [[...]] were removed; review empty-filter behavior
```
This makes the imported dashboard structurally valid, but it can change widget behavior when a filter is empty. In Metabase, optional clauses are included only when their referenced variables have values. After import, the converted SQL may either always apply a clause or lose the intended conditional behavior, depending on how the marker is removed.
## Why this matters
For production dashboard migration, this is one of the main manual-review risks after `dac import metabase`. A dashboard can validate successfully while still producing different results from Metabase for empty/default filter states.
## Desired behavior
The Metabase importer should preserve optional clause semantics in DAC output, ideally by converting Metabase optional SQL blocks to equivalent DAC/Jinja conditionals.
For example, a Metabase query like:
```sql
SELECT *
FROM orders
WHERE 1 = 1
[[AND created_at >= {{start_date}}]]
[[AND customer_id = {{customer_id}}]]
```
could become something like:
```sql
SELECT *
FROM orders
WHERE 1 = 1
{% if filters.start_date %}
AND created_at >= '{{ filters.start_date }}'
{% endif %}
{% if filters.customer_id %}
AND customer_id = '{{ filters.customer_id }}'
{% endif %}
```
The exact output shape can follow DAC's preferred filter conventions, but the key requirement is that empty filter values do not alter the query compared with Metabase.
## Suggested scope
- Detect Metabase optional SQL blocks during import.
- Preserve the optional behavior as DAC/Jinja conditionals when the referenced variable maps to a dashboard filter.
- If conversion is ambiguous, add a clear inline TODO/comment near the affected SQL rather than silently flattening the clause.
- Consider a stricter import option that fails when optional clause semantics cannot be represented safely.
- Include regression tests covering empty filter values, populated filter values, and multiple optional clauses in one query.
## Current workaround
Manually review every warning from `dac import metabase`, inspect each affected widget, and rewrite the SQL conditionals by hand.
Contributor guide
Research direction
Start with the `dac import metabase` entry point and reproduce the warning using queries containing `[[...]]` blocks. Add regression coverage for empty values, populated values, and multiple optional clauses; done means DAC output preserves conditional behavior, or clearly marks ambiguous conversions instead of silently flattening them.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- backend, data
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100