Review: Nightly Sales ETL Job Written by an AI Assistant

Your data team asked an AI coding assistant to replace a fragile spreadsheet export with a nightly job that loads sales into the warehouse. It opened this pull request. You are the reviewer, and nothing merges without your approval.

The assistant's description:

Adds load_daily_sales, run nightly by Airflow. Copies yesterday's paid orders into fact_sales and keeps daily_store_totals up to date with an upsert. Everything runs in one warehouse transaction, so a failed run leaves nothing behind and Airflow's retries are safe. Amounts are converted to integer pence to avoid rounding errors.

Where it runs: The shop database is the production PostgreSQL 15 primary; orders has 80 million rows, an index on created_at (a timestamptz) and total stored as numeric(10,2) in pounds. About 250,000 orders a day. The job runs in a container whose clock and database sessions are in UTC, and the business reports on UK days (Europe/London). Orders paid by bank transfer stay pending until the money arrives, up to 3 days later. When numbers look wrong, analysts clear the task in Airflow to re-run a given day, sometimes a week later. The warehouse is also PostgreSQL; neither warehouse table has a unique constraint yet.

Your task

• Flag every problem you would block the merge for. Click a line number to comment, and Shift-click to cover a range.
• For each one, choose the issue type and severity, and explain what fails and how to fix it.
• You are scored on the problems you catch, the right issue type and a clear explanation. Missing a critical problem fails the round.

Aim for 15 minutes, as in a real review round.

The job succeeds for 3 March. An analyst re-runs 3 March. What is in daily_store_totals and fact_sales afterwards?

On 10 March an analyst re-runs the task for 3 March. Which day does it load?

Try int(float(Decimal("19.99")) * 100) in a Python shell.

An order is placed on 3 March and paid by bank transfer on 5 March. Which run ever loads it?

Where does created_at::date put an order placed at 00:20 London time in June, and can it use the index?

A strong reviewer blocks this change because it is not idempotent: the description says retries are safe, but every re-run of a successful day adds that day again, and re-runs are routine here.

1. Totals are added to, not replaced (lines 44–48, critical). If the row exists, the job adds today's amount to it. Rolling back a failed run is fine, but any re-run of a day that already succeeded (an analyst clearing the task, or Airflow retrying after a timeout that struck after the commit) doubles that day's totals. Recompute the day and overwrite: INSERT ... ON CONFLICT (store_id, day) DO UPDATE SET amount_pence = EXCLUDED.amount_pence, with a unique constraint on (store_id, day).
2. Duplicate fact rows on re-run (lines 31–35, major). fact_sales has no key and the insert has no conflict handling, so each re-run inserts the same orders again. Delete the day's rows and insert in the same transaction, or add a unique constraint on order_id and use ON CONFLICT DO NOTHING or DO UPDATE.
3. Float conversion loses pennies (line 30, major). float(Decimal("19.99")) * 100 is 1998.9999999999998, and int() truncates it to 1998. Many prices lose a penny, which is exactly the rounding error the description promises to avoid. Stay in Decimal: int(total * 100), or have SQL return (total * 100)::bigint.
4. Loads the wrong day on a re-run (line 13, major). The day comes from the wall clock, not from the run being executed, so re-running 3 March on 10 March loads 9 March, and a retry that starts after midnight skips a day. Take the date from Airflow's logical date (data_interval_start) and pass it in.
5. UTC day and a full scan of the primary (line 20, major). In a UTC session, created_at::date splits days at UTC midnight, so during British Summer Time orders placed between 00:00 and 01:00 London time land on the previous day. The cast also stops the created_at index being used, so every night scans 80 million rows on the production primary. Use a half-open range on the raw column with bounds computed in Europe/London, ideally on a replica.
6. Late payments are never loaded (line 21, major). The job only looks at orders created yesterday that are already paid. An order paid by bank transfer two days later is pending when its day is loaded and is never picked up again. Load by when the order became paid (a paid_at column or change feed), or reprocess a trailing window of several days on each run.