Skip to content

docs(backfill): document null date_partition_parameter without depends_on_past#9710

Draft
kbammarito wants to merge 3 commits into
mainfrom
update-backfills-cookbook
Draft

docs(backfill): document null date_partition_parameter without depends_on_past#9710
kbammarito wants to merge 3 commits into
mainfrom
update-backfills-cookbook

Conversation

@kbammarito

@kbammarito kbammarito commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Description

What

Adds two rows to the backfill capability matrix in docs/cookbooks/backfilling_a_table.md for a day-partitioned table with an explicit date_partition_parameter: null and no depends_on_past — a config the matrix didn't cover:

  • offset 0 (default) → target is the whole table, so each date WRITE_TRUNCATEs everything; must be backfilled single-date (multi-date leaves only the last run). e.g. cohort_daily_churn_v1.
  • non-zero date_partition_offset → each run targets one shifted partition (run_date + offset), so multi-date is correct. e.g. clients_first_seen_28_days_later_v1.

Why

A multi-date backfill on the offset-0 case silently rewrites the whole table per date with no error (hit on DENG-11021). Docs-only.

Related Tickets & Documents

Reviewer, please follow this checklist

@kbammarito
kbammarito marked this pull request as ready for review July 21, 2026 22:38
@kbammarito
kbammarito requested a review from a team as a code owner July 21, 2026 22:38
github-actions[bot]

This comment was marked as outdated.

@kbammarito
kbammarito marked this pull request as draft July 22, 2026 01:45
@kbammarito
kbammarito marked this pull request as ready for review July 22, 2026 01:45
github-actions[bot]

This comment was marked as outdated.

Updated the documentation for backfilling a table to clarify handling of day-partitioned tables with specific parameters and conditions.
@kbammarito
kbammarito marked this pull request as draft July 22, 2026 01:56
@kbammarito
kbammarito marked this pull request as ready for review July 22, 2026 01:56
@kbammarito
kbammarito marked this pull request as draft July 22, 2026 01:56
@kbammarito
kbammarito marked this pull request as ready for review July 22, 2026 01:56

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds two rows to the backfill capability matrix in docs/cookbooks/backfilling_a_table.md, covering day-partitioned tables with an explicit date_partition_parameter: null and no depends_on_past — one for the default offset 0, one for a non-zero date_partition_offset.

I verified the referenced example tables' metadata (cohort_daily_churn_v1, clients_first_seen_28_days_later_v1, rolling_cohorts_v2) and the underlying logic in date_range.py, query.py, and backfill.py. The non-zero-offset row is accurate for the managed workflow. My one finding concerns the offset-0 row: the behavior it describes belongs to bqetl query backfill, not the managed bqetl backfill workflow this matrix documents — details inline.

