Skip to content

Collect system.query_views_log: materialized-view execution - #33

Open
CamiloSierraH wants to merge 7 commits into
mainfrom
cami/query-views-log-collector
Open

CamiloSierraH wants to merge 7 commits into
mainfrom
cami/query-views-log-collector

Conversation

@CamiloSierraH

@CamiloSierraH CamiloSierraH commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Why

A materialized view that throws, or that "succeeds" while writing nothing, is invisible in query_log — the parent INSERT reports success either way. system.query_views_log is the only place an MV's own execution is recorded, and the bundle explicitly listed it under "deliberately does not contain".

Reviewing the closed escalations that touch query_views_log turned up five recurring questions, each already answered by support engineers running ad-hoc hourly aggregates against this table:

Pattern Signature
Silent row drop QueryFinish, exception_code = 0, read_rows > 0, written_rows = 0
MV push fails after the base part committed status = 'ExceptionWhileProcessing' (60 / 241 / 252 / 341)
Per-MV memory regression after an upgrade peak_memory_usage median/max vs flat written_rows
MVs burning CPU view_duration_ms
Divergence between hops of a chain written_rows per view per bucket

What

1. The collector. system.query_views_log_3_days in all three modes, 1-hour buckets keyed by view, target, type, status and exception_code. Per bucket: executions, zero_write_executions, row/byte sums, view duration sum + max, peak memory as avg/median/max, and a 500-char sampled exception.

2. The pre-pass reads it. Six checks in inspect_bundle.py, so MV failures reach triage instead of waiting for someone to go looking.

3. Skill docs. A file-guide.md entry, HC-5.9 and HC-7.6/7.7/7.8, patterns P-35 (MV runs clean but writes nothing) and P-36 (MV push throws after the base part committed), a bundle-layout.md row, P-33/P-34 upgraded from hedged references to the real file, and query_views_log struck from the never-collected list.

Keeping the query light

query_views_log is one row per (insert × view) and rivals part_log in size, so the shape matters more than the window:

  • Aggregate, never raw rows. GROUP BY state stays bounded because every key is low-cardinality. initial_query_id and the exception text are deliberately not keys — that is the part_name regression part_log documents.
  • No uniqExact on the exception string. exception_code is a group key, so any(exception) is already representative; a hash set of full messages would buy nothing and cost real CPU.
  • stack_trace, view_query and ProfileEvents are never read — the bulk of the table's bytes.
  • gov hashes in the outer select, over the aggregated result, so SHA256 runs once per output row instead of once per source row.

The pre-pass checks

The two that required care:

  • Failures are attributed, not counted. A failed INSERT writes an ExceptionWhileProcessing row for every view in the pipeline with the same message. Counting rows per view reports four broken MVs when one is broken. The check names the highest-count view as the culprit and reports the rest as blast radius.
  • Zero-write is read as a change, not an absolute. A filtering MV legitimately writes nothing forever, so the flat > 0.9 ratio in HC-7.7 is not what got implemented. Only a view with a mixed bucket history — it wrote, then stopped — warns. Views that never wrote get one info line.

The rest: an HC-0 coverage gate (MVs exist in system.tables but no query_views_log → log_query_views = 0; without this, "no MV findings" is indistinguishable from "no telemetry"), a per-view memory step change (≥ 3× median peak across the window halves with writes flat, ≥ 6 buckets required), an mv failures N flag on the existing incident timeline, and a top-10 views table — the only output that describes the MV topology on a bundle where nothing is wrong.

Three things found by testing rather than reading

  • view_uuid is always the zero UUID. It was in the design as a recreate-detector; on 26.7.5.10 the server writes zeros into it for every view, TO-target and inner-table alike, while system.tables carries the real one. Dropped. For an MV declared without TO, view_target is db.`.inner_id.<uuid>` and that changes on a recreate.
  • The pipeline-wide exception marking above — reproduced with four MVs where only one was faulty.
  • merge() with no matching table fails with 636 CANNOT_EXTRACT_TABLE_STRUCTURE, not 60. That is the recognisable "not collected" when <query_views_log> is unconfigured.

