One account in the seed

In July I wrote about a one-word change that removed 38% of jaffle_shop's customers while all 20 data tests passed. In it I mentioned a bug I had just reported in Fivetran's dbt package for Stripe. This is what happened next, because the ending is more useful than the bug.

The short version: the package's daily overview mixed every Stripe account's money into every other account's totals. Its tests could not notice, because its seed data contained one account. The maintainers replied the next day, the fix shipped in dbt_stripe 1.10.0 on 13 August, and fixing it turned up a second bug of exactly the same kind, one layer further down.

The bug#

stripe__daily_overview describes itself as "per account per day". Its fifteen rolling_* columns, the running totals of sales, refunds, fees, payouts and balances, were computed like this:

sum({{ t }}) over (order by date_day rows unbounded preceding) as rolling_{{ t }}

There is no partition by account_id, so the running total runs across every account in the table. With the package's exact clause:

account   date         daily_sales   as written   per account
acct_A    2024-01-01   100           100          100
acct_B    2024-01-02     5           105            5
acct_A    2024-01-03    50           155          150

Account B took 5 in sales on its only day of trading and reported a running total of 105.

Why every test was green#

The package's integration seed had one account. With one account, a window with no partition by and a window partitioned by account return identical rows. Not similar: identical. No assertion run on that data, however careful, can tell the broken query from the correct one. The model also had no data tests declared on it.

That is the question I now ask of any test suite:

Is there a plausible wrong version of this SQL that returns exactly the same output on the data you test with?

If there is, the suite is blind to that mistake by construction. Here the wrong version was one missing clause away.

Proving the fix, not just making it#

The fix is three lines: partition by account_id, source_relation on the three windows involved. source_relation is there because it is part of the model's grain, and two unioned connectors collide the same way two accounts do. The real work was showing that the fix is right, and that the new test can actually fail.

• A second account in the seed. Without it, the test I was about to write would pass against the broken models and prove nothing.
• The size of the error. With two accounts holding identical activity, each one's final rolling sales came out at 42,260,636.00 before the fix, exactly twice its own 21,130,318.00. With N accounts it would be N times. After the fix, each reads 21,130,318.00.
• A test that fails on the bug. It asserts that every rolling_* column equals the running total of its own account's daily values. Against the pre-fix models it fails on 4,021 of 4,022 rows. Against the fix it passes.
• Nobody else moves. With the original single-account seed, I built the model before and after and compared every column of every row: 2,011 rows on both sides, 0 different.

That last check matters as much as the test. A fix to a widely used package that quietly changes everyone's numbers is an incident of its own.

One layer down, the same root cause#

While reproducing, I noticed the daily totals were shared too. int_stripe__account_daily joined balance transactions and failed charges to the date spine on the date and the connector, but not on the account. With two accounts, each one's daily sales included the other's transactions. The rolling totals were wrong, and so were the numbers they rolled up.

The maintainer asked me to fold that into the same PR, and the fixture problem came straight back. In the seed, connected_account_id was null on every transaction. With no transaction naming an account, a join on the account has nothing to separate, so the test I wrote for it passed against the broken models as well. I had to give three of the nine seed transactions to the second account before the test could fail at all. Then:

Account 1234 had been reporting three sales that belonged to 5678.

The join has one decision in it worth showing:

and coalesce(balance_transaction.connected_account_id, date_spine.account_id) = date_spine.account_id

connected_account_id is null on every transaction for a destination that does not use Stripe Connect, which is most of them. A strict equality join would have dropped all of those and reported zeros for most users. Falling back to the spine's own account keeps that case exactly as it was, while a transaction that does name an account now counts only there. A transaction that names no account still counts toward every account, because the source data does not say whose it is. Rather than invent a rule for that, it is written down as a known limit in the package's DECISIONLOG.

The same PR fixed a third, smaller one: account_daily_id hashed the account and the date but not the connector, so two unioned connectors sharing an account id and a date produced the same key.

How it was received#

Fivetran's maintainers replied the day after the report and asked for the PR. They asked me to add the two extra findings, said they would run it against richer test data of their own, and shipped it in 1.10.0 on 13 August, eighteen days after the first report. The release notes list each change as breaking or possibly breaking, so anyone on several accounts knows why their numbers moved. One line from the review sums up the whole episode better than I can:

Our seed data isn't very comprehensive.

That is not a criticism of them. It is true of nearly every dbt project, mine included. Seed data gets written once, to make the build pass, and from then on it quietly decides which mistakes the tests are able to see.

What I check in fixtures now#

Four things, all cheap:

1. Every grain key has at least two values. If the model is "per account per day", the fixture has two accounts, and they do not have identical activity. One value of a key makes every partition, group by and join on that key untestable.
2. Every join has rows that match and rows that do not. A customer with no orders, an order with no payment, a transaction with no account. These are the rows that tell left from inner, and a strict join from a fallback.
3. Nullable columns have nulls. If production can send a null connected_account_id, so does the fixture, in some rows and not in others.
4. The test fails on the wrong version. Before trusting a new test, break the model the way you expect it could be broken and watch the test go red. If it stays green, the fixture cannot express the bug yet.

The fourth is the one that caught me twice in a single PR.

Where Misata fits#

I found this bug by reading SQL, not with a tool. What I build, Misata, helps with the fixture half: misata dbt-unit-test reads your manifest and generates the mocked inputs for a model's unit test, with keys that resolve across them and five rows per input by default rather than one, and it leaves the expected output to you. It would not have told anyone the window was missing a partition. It does make it easier to start from a fixture in which that mistake is visible.

Today that lives in a Python CLI beside your project. I am trying to find out whether it belongs inside it instead: a dedicated dbt package, so that fixtures are generated from your models in a dbt command, and checks like "at least two accounts" or "some transactions with no account" are declared next to the models they protect. Before I build it, I would rather hear from people who write dbt tests. When did you last write a seed or a unit test fixture by hand, what made it painful, and would you install a package to do it? Reply on the dbt Slack or write to me at hello@misata.studio.

The record: the issue, the pull request and release 1.10.0.