bruin-data / bruin-data/dac

Metabase importer should preserve optional SQL clause filter behavior

Open
#43 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.