| `shredder_mitigation: true` label on the table | ✅ Yes (field required) | [`telemetry_derived.desktop_retention_v1`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/desktop_retention_v1) | The entry must have `shredder_mitigation: true`; otherwise `initiate` stops. The metadata label and the entry's `shredder_mitigation` value must match. |
| Workgroup-restricted (access-controlled) dataset | ✅ Yes | | The staging and backup tables will mirror the prod table's IAM policy and its dataset access. |
| `depends_on_past: true` with a null `date_partition_parameter` | ✅ Yes (special handling) | [`telemetry_derived.clients_first_seen_v3`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/clients_first_seen_v3) | A whole-table refresh that depends on its own prior state can't be backfilled one partition at a time, so the normal day-by-day path is rejected by `validate_depends_on_past`. Instead, pass `--reinitialize-table` to rebuild the whole table from its `is_init()` query (the same logic `bqetl query initialize` uses), run in parallel per `sample_id` into staging and swapped in as a full-table replace on complete. Requires the table's `query.sql` to use the `is_init()` pattern. For a targeted single-column fill, an alternative is a per-partition `custom_query_path` backfill with `override_depends_on_past_null_partition: true`. See [Reinitializing a whole table](#reinitializing-a-whole-table) and [Per-partition custom-query backfill](#per-partition-custom-query-backfill-null-date_partition_parameter). |
| Day-partitioned table with `date_partition_parameter: null`, no `depends_on_past`, and `date_partition_offset: 0` (the default) | ⚠️ Single-date only | [`telemetry_derived.cohort_daily_churn_v1`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/cohort_daily_churn_v1) | <ul><li>Omitting `depends_on_past` defaults it to `false`, so this matches no guarded or null-param special-handling path.</li><li>An explicit `date_partition_parameter: null` with offset 0 targets the **whole table**: each backfill date runs a `WRITE_TRUNCATE` of everything, so a multi-date range re-truncates once per date and only the last run survives. Backfill these **single-date** (`start_date` equals `end_date`).</li><li>Distinct from *omitting* `date_partition_parameter` entirely, which defaults it to `submission_date` — a normal one-partition-per-day incremental where a full multi-date backfill is correct (e.g. [`rolling_cohorts_v2`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/rolling_cohorts_v2)).</li></ul> |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: This row describes the wrong command for this matrix. The matrix intro scopes it to the managed workflow ("The managed backfill workflow (bqetl backfill create / initiate)..."), but the managed workflow cannot complete this config at all — not even single-date. During complete, _copy_backfill_staging_to_prod takes the per-partition branch for a day-partitioned table (partitioning_type is DAY, not None), calls get_backfill_partition(date, None, 0, DAY) which always returns None for a null param with offset 0 (date_range.py:95), and raises ValueError("Null partition found completing backfill") (backfill.py:1412-1423). This is not date-range dependent: the test test_copy_backfill_staging_to_prod_null_partition_param_without_override_raises (tests/cli/test_cli_backfill.py:3700) proves it raises with start_date == end_date.

The "WRITE_TRUNCATE the whole table per date, only the last run survives, so backfill single-date" behavior you describe is a property of the unmanaged bqetl query backfill command (query.py:516-525, --replace) — the DENG-11021 scenario — not of bqetl backfill. As written, a reader who follows this row (set start_date == end_date and run the managed backfill) will still hit Null partition found at complete.

Two things to fix: (1) ⚠️ elsewhere in this table means "override required" (an enforced gate); using it for an unenforced "single-date only" advisory overloads the symbol; and (2) the guidance should state that this config isn't completable via the managed workflow and that the single-date rule applies to bqetl query backfill. Note that cohort_daily_churn_v1's own backfill.yaml is a managed multi-date Initiate entry (DENG-11021) — exactly the entry the managed complete would reject.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds two rows to the backfill capability matrix in docs/cookbooks/backfilling_a_table.md, covering a day-partitioned table with an explicit date_partition_parameter: null and no depends_on_past: offset 0 (whole-table WRITE_TRUNCATE, single-date only) and non-zero date_partition_offset (per-partition, multi-date safe).

I verified the technical claims against the code. get_backfill_partition (bigquery_etl/backfill/date_range.py:95) returns None — i.e. the whole-table destination — exactly when date_partition_parameter is None and date_partition_offset == 0, and a decorated $partition destination otherwise; resolve_date_partition_parameter defaults an omitted param to submission_date. The managed initiate flow replays the query per date over the entry range with no override for the plain offset-0 case, so the staging table is re-truncated per date, matching the "only the last run survives" claim. The three example tables' metadata.yaml all match their described configs. The claims are accurate; my one comment is a documentation-clarity suggestion about how the new ⚠️ row relates to the table's enforcement framing.

| `shredder_mitigation: true` label on the table | ✅ Yes (field required) | [`telemetry_derived.desktop_retention_v1`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/desktop_retention_v1) | The entry must have `shredder_mitigation: true`; otherwise `initiate` stops. The metadata label and the entry's `shredder_mitigation` value must match. |
| Workgroup-restricted (access-controlled) dataset | ✅ Yes | | The staging and backup tables will mirror the prod table's IAM policy and its dataset access. |
| `depends_on_past: true` with a null `date_partition_parameter` | ✅ Yes (special handling) | [`telemetry_derived.clients_first_seen_v3`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/clients_first_seen_v3) | A whole-table refresh that depends on its own prior state can't be backfilled one partition at a time, so the normal day-by-day path is rejected by `validate_depends_on_past`. Instead, pass `--reinitialize-table` to rebuild the whole table from its `is_init()` query (the same logic `bqetl query initialize` uses), run in parallel per `sample_id` into staging and swapped in as a full-table replace on complete. Requires the table's `query.sql` to use the `is_init()` pattern. For a targeted single-column fill, an alternative is a per-partition `custom_query_path` backfill with `override_depends_on_past_null_partition: true`. See [Reinitializing a whole table](#reinitializing-a-whole-table) and [Per-partition custom-query backfill](#per-partition-custom-query-backfill-null-date_partition_parameter). |
| Day-partitioned table with `date_partition_parameter: null`, no `depends_on_past`, and `date_partition_offset: 0` (the default) | ⚠️ Single-date only | [`telemetry_derived.cohort_daily_churn_v1`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/cohort_daily_churn_v1) | <ul><li>Omitting `depends_on_past` defaults it to `false`, so this matches no guarded or null-param special-handling path.</li><li>An explicit `date_partition_parameter: null` with offset 0 targets the **whole table**: each backfill date runs a `WRITE_TRUNCATE` of everything, so a multi-date range re-truncates once per date and only the last run survives. Backfill these **single-date** (`start_date` equals `end_date`).</li><li>Distinct from *omitting* `date_partition_parameter` entirely, which defaults it to `submission_date` — a normal one-partition-per-day incremental where a full multi-date backfill is correct (e.g. [`rolling_cohorts_v2`](https://github.com/mozilla/bigquery-etl/tree/main/sql/moz-fx-data-shared-prod/telemetry_derived/rolling_cohorts_v2)).</li></ul> |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: The ⚠️ in this row means something different from the other ⚠️ rows and it's worth making that explicit. The intro frames the table around what the tool enforces ("some are allowed only with an explicit override flag", and line 29: "The rejections are enforced by validate_table_metadata"), and the other ⚠️ rows are Override required — the backfill code stops you until you set a flag. This row is the opposite: a multi-date range on an offset-0 null-param table is not rejected — it runs without error and silently keeps only the last date (which is exactly the DENG-11021 footgun this PR documents). The note says "only the last run survives" but never states that the tool won't stop you, so a reader scanning the Allowed? column could reasonably assume this config is guarded like the Override required rows. Consider adding a few words to the note making the unenforced/silent nature explicit, e.g. "the backfill code does not reject a multi-date range here — it runs with no error and silently keeps only the last date."

@kbammarito
kbammarito marked this pull request as draft July 22, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant