Data Modeling Critique an Existing Model

The reasoning

The table declares one row per order line and then carries a measure that is not a property of a line. SO-1 has two lines, so its 30 discount is stored twice and summed twice. Nothing about the table is corrupt: the grain holds, the line measures are right, and only the borrowed column is wrong — and only when it is summed.

That is why it survives review for a year. `MAX(order_discount)` and `AVG(order_discount)` both look plausible, uniqueness tests on (order_id, line_no) pass, and row counts tie to source. The single query that exposes it is the one the finance team runs.

The corrected model splits by grain: line measures stay on the line fact, order measures move to an order fact, and the two are combined by aggregating each to a common grain rather than by joining them. That is drill-across, and the relationship note says so explicitly, because joining the two facts directly would re-create a fan-out of a different shape.

The alternative fix is to allocate the discount down to line grain in proportion to line amount, storing the allocated value. That keeps one table and makes the measure genuinely additive, at the cost of baking an allocation rule and a rounding policy into the data. Either is defensible; leaving the column where it is, is not.

The reasoning

Ask what one row means and the table has no answer — some rows are orders and some are shipments. Every measure is therefore NULL for half the rows, `COUNT(*)` counts a mixture, and any denominator computed from the row count is wrong. Average order value here is 160 over 4 rows rather than over 2.

It also forces every downstream query to carry a filter it can silently omit. Analysts must remember `WHERE event_type = 'ordered'` on every aggregate, and the day someone forgets, the number is wrong rather than absent. A model that requires a `WHERE` clause for correctness has moved the grain into a convention.

The fix is one fact per business process. Two tables, two grains, two sentences: one row per order placed, one row per shipment despatched. Both are conformed by order, so combining them is drill-across — aggregate each to order grain, then join the aggregates — which is exactly the discipline the relationship note records.

The counter-argument you will hear is that one table is fewer objects to maintain and lets you see the whole lifecycle in one place. If lifecycle timing is the actual requirement, the right shape is an accumulating snapshot: one row per order with a column per milestone, updated as it progresses. That answers the lifecycle question properly rather than by mixing grains.

The reasoning

The diagram shows the symptom directly: no grain sentence on the table. That is worth treating as the finding rather than as a missing comment, because a declared grain is what gives you a key to assert uniqueness on. Without one, a duplicate load is indistinguishable from real data.

Running the check by hand shows 4 rows across 3 distinct sessions, and revenue inflated by 30 — the duplicated session's revenue counted twice. Every ratio built on this table is also wrong, because both numerator and denominator moved by different amounts.

The corrected model declares one row per session and makes `session_id` the key. That single change turns the invariant into something the warehouse can enforce and the pipeline can test — the duplicate now cannot load at all, rather than loading and being discovered by a stakeholder.

The broader point for a critique: a table named for its source (`events_wide`) rather than for a business process is a strong signal that the grain was never chosen. Naming a fact after what it measures — sessions, orders, shipments — usually forces the question that this model skipped.

The reasoning

The rows are correct and the grain is correct — one row per sku per day is a textbook periodic snapshot. The defect is that `units_on_hand` is a state, not an event, so adding it across days counts the same stock once per day it sat there.

This is the failure mode that makes snapshots dangerous rather than merely large: the table looks exactly like a transaction fact, the column looks exactly like an additive measure, and SQL will sum anything numeric without complaint. Nothing in the schema distinguishes 'units that moved' from 'units that were there'.

The corrected model changes almost nothing physically — the same rows, renamed to say what it is, with the measure annotated as semi-additive — and changes the query to take the latest day. The real fix is that the rule now lives somewhere: in the column note here, and in a semantic layer in production, so that every consumer inherits it instead of each analyst rediscovering it.

Worth adding in an interview: semi-additive measures usually want more than 'last value'. Average daily stock, opening and closing balance, and stock at period end are all legitimate and all different, so the semantic layer should expose named metrics rather than a raw column that anyone can sum.

The reasoning

A dimension's implicit contract is one row per entity. Break it and every join through that dimension becomes a fan-out — the fact row is duplicated once per matching dimension row, and every measure summed afterwards is multiplied. Two rows for sku A doubles A's revenue, and the join looks completely normal.

The specific weakness here is that the fact joins on the *natural* key. The dimension does have a surrogate primary key, so `product_sk` is unique — but nothing constrains `sku`, which is the column actually used in the join. A primary key that nobody joins on protects nothing.

