#73 fix(silver): stop FY0/P0 — treat missed calendar join as NULL, not 0

[Anonymous]

Originally created by: grynn-in

Fixes [#71].

Root cause (two bugs combined)

silver_gl_entries set fiscal_year/fiscal_period via coalesce(fp.fiscal_year, extract_year(accounting_date)), where fp is a LEFT JOIN to the fiscal-calendar lookup. Every GL row landed in FY0/P0 because:

1. ClickHouse fills unmatched LEFT JOIN numeric columns with 0, not NULL (join_use_nulls defaults to 0). A missed calendar join gives fp.fiscal_year = 0, and coalesce(0, ...) returns 0 — so the accounting-date fallback from [#65] never runs.
2. The calendar join misses anyway for the demo: silver_fiscal_periods.calendar_id = 'Standard' while entity_fiscal_calendars map to 'Fiscal'.

Fix

Wrap the calendar values in nullIf(..., 0) so the no-match sentinel becomes NULL and the posting-date fallback applies. Precedence: calendar (when matched) → accounting-date derivation.

coalesce(nullIf(fp.fiscal_year, 0),   extract_year(gae.accounting_date))  as fiscal_year,
coalesce(nullIf(fp.fiscal_period, 0), extract_month(gae.accounting_date)) as fiscal_period,

Verification (D365 demo)

• silver_gl_entries.fiscal_year: 0 → 2024 (862 rows)
• gold_trial_balance: now spans 2024, periods 1–12
• End-to-end through the Excel API:
  POST /api/method/konsol.api.epm_batch [{entity:AMHQ, year:2024, period:1, account:1010}] → {"values":[3045306.0]}

Follow-up (separate)

The calendar_id mismatch (Standard vs Fiscal) means the calendar-derived path never matches today, so fiscal currently always comes from the posting date. Correct for calendar fiscal years (the demo) but wrong for offset fiscal years — worth its own fix.

[Anonymous]

Originally posted by: grynn-in

Independent review — APPROVE

The central risk — could nullIf(..., 0) discard a legitimate value from a successful calendar match? — does not materialize:

• fiscal_period = calendar_month, and in silver_fiscal_periods calendar_month = month_offset + 1 with month_offset ∈ range(12) → always 1..12, never 0. The period-0/OPN concept exists only as a synthetic row in gold_period_hierarchy (from the fiscal_extra_periods var) and never flows through this join, so there's no collision.
• fiscal_year = toYear(year_start_date) → a real 4-digit year for any genuine calendar row, never 0.

So nullIf(...,0) can only fire on the ClickHouse LEFT-JOIN zero-fill (the no-match case) — exactly the intent. coalesce(Nullable(T), toYear/toMonth(...)) collapses back to non-nullable, and gold casts via assumeNotNull-wrapped cast_to_uint16/uint8, so no nullability/type regression.

Optional follow-up (non-blocking): once the calendar_id-mismatch fix lands and the join matches for real, a silent fallback could re-mask a future re-break of the join. A small dbt test asserting fallback usage is ~0 for D365-calendared entities would turn that into a failing test instead of silent FY0 again.

[Anonymous]

Originally posted by: grynn-in

Closing as a duplicate. Current main already fixes this via [#65] — if(fp.fiscal_year != 0, fp.fiscal_year, extract_year(accounting_date)), functionally identical to this PR's coalesce(nullIf(fp.fiscal_year, 0), ...), same root cause (ClickHouse LEFT-JOIN 0-fill). My clone predated [#65]. The seed fix that makes the calendar actually match lives in [#76].

[Anonymous]

Ticket changed by: grynn-in

• status: open --> closed