merge(system, '^query_views_log') rather than the bare table so a post-upgrade query_views_log_0 still counts — the pre-upgrade baseline is exactly what a memory regression needs.

Verification

SQL against 22.8.21.38 (the floor) and 26.7.5.10:

  • all six files execute, including GROUP BY ALL, clusterAllReplicas and the gov hashing
  • onprem, cloud and gov collect end-to-end; the 23.11 rung is selected correctly
  • both signatures reproduced in a purpose-built MV workload — a filtering MV at zero_write_executions = 5/5, a broken JOIN at exception_code = 60
  • gov output contains no raw identifiers and no exception text

Pre-pass against four bundles: a real onprem one (correct culprit attribution across four views where one was faulty), a gov one (hashed labels, identical ratios), one with the file removed (coverage gate fires), and a synthetic fixture for the stopped-writing and memory-step branches — where a steady view correctly raised nothing. --json mode checked on all four.

make test passes, including TestShippedQueryDefaultWindows and TestGovQueries_NoRawIdentifiersOrDDL.

Correction (human review)

The description above and the first version of the docs said an MV that throws is "invisible in query_log — the parent INSERT reports success either way." That is wrong. By default the INSERT fails with … while pushing to view db.mv; only under materialized_views_ignore_errors = 1 does it succeed, and then text_log records "Error is ignored because the setting … is enabled". The silent-drop class (P-35) is the one invisible in query_log. Corrected everywhere in the fourth commit, together with the pre-pass counting fixes: failures grouped by the view the message names and counted once (not once per sibling), a threshold on the timeline flag, one finding per culprit, rolled-back written_rows excluded, the sparse-MV false positive on HC-7.7, and refreshable MVs excluded from the coverage gate.

Review follow-up

Six findings, all addressed in the third commit and each checked against a live server first:

# Finding Verdict Fix
1–2 gov split view_target on every dot; .inner_id.<uuid> targets hashed to a bare backtick real split at the first dot, strip backticks; hashes verified to equal system.tables's for TO and inner targets
3 QueryStart rows counted as executions theoretical — the server writes no QueryStart rows for views (checked) skipped anyway, as the query_log pass does
4 attribution by row count ties on every sibling real — and the fix is exact, not heuristic: every sibling's message says while pushing to view <culprit> parsed from the text; strict count lead as fallback; otherwise no view is named
5 stopped-writing lost bucket order — a view that started writing fired too real ordered per-hour history: last write, then ≥ 2 later read-only hours; fixture with a started-writing view stays silent
6 HC-7.7 documented the flat ratio the code deliberately doesn't implement real row rewritten to the ordered rule, with the reason the ratio is not it

Re-run on the real bundle (mv_join named from the text, three siblings correctly reported as blast radius) and on two fixtures (--json clean); make test passes.

🤖 Generated with Claude Code

CamiloSierraH and others added 2 commits September 25, 2026 16:21
A materialized view that throws, or that "succeeds" while writing nothing,
is invisible in query_log — the parent INSERT reports success either way.
system.query_views_log is the only place an MV's own execution is recorded,
and the bundle explicitly did not collect it.

Adds system.query_views_log_3_days to all three modes, aggregated into
1-hour buckets by view, target, type, status and exception_code. Per bucket:
executions, a zero_write_executions counter (read_rows > 0 AND
written_rows = 0), row/byte sums, view duration sum and max, peak memory as
avg/median/max, and a 500-char sampled exception.

Design notes:

- Aggregate, not raw rows. query_views_log is one row per (insert x view)
  and rivals part_log in size. GROUP BY state stays bounded because the keys
  are all low-cardinality; initial_query_id and the exception text are
  deliberately not keys (the part_name regression part_log documents).

- merge(system, '^query_views_log') so a post-upgrade query_views_log_0
  still counts — the pre-upgrade baseline is exactly what a per-view memory
  regression needs. With no such table it fails with 636
  CANNOT_EXTRACT_TABLE_STRUCTURE, a recognisable "not collected".

- No uniqExact on the exception string: exception_code is a group key, so
  any(exception) is already representative and a hash set of full messages
  would buy nothing.

- peak memory as median/max, not just sum: a sum tracks insert volume and
  hides a single view that got more expensive per block.

- view_uuid is NOT collected. Verified on 26.7.5.10 that the server writes
  the zero UUID into it for every view while system.tables carries the real
  one. For an MV declared without TO, view_target is db.`.inner_id.<uuid>`
  and that does change on a recreate.

- Every aggregate argument is qualified through the table alias. Without it
  `sum(view_duration_ms) AS view_duration_ms` shadows the column and a later
  max() over it is rejected with ILLEGAL_AGGREGATION 184.

- gov splits view_name/view_target on '.' and hashes each half, so the
  PrintGovNameMapping CSV can reverse them, and ships no exception text.
  Hashing sits in the outer select over the aggregated result, so SHA256
  runs once per output row rather than once per source row.

Skill updates: new file-guide entry, HC-5.9 and HC-7.6/7.7/7.8, patterns
P-35 (MV runs clean but writes nothing) and P-36 (MV push throws after the
base part committed), a bundle-layout row, and query_views_log removed from
running-the-tool's "deliberately not collected" list. Both new patterns
record the trap found while testing: a failed INSERT marks EVERY view in the
pipeline with ExceptionWhileProcessing and the same message, not just the
one that broke.

Verified against ClickHouse 22.8.21.38 and 26.7.5.10: all six files run, and
onprem/cloud/gov collect end-to-end with the silent-drop and MV-failure
signatures reproduced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The query_views_log collector landed without a reader, so MV failures — the
highest-severity thing that file records — only reached an analyst who went
looking. Adds six checks to the deterministic pre-pass.

Coverage gate (HC-0): cross-checks system.tables for MaterializedView engines
against the file. If the server has MVs and query_views_log is absent or empty,
log_query_views = 0 or <query_views_log> is unconfigured, and "no MV findings"
means no telemetry rather than a healthy pipeline. This is the check that stops
a false all-clear, so SKILL.md step 0 now asks for it in the coverage lines.

Failures (HC-7.6/P-36), attributed correctly: a failed INSERT writes an
ExceptionWhileProcessing row for EVERY view in the pipeline carrying the same
message. Naively counting rows per view reports four broken MVs when one is
broken, so the check names the view with the highest count as the culprit and
reports the rest as blast radius.

Zero-write as a CHANGE, not an absolute (HC-7.7/P-35): a filtering MV
legitimately writes nothing forever, which is why the flat ratio in HC-7.7 is
not what gets implemented. Only a view with a mixed bucket history — it wrote,
then stopped — raises a warning. Views that never wrote get one info line.

Per-view memory step change (HC-5.9/P-54): median peak memory of the window's
second half against its first, flagged at >= 3x only while written rows stay
within +/-25%. Needs >= 6 buckets, so it stays quiet on thin windows.

Timeline flag: MV push failures per hour feed the existing incident table as an
"mv failures N" flag, which is what correlates them with Keeper loss or stalled
merges. No new column, so bundles without MVs are unchanged.

Usage table: top 10 views by executions with written rows, zero-write %, failed
pushes, median peak memory and max duration — the one output that describes the
MV topology on a healthy bundle, where no finding fires.

Works unchanged in gov mode: view_label() falls back to the hashed
view_database/view_table pair, and every count and ratio is identical.

Verified on a real onprem bundle (correct culprit attribution across four views
where one was faulty), a gov bundle, a bundle with the file removed, and a
synthetic fixture for the stopped-writing and memory-step branches — where a
steady view correctly raised nothing. make test passes; --json mode checked on
all four.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues affect collector correctness, diagnostics, and documentation alignment.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 Medium severity · 1 Low severity

Open (6)
What changed in this PR

Adds system.query_views_log collection and materialized-view diagnostics across ClickHouse deployment modes, including failure, zero-write, memory, and coverage analysis.

Changes:

  • Adds versioned collectors for on-prem, cloud, and government environments.
  • Extends bundle inspection and reporting with MV health checks.
  • Updates documentation, patterns, bundle layout, and default-window tests.

Unresolved issues remain in collector target parsing, execution accounting, exception attribution, temporal zero-write detection, coverage interpretation, and health-check documentation alignment.