The corrected model fixes both halves: deduplicate the dimension on its natural key so the contract holds, and have the fact carry `product_sk` so the join is on a column that is unique by construction. After that, a duplicate load cannot fan out even if it happens, because the surrogate cannot repeat.

The operational lesson is worth stating: a uniqueness assertion on every dimension's natural key is the single most valuable test in a warehouse, because this failure is silent, load-time, and inflates money. And a dimension load should be a merge on the natural key rather than an append, so the duplicate never arrives.

The reasoning

Each fact is at its own grain and neither is unique on `order_id` — that is the whole point of a fact table. Joining on a column that repeats on both sides produces the cross product within each order, so every line pairs with every payment. Both measures inflate, and they inflate by different factors whenever the two counts differ.

What makes it dangerous is that the answer is not obviously wrong. Revenue and paid both read 600, they still agree with each other, and the reconciliation the analyst was doing appears to pass. A comparison built on two equally-inflated numbers looks healthiest exactly when it is most wrong.

The correct pattern is drill-across: aggregate each fact to a common grain — order, here — in its own subquery, then join the two single-row-per-order results. That join is now one-to-one, so nothing multiplies, and the corrected query returns 300 and 300.

The rule to carry away is that facts are never joined to each other directly; they are conformed through shared dimensions and combined after aggregation. If you find yourself writing `JOIN fct_ ... ON` with another fact on the right, that is the signal.

The reasoning

A dimension is defined by being unique on the entity it describes. Storing the account relationship inside it changes the grain to one row per customer per account, which nothing in the model records — the table is still called `dim_customer` and is still joined as if it were one row per customer.

The consequence is a fan-out on every fact that touches the dimension, not just on the reports that care about accounts. That is the difference between this and a bridge: a bridge is joined only by the queries that need the many-to-many, whereas a compromised dimension damages everything downstream of it.

The corrected model restores `dim_customer` to one row per customer and puts the pairing in `br_customer_account`, with an allocation factor so revenue by account still totals to real revenue. Queries that do not care about accounts join the dimension and are unaffected; queries that do, opt into the bridge and its weights.

The judgement call underneath is whether the relationship is genuinely many-to-many. If a customer can belong to several accounts *at once*, it is, and it needs a bridge. If they move between accounts over time, that is a Type 2 attribute rather than a bridge — different problem, different answer, and confusing the two is common.

The reasoning

The bridge is doing its job — it correctly records that SO-1 belongs to two campaigns. The fan-out that follows is not a bug in the join; it is the arithmetic consequence of one order legitimately belonging to two things. Any measure summed after that join is multiplied by the number of associations.

The individual figures are what make this hard to spot. Email at 160 and paid at 100 are both defensible-looking numbers, and a report showing campaign revenue rarely shows the company total beside it. The error only appears when someone adds the columns up.

The allocation factor turns the association into a weighted one: 0.5 to each campaign for a two-campaign order, 1.0 for a single-campaign order. Multiply before summing and the measure is additive again at every level, with no correction needed in any query.

The genuinely hard part is not technical. The weights encode an attribution model — even split, first touch, last touch, time decay — and marketing and finance will want different ones. Two honest options: name the rule in the column so the choice is visible, or carry several weight columns and let the consumer pick, accepting that two dashboards will then disagree on purpose rather than by accident.

The reasoning

Type 1 keeps one row per customer, so there is no version for the join to select — the query gets today's region for a transaction from January, and there is no record that it was ever anything else. The earlier number is not merely hard to reproduce; it is unreproducible from this model.

The disappearance of the EU row is the part that causes the incident. A stakeholder comparing the two runs concludes revenue was deleted or misclassified, and every hypothesis they reach for first — a broken load, a dropped partition — is wrong. The cause is a dimension update that nothing logged.

The corrected model makes `region` Type 2 with `valid_from` / `valid_to`, and joins as-of the order date. January is now grouped by January's region permanently, and the March view is still available by joining on `is_current` when someone genuinely wants 'grouped by where they are now'.

The decision rule worth stating: Type 1 is for correcting values that were wrong, Type 2 for recording values that changed. A misspelled name is a correction. A region change is history. The test is whether anyone would ever want the old value back — and if a report groups by it, they will.

The reasoning

Consecutive Type 2 versions normally share a boundary — the old row's `valid_to` equals the new row's `valid_from` — because that is what makes the timeline continuous with no gap. With an inclusive comparison on both ends, the boundary instant belongs to two rows, and any fact landing on it fans out.

The failure is rare by construction, which is what makes it dangerous. Only facts dated exactly on a change day are affected, so a test with a handful of rows almost certainly misses it, and the symptom in production is a total that is slightly too high on some days and correct on others.

The fix is the half-open interval: greater-than-or-equal on the lower bound, strictly-less-than on the upper. Every instant then belongs to exactly one version, boundaries included, and the join returns one row. The corrected model records this in the column note, because 'valid_to is exclusive' is a convention a reader cannot infer from the data.

The alternative is to store `valid_to` as the last instant of the old version rather than the first of the new. That works and is worse: it requires a date arithmetic decision (minus one day? one second? what about a time zone?) that varies by column type, and it makes the timeline harder to reason about. Half-open intervals avoid the question entirely.

Whichever you choose, the invariant deserves a test: no two versions of the same natural key may overlap, and there should be no gaps. That test is cheap and catches both this and a whole family of load bugs.

The reasoning

The instinct — do not store what you can derive — is a good one in an operational schema and wrong here. The list price in the dimension is a *current* attribute of the product; the price charged was a property of the transaction, affected by discounts, contracts and promotions that the dimension does not know about. They were never the same number.

Because revenue is computed rather than stored, every historical figure is recomputed on every run. A price rise silently restates last year, invoices stop reconciling to the warehouse, and there is no version of the data that still agrees with what the customer was billed.

The corrected model stores `unit_price` and `extended_amount` on the fact — the amount actually charged, frozen at the moment of the transaction. The dimension keeps `list_price` as a descriptive attribute, which is now genuinely useful: comparing charged against list is how you measure discounting.

The general principle is worth naming: a fact table records what happened, so anything that was true only at that moment belongs on it. That includes the price, the exchange rate used, the tax rate applied and any attribute the business would consider part of the transaction record. Deriving them later from a dimension makes history depend on the present.

The reasoning

This is the failure that wastes the whole investment in Type 2. The dimension has both versions, the load is correct, the history is there — and the query restricts to one row per employee, which makes the dimension behave exactly like Type 1 while costing more to store and load.

It is worth noticing that the wrong answer is not obviously wrong. Sales showing 2 tickets is only suspicious if you know sales does not handle tickets. In a larger organisation, work quietly migrating between teams as people move is invisible, and headcount-based metrics drift for reasons nobody can trace.

The corrected version changes nothing about the model — the diagram is identical — and only the join predicate differs, matching on the natural key with the ticket's close date falling inside the version's validity window. That is the whole fix, and its size relative to the damage is the point of the question.

The durable answer is to stop leaving the choice to the query. Resolve the version at load time so the fact carries the surrogate key of the correct version, and the as-of logic exists in exactly one place. Where that is not possible, define both metrics in the semantic layer under distinct names, so 'by team at the time' and 'by team now' are chosen deliberately rather than by whichever predicate someone typed.

The reasoning

The primary-key declaration is a claim the data cannot honour: the moment a second version is written, `customer_id` repeats. The diagram states the grain as one row per customer per version, which is directly at odds with the key — and that contradiction is the finding.

The fan-out follows automatically. A fact row joining on `customer_id` matches every version of that customer, so its measures are counted once per version. A customer with five historical changes multiplies their revenue by five, which means the error grows with how much history you keep.

The corrected model gives each version its own surrogate key and has the load resolve which version applies when the fact is written. The fact then references one specific version, the join is a plain equality on a unique column, and there is no as-of predicate for anyone to get wrong.

This is the strongest argument for surrogate keys and the one most often left out in favour of 'integers join faster'. Speed is a secondary benefit. The primary reason is that the natural key stops being unique the moment you keep history, and a fact needs to point at a version rather than at an entity.

The reasoning

String concatenation throws away the boundary between the parts. Any value that can contain the delimiter makes the mapping ambiguous, and account codes, SKUs and plan names from upstream systems routinely contain hyphens. The query shows it directly: 2 rows, 2 distinct pairs, and only 1 distinct key.

What makes it dangerous is that the collision does not raise. Depending on how the key is used, it either merges two entities into one — losing rows in a dedup or a merge — or fails a uniqueness assertion for reasons that look impossible, because the underlying columns really are distinct.

The corrected model declares the composite key on the columns directly. Every warehouse supports a multi-column key or unique constraint, so there is no reason to flatten it into a string, and comparisons then respect the boundaries.

Where a single-column key is genuinely required — as a join key, or as a hash key in a Data Vault or lakehouse model — the safe construction is a hash over the parts with an unambiguous separator: a character that cannot occur in the data, or length-prefixed encoding, and a documented rule about NULLs. `MD5(a || '|' || b)` has exactly the same collision problem as the plain concatenation if `|` can appear.

The reasoning

A NULL never equals anything, so an inner join discards the row and every measure on it. The fact table is intact and the reports are short — which is exactly why 'the warehouse total does not match source' is such a common and confusing complaint. Nothing errors and the fact row is right there when you query the table directly.

Telling analysts to use a LEFT JOIN is not a fix, because it has to be remembered at every call site and BI tools generate inner joins by default. A model whose correctness depends on every consumer choosing the right join type has moved the invariant out of the data.

The corrected model adds a reserved row — key -1, 'Unknown' — and routes unmatched facts to it during the load, so the foreign key is never NULL. Totals now tie, the join can be an inner join, and the unmatched volume appears as a visible bucket someone can be asked about rather than as an absence nobody can see.

Two refinements worth mentioning. Distinguish 'not applicable' from 'not yet arrived' with different reserved keys, so the second can be monitored and repaired as a late-arriving dimension. And declare the foreign key NOT NULL once the unknown member exists, so the model enforces what the load promises.

The reasoning

The key was doing two jobs: identifying a store and describing it. That works while every code follows the scheme, and the first exception produces a silent new category. Nothing errors — `substr` happily returns `AC` — so a region appears in the report that does not exist in the business.

The second failure is change. Re-badge a store from mall to flagship, or move it between reporting regions, and either the key changes — breaking every fact that references it — or the key keeps lying. A smart key makes an attribute permanent by accident, which is a decision nobody made.

The corrected model keeps the code as a natural key, because it is how humans and source systems refer to the store, and promotes region and format to real columns. They can now be corrected, changed, given Type 2 history if the business needs it, and validated against a known list.

This is the same reasoning as the surrogate-key questions, from another direction: keys should identify, attributes should describe, and any design that merges the two makes both harder to change. The tell in a review is a report containing `substr`, `left(`, `split_part` or a regex applied to a key column.

The reasoning

A percentage is only meaningful with its denominator attached, and a fact table stores the value without it. `AVG(margin_pct)` treats every order as equally important, so the tiny 50%-margin order pulls the company figure from 20.3% to 35%. The larger the spread in order sizes, the worse it gets.

This is non-additivity in its purest form: the measure cannot be summed *or* averaged correctly across any dimension. That makes it different from the semi-additive case, where a rule about time is enough — here there is no aggregation of the stored column that gives the right answer at a coarser grain.

The corrected model stores `margin` alongside `revenue`, both additive, and derives the ratio at query time. That is correct at every grain — by order, by region, by month, company-wide — with no rule for anyone to remember, because the arithmetic is done after the aggregation rather than before.

The rule to state: never store a ratio in a fact table, store its components. Where a ratio must be presented consistently, define it once in the semantic layer as a derived metric over the two sums, so every tool computes it the same way and nobody can average it by mistake.

The reasoning

The column is not a measure, it is three measures sharing a name. Because they are all numeric, SQL sums them without complaint and produces a figure with no unit — dominated here by the yen order, which contributes 10,000 of the 10,200.

The model has no way to stop this. Any consumer who does not think to group by currency gets a meaningless total, and grouping by currency is not what the business wants either — 'revenue' is one number, not three.

The corrected model keeps `amount_local` with its currency for audit and reconciliation, and adds `amount_eur` converted at the rate on the order date. Aggregation uses the converted column, and the local amount is still there to prove what was actually transacted.

The subtlety worth raising unprompted is *which* rate. Converting at the transaction date freezes revenue as it was earned, so history never moves — which is what finance normally wants. Converting at a reporting-period rate makes periods comparable but restates the past every time rates move. Both are used; the failure is not choosing, or choosing per-report. Store the rate you used on the fact so the conversion is auditable.