File Summary
skills/​clickhouse-diagnostic/​SKILL.md Documents MV telemetry coverage.
skills/​clickhouse-diagnostic/​scripts/​inspect_bundle.py Adds MV analysis and reporting.
skills/​clickhouse-diagnostic/​references/​running-the-tool.md Documents the MV collection window.
skills/​clickhouse-diagnostic/​references/​known-patterns.md Adds MV failure and zero-write patterns.
skills/​clickhouse-diagnostic/​references/​health-checks.md Adds MV health-check guidance.
skills/​clickhouse-diagnostic/​references/​file-guide.md Documents the new bundle file.
skills/​clickhouse-diagnostic/​references/​bundle-layout.md Defines MV archive schema and redaction.
README.md Lists the collector and window.
queries.onprem/​system.query_views_log_3_days.sql Adds the baseline on-prem collector.
queries.onprem/​23.11.1.0/​system.query_views_log_3_days.sql Adds the versioned on-prem collector.
queries.gov/​system.query_views_log_3_days.sql Adds the redacted government collector.
queries.gov/​23.11.1.0/​system.query_views_log_3_days.sql Adds the versioned redacted government collector.
queries.cloud/​system.query_views_log_3_days.sql Adds the cloud collector.
queries.cloud/​23.11.1.0/​system.query_views_log_3_days.sql Adds the versioned cloud collector.
internal/​query/​window_test.go Verifies the new default window.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread queries.gov/23.11.1.0/system.query_views_log_3_days.sql Outdated
Comment thread queries.gov/system.query_views_log_3_days.sql Outdated
Comment thread skills/clickhouse-diagnostic/scripts/inspect_bundle.py
Comment thread skills/clickhouse-diagnostic/scripts/inspect_bundle.py Outdated
Comment thread skills/clickhouse-diagnostic/scripts/inspect_bundle.py Outdated
Comment thread skills/clickhouse-diagnostic/references/health-checks.md Outdated
…rdered stop detection

Six findings on #33, each checked against a live 26.7 server first.

Gov hashing split view_name / view_target on EVERY dot. An MV declared with an
ENGINE writes to db.`.inner_id.<uuid>` — a name with dots inside — so
splitByChar('.', view_target)[2] was a bare backtick for every inner target,
hashing to one value that matched nothing in the mapping CSV. Both gov files
now split at the first dot only and strip backticks; verified that the hashed
halves equal hex(SHA256(database)) / hex(SHA256(name)) of the matching
system.tables row for a TO target and an inner target alike.

The pre-pass counted every row as an execution. The server writes only
QueryFinish / ExceptionWhileProcessing for views (checked: no QueryStart rows
exist), so this was theoretical — skipped anyway, as the query_log pass does.

Attribution picked the view with the most failed pushes, but a failed INSERT
gives every sibling an identical row, so counts tie and the sort order chose.
The sampled exception says which view threw — "… while pushing to view db.mv"
— on every sibling's row and well inside the 500-char cap (position ~200 of
287 in the real bundle). The pre-pass now parses that; without text (gov) it
names a view only on a strict count lead; with neither it lists the failing
views and says attribution is unavailable. On a fixture with three siblings
at 7 failures each it names the one the text names.

Stopped-writing kept counts of writing and zero-write buckets, so a view that
STARTED writing (zeros first) fired the same warning. It now keeps the
ordered per-hour history: the last hour with writes, then the run of later
hours that read rows and wrote none; two or more of those is the finding, and
the message quotes the hours. A view that started writing has an empty
trailing run and is not flagged (fixture-tested).

HC-7.7 still described the flat ratio the code deliberately does not
implement; it now describes the ordered rule and says why the ratio is not
it. HC-7.6 says how the culprit is attributed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The cloud collector has an unsupported syntax path, and MV coverage and analysis logic need corrections.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (6)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Half-window logic does not enforce the documented calendar-day boundary

skills/​clickhouse-diagnostic/​references/​health-checks.md:112