The reasoning

Two tables at the same grain with the same measure name and different values is a definition problem wearing a modelling costume. Neither number is wrong — net revenue really is 150 and gross really is 160 — but the model gives a consumer no way to know which they have, so the disagreement surfaces as a trust problem in a meeting.

Duplicating the fact also duplicates the maintenance: two loads, two sets of tests, two places for the grain to drift. When they eventually diverge for a genuine reason — one picks up a new order source — nobody can tell whether the gap is the definition or a bug.

The corrected model stores the components — gross revenue and returns — on a single fact at order grain, and derives both metrics from it. `net_revenue` and `gross_revenue` become named definitions rather than table names, so a consumer chooses explicitly and both are guaranteed consistent because they come from the same rows.

Where those definitions live matters as much as the fact. A semantic layer or metrics store gives you one place where `net_revenue = gross_revenue - returns` is written down, versioned and reviewed — which is what stops a third definition appearing the next time a team needs a number quickly.

The reasoning

Additive measures survive pre-aggregation because a sum of sums is a sum. A distinct count has no such property: knowing 2 distinct customers on Monday and 1 on Tuesday tells you nothing about the distinct total, because you cannot tell whether they overlap. Here C1 appears on both days, so 2 + 1 + 1 gives 4 for 3 real customers.

It is not fixable by storing a different aggregate. Any pre-aggregate at day-region grain has already discarded the identities it would need, and the error is not a constant — it depends entirely on how much the population overlaps between the rows being combined, so it cannot even be corrected after the fact.

The corrected model computes the distinct count from the atomic fact, which retains `customer_id` and therefore answers the question at any grain. That is slower, and it is the only exact option.

Where the exact query is too slow, there are two real answers. Store a sketch — HyperLogLog — which *is* mergeable, so per-day sketches can be unioned to give an approximate monthly figure within a percent or so; most warehouses have native support. Or pre-compute the specific roll-ups the business asks for, each with its own distinct count, accepting that a new grain needs a new aggregate. What you must not do is store a distinct count and let people add it up.

The reasoning

**1 · Ask what it is for.** A schema cannot be reviewed in the abstract. Get three questions it must answer and the volume it runs at. Half the "problems" in an unfamiliar model are deliberate trade-offs for a workload you have not been told about, and leading with those makes the rest of your review sound guessed.

**2 · Read the grain of every table, out loud.** "One row per ___" for each one. The tables where you cannot finish the sentence are where the defects are, in my experience almost without exception. A table whose grain nobody can state is a table whose sums nobody can trust.

**3 · Trace one required question through the joins.** Follow it table by table and watch for a **fan-out** — a join from a coarse grain to a finer one that multiplies rows before an aggregate. This is the highest-yield single check in a review, because it produces wrong numbers with no error, so it survives testing.

**4 · Ask what changes and what must not.** Which values move over time, and would a change reach the past? A price on a product with no history, referenced by an old order line, means every historical order silently re-prices. That is one column and it is usually the most expensive defect in the diagram.

**5 · Then look at keys and constraints.** What is unique, what is mandatory, and which business rule is being enforced by hope rather than by the database. Only now, because a constraint on the wrong grain is not worth discussing.

The reasoning

**Undeclared grain.** Nobody wrote down what one row means, so two engineers write two correct-looking queries that return different totals. **Cost:** an argument nobody can settle from the data, usually discovered in a meeting rather than in a test.

**Fan-out on a join.** Joining a coarse table to a finer one multiplies rows before the aggregate. **Cost:** inflated revenue with no error message. This is the one I look for first in any unfamiliar model, because it survives every layer of testing that checks for exceptions rather than for values.

**A mutable attribute with no history.** A price, an address or a plan updated in place, referenced by rows that were written when it was different. **Cost:** the past changes. Last quarter’s report re-run today no longer matches the copy in the board pack, and no one can say which is right.

**A rule enforced in the application rather than by a constraint.** "We check before inserting." Two requests pass the check in the same millisecond and both write. **Cost:** the double booking, the duplicate order, the second payment — and the check is not wrong, it is just not a constraint, which is a distinction the failure teaches expensively.

**The thread through all four:** each produces *wrong data* rather than *an error*. Nothing crashes, nothing alerts, and the defect is found by a person who did not believe a number. That is why a model is reviewed against the questions it must answer rather than against a style guide.