The implementation splits the available memory buckets into chronological halves (ms[:len(ms)//2] and the remainder) and never checks a calendar/day boundary. The new HC-5.9 wording therefore promises a narrower condition than the pre-pass actually applies and can make an ordinary within-day step look like an upgrade regression; document the implemented half-window condition (and its six-bucket minimum), or change the code to enforce a day boundary.

This issue also appears on line 137 of the same file.

Medium severity Zero-row results are misclassified as absent query-view logs

skills/​clickhouse-diagnostic/​scripts/​inspect_bundle.py:689

read_jsonl returns an empty list both when the result file is absent and when a successfully collected file contains zero rows. Thus an idle server with configured query_views_log and existing MVs is reported as log_query_views = 0/not configured, even though the collector ran successfully and simply saw no MV executions in the window. Check the file path separately (and reserve HC-0 for an absent/failed collector) before emitting this coverage warning.

Low severity Per-view statistics are incorrectly described as aggregate metrics

skills/​clickhouse-diagnostic/​references/​known-patterns.md:215

The new collector groups by view, so max_view_duration_ms and median_peak_memory_usage are already per-view statistics; neither is summed across views (and summing maxima or medians would not be meaningful). This wording can lead the P-34 analysis to compute an invalid aggregate instead of comparing the per-view values and summing only additive counters such as duration or rows.

Comment thread queries.cloud/system.query_views_log_3_days.sql
CI's gofmt step failed on the one Go line this branch adds: the new key in
TestShippedQueryDefaultWindows was one space short of the aligned value
column. Local runs used make test, which does not run gofmt — the CI
recipe does, so it caught it. No behaviour change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@davidhogeg-ch davidhogeg-ch self-assigned this Sep 29, 2026
@davidhogeg-ch
davidhogeg-ch self-requested a review September 29, 2026 01:24
@davidhogeg-ch davidhogeg-ch removed their assignment Sep 29, 2026

@davidhogeg-ch davidhogeg-ch 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.

Request changes. The collector SQL looks sound. The gov split fix and the attribution-from-message approach are good. The problems below are in the pre-pass counting and in one wrong claim in the docs.

1. When an MV throws under default settings, the parent INSERT fails. It does not report success.
README.md:21, file-guide.md:248 and known-patterns.md:229 (P-36 "You see") all say the INSERT reports success either way. Unless materialized_views_ignore_errors = 1 is set, InsertDependenciesBuilder.cpp sends the exception on to the INSERT, which fails. Repro on Cloud 26.6.1.2191, with a source table, mv_ok, and mv_bad (which throws via throwIf):

INSERT query_log (parent) query_views_log rows landed
default ExceptionWhileProcessing 395, … while pushing to view pr33.mv_bad. mv_bad ExceptionWhileProcessing written=0 · mv_ok ExceptionWhileProcessing written=2 src [1,2] · dst_ok none · dst_bad none
materialized_views_ignore_errors=1 QueryFinish 0 mv_bad ExceptionWhileProcessing · mv_ok QueryFinish written=1 src [3] · dst_ok [3]

So "invisible in query_log" holds only for P-35 and for ignore_errors=1. P-36 needs both cases: by default the INSERT fails with "while pushing to view X"; with ignore_errors=1 the INSERT is clean and there is a text_log "Error is ignored" warning. The row showing every sibling marked with the same error also only happens under the default setting.

2. Failures are still counted once per sibling view, not once per failed INSERT.

  • inspect_bundle.py:719: tl(h)["mv_failed"] += ex adds a count for every view.
  • :775: total sums across the failing views and drives critical at ≥ 100.
  • :740: views_failing includes the siblings.

In a fixture with 1 broken view, 3 siblings and 30 failed INSERTs, the pre-pass reports 120 failures, critical, and 4 failing views. Count the culprit's failures, or take the max per hour.

3. The incident flag for MV failures has no threshold (:857). The list keeps only the first 48 hours, oldest first (:864). In a fixture with one failed push per hour for 72 h plus a 900-exception TOO_MANY_PARTS hour at the end, the TOO_MANY_PARTS hour disappears from the table. On main it shows. Please add a threshold the way the other flags have one.

4. Two unrelated broken MVs are reported as one culprit. named.most_common(1) (:770) names one view. The other failing view is then reported as "carry the same message", which is false when its message names itself. Group failing views by the view named in their message and report each group. Also, v["exc"] keeps only the first message per view.

5. Written-row totals include rows that were never committed (:710). On an ExceptionWhileProcessing row, written_rows counts rows that never landed (mv_ok above: written=2, target empty). Only add written rows from QueryFinish rows. Otherwise the summary's rows written, the top-views table and the HC-7.8 comparison all over-report.

6. HC-7.7 flags sparse filtering MVs as "stopped" (:807). A view that writes every ~6 h is flagged "stopped writing … not a filtering MV" at the end of the window. The last, still-filling bucket counts toward the 2 trailing hours. Leave out the final bucket, and compare the trailing gap with the view's own largest earlier gap.

7. Refreshable MVs trigger a false HC-0 warning (:677). The gate counts every engine = 'MaterializedView'. A refreshable MV has no insert dependency, so it never writes to query_views_log. A server whose only MVs are refreshable gets the "log_query_views = 0" warning. The wording also overstates the cause: an empty file can also come from no inserts in the window, log_queries_min_type, or log_queries_min_query_duration_ms.

Nits:

  • health-checks.md:112: says "across a day boundary", but the code splits the hourly buckets in half by count (:828).
  • Gov first-dot split: a quoted database name containing a dot (`a.b`.mv) splits into a / b`.mv. It's rare, but the comment's "an unquoted database name cannot contain one" doesn't cover it, because getFullTableName quotes the name.

… per sibling

Seven findings and two nits from review on #33, each checked live first.

1. The docs said an MV that throws is "invisible in query_log — the parent
INSERT reports success either way". Wrong. By default the INSERT fails with
"… while pushing to view db.mv" (reproduced: Code 395 from a throwIf view);
only under materialized_views_ignore_errors = 1 does the INSERT succeed, and
then text_log says "Error is ignored because the setting … is enabled" and
only the throwing view gets a row. The sibling marking — every view in the
pipeline carrying the same message — happens only in the default case.
README, file-guide, P-36, HC-7.6 and bundle-layout now say both shapes.

2. Failures were counted once per sibling: the timeline summed every view's
rows, `total` summed across views and drove critical at 100, and
views_failing counted siblings. One broken view with three siblings and 30
failed INSERTs reported 120 failures, critical, four failing views. Failing
views are now grouped by the view their message names; a culprit's failures
are its own row count; the per-hour timeline value is the max over views,
never the sum. Same fixture: 30, warning, one failing view, three siblings.

3. The "mv failures" incident flag had no threshold, so one failed push an
hour for 72 hours filled the 48-row table and pushed out a 900-exception
TOO_MANY_PARTS hour. Threshold of 10 per hour, like the other flags; HC-7.6
still reports the total. Fixture: the storm hour is back in the table.

4. Two unrelated broken views were reported as one culprit plus a false
"carries the same message". Every failure row's message is kept now, not the
first per view, and one finding is emitted per named culprit; a culprit with
no siblings is described as the ignore_errors shape.

5. written_rows on an ExceptionWhileProcessing row counts rows that never
landed (reproduced: sibling written=1, target empty). Row and byte totals,
zero-write counts and the write history come from QueryFinish rows only.

6. HC-7.7 flagged a view that writes every six hours as "stopped" at the end
of the window. The still-filling final bucket is left out, and the trailing
quiet run must exceed the view's own largest earlier gap between writes.
Fixture: six-hourly view silent, hourly-then-stopped view flagged.

7. The coverage gate counted refreshable MVs, which never write
query_views_log, so a server whose only MVs are refreshable got the warning.
They are excluded by their DDL (REFRESH EVERY / AFTER; in gov, where there
is no DDL, the wording hedges), and the causes listed now include no inserts
in the window and the log_queries_min_* filters.

Nits: HC-5.9 says "first half against second half of the hourly buckets"
rather than "across a day boundary", which is what the code does; the gov
split handles a quoted database name — `a.b`.mv prints exactly so — by
splitting at the dot after the closing backtick when the name starts quoted.
Verified live for `a.b`.mv, graph_demo.mv_inner and a plain name.

Six fixtures under the reviewer's scenarios all run clean in --json too.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@CamiloSierraH

Copy link
Copy Markdown
Collaborator Author

@davidhogeg-ch — thank you, every point held up when I reproduced it, and one of them was a plain factual error in my docs. All addressed in 75160d2.

1. "The INSERT reports success either way" — wrong, fixed. Reproduced your table on 26.7 with a throwIf view: by default the INSERT fails (Code: 395 … while pushing to view), and only under materialized_views_ignore_errors = 1 is it clean, with the text_log "Error is ignored because the setting … is enabled" line. README, file-guide, P-36, HC-7.6 and bundle-layout now describe both shapes, including that the sibling marking exists only in the default case.

2. Counted once, not per sibling. Failing views are grouped by the view their message names; a culprit's failures are its own row count; the timeline takes the per-hour max over views. Your fixture (1 broken + 3 siblings + 30 failed INSERTs) now reads: failed 30 push(es), warning, 1 failing view, "3 sibling view(s) carry its message".

3. Threshold. mv failures flags an hour at ≥ 10, like the other flags; HC-7.6 still reports the total. Your fixture (72 h × 1 push + a 900-exception hour): the TOO_MANY_PARTS hour is back in the table and no mv failures flags appear.

4. One finding per culprit. Every failure row's message is kept, not the first per view. Fixture with mv_x (+1 sibling) and an unrelated mv_y: two findings; mv_y, having no siblings, is described as the ignore_errors shape rather than as "carries the same message".

5. Rolled-back writes excluded. Rows, bytes, zero-write counts and the write history come from QueryFinish rows only. Reproduced your sibling written=1, target empty.

6. Sparse filtering MV. The final still-filling bucket is excluded and the trailing quiet run must exceed the view's own largest earlier gap. Fixture: a six-hourly view is silent; an hourly-then-stopped one is flagged, with "longest earlier quiet gap was 0 hour(s)" in the evidence.

7. Refreshable MVs. Excluded from the coverage gate by their DDL (REFRESH EVERY|AFTER); a refreshable-only server raises nothing; the wording now lists no-inserts-in-window and the log_queries_min_* filters as causes. In gov (no DDL) it says the kind cannot be told apart.

Nits. HC-5.9 now says what the code does (first half vs second half of the hourly buckets). The gov split handles a quoted database: when the name starts with a backtick the separator is the dot after the closing backtick — verified live that \a.b`.mv→a.b/mv, and that graph_demo.mv_inner's .inner_id.…` target still splits correctly.

Six fixtures under your scenarios live in the commit message; make test (incl. the gov leak guard over the new SQL) passes.

CamiloSierraH and others added 2 commits September 29, 2026 09:10
The per-culprit code tally merged whichever sibling was processed before
the culprit, so the real bundle read UNKNOWN_TABLE(60)=4 beside "failed 3
push(es)". The culprit's own rows are now authoritative for both numbers;
sibling codes are used only when the named view has no rows of its own
(it is not in the bundle's window).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ry_views_log branch

Two additive conflicts: the README collector table (this branch's
query_views_log row beside main's view_refreshes and data_skipping_indices
rows) and HC-7, where main's 7.6 (view_refreshes) landed first — this
branch's rows are renumbered 7.7 failures, 7.8 stopped writing, 7.9 chain
divergence, and inspect_bundle's finding tags follow. The merge note in the
PR said whichever landed second would renumber; this is that.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@CamiloSierraH

Copy link
Copy Markdown
Collaborator Author

Rebased onto main after #32 and #34 merged. One consequence for anyone reading the review above against the current docs: #34's HC-7.6 (refreshable MV not refreshing) landed first, so this branch's rows are now HC-7.7 (MV push failed), HC-7.8 (stopped writing) and HC-7.9 (chain divergence); the pre-pass tags follow. HC-5.9 is unchanged. Also a small follow-up in fbeaea2: a culprit's error-code tally now comes from its own rows — it had absorbed a sibling's count in one ordering (=4 beside failed 3).

@davidhogeg-ch davidhogeg-ch 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.

LGTM

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.

3 participants