fix(analytics): move the DuckDB live suite out of tests/integration, and add the BI docs
CI (SIAX Cloud) / security (push) Successful in 28s
CI (SIAX Cloud) / integration (push) Successful in 42s
Security Scan (SIAX Cloud) / security (push) Successful in 21s
CI (SIAX Cloud) / quality (push) Successful in 1m2s
CI (SIAX Cloud) / security (pull_request) Successful in 23s
CI (SIAX Cloud) / integration (pull_request) Successful in 40s
CI (SIAX Cloud) / quality (pull_request) Successful in 58s
Security Scan (SIAX Cloud) / security (pull_request) Successful in 20s
CI (SIAX Cloud) / security (push) Successful in 28s
CI (SIAX Cloud) / integration (push) Successful in 42s
Security Scan (SIAX Cloud) / security (push) Successful in 21s
CI (SIAX Cloud) / quality (push) Successful in 1m2s
CI (SIAX Cloud) / security (pull_request) Successful in 23s
CI (SIAX Cloud) / integration (pull_request) Successful in 40s
CI (SIAX Cloud) / quality (pull_request) Successful in 58s
Security Scan (SIAX Cloud) / security (pull_request) Successful in 20s
CI's integration job failed, and it failed CORRECTLY. That job provides a live OpenSearch and a live ClickHouse and then asserts that nothing in tests/integration/ skipped, on the stated principle that "a skipped suite must never masquerade as a green integration gate". The DuckDB suite needs a Python interpreter that job does not provide, so it skipped, and the guard fired — the first suite in that directory ever to skip there. Moving the file to tests/duckdb/ respects the guard rather than weakening it. Loosening it to tolerate skips would have traded a real gate for one green job, which is the trade this estate keeps regretting. The suite still runs, still fails rather than skipping when DUCKDB_PYTHON is set and DuckDB is broken, and is still pinned by the committed evidence. Also fixed: "does NOT ship the Iceberg table" asserted against the FILESYSTEM when the claim is about what is COMMITTED. The live suite rebuilds that table locally, so the assertion failed whenever both suites ran in one pass — it was testing the wrong thing rather than catching a real problem. It now asserts against `git ls-files`, and also asserts the parquet/csv fixtures ARE tracked, so a version that stopped shipping any fixture would fail too. docs/analytics/BI_QUERY_METRIC_QUALITY.md is organised around what is PROVEN and what is NOT, because the second list is the one that gets dropped. It records the five findings and the six open items, including that BI-P029's MetricFlow was never executed and that BI-P021's budget is a bound rather than a hard cap (measured 130x overshoot at a 100ms poll). It also reports, without changing it, a latent flaw in .gitea/workflows/ci.yml (a protected file this branch does not touch): the integration guard's second pattern `Tests [0-9]+ passed` never matches, because vitest writes that summary with ANSI colour codes between the words. The guard is therefore STRICTER than it reads — "any skip fails" rather than "skipped and nothing passed" — so nothing is currently unguarded, but the escape hatch does not work and someone will eventually be puzzled by it. 957 tests pass with ClickHouse, Postgres and DuckDB all live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,155 @@
|
||||
# BI-P014, P016–P022, P025–P026, P028–P031 — query, metric, quality
|
||||
|
||||
Extends the analytics core merged in `0pt#34` (BI-P002..P012). Nothing there is
|
||||
rebuilt.
|
||||
|
||||
This document is organised around what is **proven** and what is **not**,
|
||||
because the second list is the one that gets dropped.
|
||||
|
||||
---
|
||||
|
||||
## Where things live
|
||||
|
||||
| prompt | module | live proof |
|
||||
| -------------------------------------- | ----------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
|
||||
| BI-P014 DuckDB provider | `src/analytics/providers/duckdb-provider.ts` | `tests/duckdb/duckdb-provider.live.test.ts` |
|
||||
| BI-P016 query request contract | `src/analytics/contract/query-request.ts`, `compile-clickhouse.ts`, `compile-semantic.ts` | unit only (pure compilation) |
|
||||
| BI-P017 analyst SQL contract | `src/analytics/sql/` | `tests/integration/analyst-sql-clickhouse.integration.test.ts` |
|
||||
| BI-P018 execution model | `src/analytics/execution/query-execution-record.ts` | `sql/009` + Postgres realinfra |
|
||||
| BI-P019 cancellation | `src/analytics/execution/cancellation.ts` | `tests/integration/query-cancellation-budget.integration.test.ts` |
|
||||
| BI-P020 query cache | `src/analytics/cache/query-cache.ts` | unit only (no engine involved) |
|
||||
| BI-P021 cost / budget | `src/analytics/cost/query-budget.ts` | same live suite as BI-P019 |
|
||||
| BI-P022 dbt contract | `src/analytics/transform/dbt-contract.ts`, `analytics/dbt/` | contract only — execution is `n0d#38` |
|
||||
| BI-P025 quality provider | `src/analytics/quality/` | `tests/integration/data-quality-clickhouse.integration.test.ts` |
|
||||
| BI-P026 freshness | `src/analytics/quality/freshness.ts` | unit + `sql/009` defaults |
|
||||
| BI-P028/P031 metric model + governance | `src/analytics/metrics/metric-model.ts` | Postgres trigger, realinfra |
|
||||
| BI-P029 semantic provider | `src/analytics/metrics/semantic-provider.ts` | **partially — see below** |
|
||||
| BI-P030 semantic registry | `src/analytics/metrics/semantic-registry.ts` | unit + Postgres constraints |
|
||||
|
||||
---
|
||||
|
||||
## Five findings that changed the implementation
|
||||
|
||||
Each was found by writing a test that could fail, and each is pinned so it
|
||||
cannot quietly stop being true.
|
||||
|
||||
### 1. ClickHouse `readonly = 1` on a user profile forbids **every** request setting
|
||||
|
||||
Measured on 24.8.14.39. A user whose profile carries `readonly = 1` answers
|
||||
`Code: 164. Cannot modify 'max_rows_to_read' setting in readonly mode` to any
|
||||
setting sent with the statement. So BI-P012's ceilings **cannot travel with an
|
||||
analyst statement** — they are baked into the user's `SETTINGS PROFILE` at
|
||||
provisioning. Proven both ways: an over-budget scan fails with code 158 from
|
||||
the profile with no setting sent, and an attempt to raise it from the request
|
||||
fails with 164.
|
||||
|
||||
### 2. `CREATE ROW POLICY IF NOT EXISTS` is a silent no-op on re-provisioning
|
||||
|
||||
Dropping a user empties the policy's apply-to list. The next provisioning run
|
||||
sees the policy exists and does nothing, so it survives with
|
||||
`apply_to_list = []` — applying to **nobody** — while `system.row_policies`
|
||||
still shows the correct `select_filter`. The restricted user read 100 of 100
|
||||
rows, **both tenants**, from the base table.
|
||||
|
||||
Fixed with `OR REPLACE`, plus `rowPolicyVerificationSql()` which turns "the
|
||||
policy is applied" from an assumption into a query, and a test asserting
|
||||
`applied_to_user = 1` per object. `CREATE SETTINGS PROFILE IF NOT EXISTS` has
|
||||
the identical shape and is fixed the same way.
|
||||
|
||||
### 3. DuckDB's `allowed_directories` **alone** is a no-op
|
||||
|
||||
Measured on 1.5.5. With `allowed_directories` set to one mount directory and
|
||||
nothing else, `read_csv('/etc/passwd')` **returned the file** and an `https://`
|
||||
URL was fetched over the network. Only `enable_external_access = false`
|
||||
alongside it makes the allow-list bind.
|
||||
|
||||
The obvious spelling of this control appears in `duckdb_settings()`, looks like
|
||||
a sandbox, and permits everything. The measurement is pinned by a test that
|
||||
never skips, so simplifying the bootstrap breaks the build rather than the
|
||||
guarantee.
|
||||
|
||||
### 4. `pg_constraint.conname` is not schema-scoped
|
||||
|
||||
`SELECT 1 FROM pg_constraint WHERE conname = '...'` matches a same-named
|
||||
constraint in **any** schema, so `sql/009` applied into a second schema
|
||||
silently skipped every guarded `CHECK`. Every guard now scopes via
|
||||
`conrelid = '<table>'::regclass`.
|
||||
|
||||
### 5. `describe.skipIf(flagSetInBeforeAll)` skips **always**
|
||||
|
||||
Vitest evaluates `skipIf` at collection time. Ten tests reported green while
|
||||
skipping against a live, reachable ClickHouse. Replaced with the repo's
|
||||
env-var gate, which **fails** when the variable is set and the server is down.
|
||||
|
||||
---
|
||||
|
||||
## What is NOT proven
|
||||
|
||||
- **BI-P029 / MetricFlow was never executed.** No Python runtime is
|
||||
provisioned for this repo's CI, and BI-P029's "pin a currently verified
|
||||
version" needs a licence and supply-chain review that has not happened.
|
||||
`MetricFlowProvider.providerReadiness()` returns `verified: false` and names
|
||||
all four open items. What _is_ proven is that the engine-neutral spec
|
||||
compiles to a MetricFlow-shaped query and that canonical SIAX metric ids
|
||||
survive a provider swap — tested against a second, deliberately
|
||||
different-named provider so the equality is not trivially true.
|
||||
- **The Superset integration path and the AI Analyst comparison** (BI-P029's
|
||||
other two DoD surfaces) are not implemented.
|
||||
- **dbt is not run here.** `DBT_RUNTIME_PIN` has both review flags `false`, and
|
||||
`assertDbtRuntimeCertified` refuses a production run until they are true.
|
||||
- **DuckDB Arrow IPC** is unproven: the `arrow` extension is a network install
|
||||
the offline environment could not download. `readiness()` says so.
|
||||
- **BI-P021's budget is a bound, not a hard cap.** In-flight enforcement is
|
||||
granular to the poll interval. Measured: a 1,000,000-row window cancelled the
|
||||
query at **130,883,409 rows**, 92 ms in — a 130× overshoot. Still a bound
|
||||
(unwatched, the same query read 6.5 billion rows in one second), but anyone
|
||||
quoting it as a hard cap is wrong.
|
||||
- **The per-process window store does not bind across replicas.**
|
||||
`describeEnforcement()` reports `"with N replicas a tenant can spend up to N
|
||||
times this budget"`. A shared store (Valkey, or the CL0UD quota service)
|
||||
behind `SharedWindowStore` is required for it to bind.
|
||||
|
||||
---
|
||||
|
||||
## Running the live suites
|
||||
|
||||
```bash
|
||||
docker run -d --name ch -p 28123:8123 -e CLICKHOUSE_SKIP_USER_SETUP=1 \
|
||||
clickhouse/clickhouse-server:24.8
|
||||
docker run -d --name pg -p 25433:5432 -e POSTGRES_PASSWORD=test \
|
||||
-e POSTGRES_DB=opt postgres:16-alpine
|
||||
|
||||
CLICKHOUSE_URL=http://127.0.0.1:28123 pnpm vitest run tests/integration
|
||||
ANALYTICS_TEST_DATABASE_URL=postgres://postgres:test@127.0.0.1:25433/opt \
|
||||
pnpm vitest run tests/analytics/analytics-governance-postgres.realinfra.test.ts
|
||||
|
||||
python -m venv .venv && .venv/bin/pip install duckdb pyarrow 'pyiceberg[sql-sqlite]'
|
||||
DUCKDB_PYTHON=.venv/bin/python pnpm vitest run tests/duckdb
|
||||
```
|
||||
|
||||
Every one of these **fails** rather than skipping when its variable is set and
|
||||
the backend is unreachable.
|
||||
|
||||
---
|
||||
|
||||
## One note for whoever maintains `.gitea/workflows/ci.yml`
|
||||
|
||||
Not changed here — it is a protected file, and this branch touched none.
|
||||
|
||||
The integration job's guard is:
|
||||
|
||||
```sh
|
||||
if grep -qE "skipped" /tmp/itest.log && ! grep -qE "Tests [0-9]+ passed" /tmp/itest.log
|
||||
```
|
||||
|
||||
Vitest writes that summary with ANSI colour codes, so the literal text is
|
||||
`Tests [22m [1m[32m81 passed[39m…`. The second pattern therefore **never
|
||||
matches**, and the guard is effectively "any skip fails the job" rather than
|
||||
the "skipped _and_ nothing passed" it reads as. That is stricter than intended,
|
||||
not looser, so nothing is currently unguarded — but the escape hatch does not
|
||||
work, and someone will eventually be puzzled by it. Piping through
|
||||
`sed 's/\x1b\[[0-9;]*m//g'` before the grep would restore the intended
|
||||
behaviour.
|
||||
|
||||
This is how the DuckDB suite's placement was caught, so the guard earned its
|
||||
keep either way.
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"duckdb_version": "1.5.5",
|
||||
"recorded_at": "2026-09-07T00:53:52.439Z",
|
||||
"recorded_at": "2026-09-07T01:07:03.476Z",
|
||||
"parquet": {
|
||||
"SE": 394.5,
|
||||
"NO": 105.5
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
"prompt": "BI-P017",
|
||||
"claim": "Every bypass attempt, executed directly against a live ClickHouse as the tenant's restricted user WITH THE STATEMENT GUARD REMOVED, fails to return a foreign-tenant row, an ungranted-table row, or server metadata.",
|
||||
"clickhouse_version": "24.8.14.39",
|
||||
"recorded_at": "2026-09-07T00:19:07.198Z",
|
||||
"recorded_at": "2026-09-07T01:07:07.196Z",
|
||||
"base_table_rows_both_tenants": 100,
|
||||
"tenant_view_rows_alpha": 50,
|
||||
"attempts_total": 64,
|
||||
@@ -572,7 +572,7 @@
|
||||
"leaked_ungranted_table_row": false,
|
||||
"leaked_server_metadata": false,
|
||||
"guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: \"SYSTEM\" is not permitted in a read-only analyst session",
|
||||
"engine_response_head": "ALPHA_ROW_0 ALPHA_ROW_1 ALPHA_ROW_2 ALPHA_ROW_3 ALPHA_ROW_4 ALPHA_ROW_5 ALPHA_ROW_6 ALPHA_ROW_7 ALPHA_ROW_8 ALPHA_ROW_9 ALPHA_ROW_10 ALPHA_ROW_11 ALPHA_ROW_12 A"
|
||||
"engine_response_head": "events sales events sales ALPHA_ROW_0 ALPHA_ROW_1 ALPHA_ROW_2 ALPHA_ROW_3 ALPHA_ROW_4 ALPHA_ROW_5 ALPHA_ROW_6 ALPHA_ROW_7 ALPHA_ROW_8 ALPHA_ROW_9 ALPHA_ROW_10 A"
|
||||
},
|
||||
{
|
||||
"id": "unterminated-comment",
|
||||
@@ -656,7 +656,7 @@
|
||||
"leaked_ungranted_table_row": false,
|
||||
"leaked_server_metadata": false,
|
||||
"guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: \"SYSTEM\" is not permitted in a read-only analyst session",
|
||||
"engine_response_head": "opt_analytics events 12706469-5605-436f-94ff-fc3c112aec6f MergeTree 0 ['/var/lib/clickhouse/store/127/12706469-5605-436f-94ff-fc3c112aec6f/'] /var/lib/clickhous"
|
||||
"engine_response_head": "opt_analytics events 30087fa6-7696-4216-abc7-507d8ab01e10 MergeTree 0 ['/var/lib/clickhouse/store/300/30087fa6-7696-4216-abc7-507d8ab01e10/'] /var/lib/clickhous"
|
||||
},
|
||||
{
|
||||
"id": "describe-subquery",
|
||||
|
||||
@@ -1,20 +1,20 @@
|
||||
{
|
||||
"clickhouse_version": "24.8.14.39",
|
||||
"recorded_at": "2026-09-07T00:25:59.706Z",
|
||||
"recorded_at": "2026-09-07T01:07:02.373Z",
|
||||
"counterfactual_still_running_after_2s": true,
|
||||
"cancellation": {
|
||||
"rows_read_before_cancel": 9124163046,
|
||||
"elapsed_ms_to_verified_stop": 35,
|
||||
"rows_read_before_cancel": 4955255022,
|
||||
"elapsed_ms_to_verified_stop": 110,
|
||||
"verified_stopped": true,
|
||||
"clickhouse_query_log_type": "ExceptionWhileProcessing",
|
||||
"clickhouse_exception_code": 394
|
||||
},
|
||||
"budget_in_flight": {
|
||||
"budget_max_rows_scanned": 1000000,
|
||||
"observed_rows_read_at_cancel": 127743777,
|
||||
"observed_elapsed_ms_at_cancel": 12,
|
||||
"observed_rows_read_at_cancel": 215980518,
|
||||
"observed_elapsed_ms_at_cancel": 60,
|
||||
"polls_before_breach": 1,
|
||||
"overshoot_factor": 128,
|
||||
"overshoot_factor": 216,
|
||||
"overshoot_note": "in-flight enforcement is granular to the poll interval; the query overshot the window by this factor before the poll that caught it. A bound, not a hard cap.",
|
||||
"cancellation_verified": true,
|
||||
"clickhouse_exception_code": 394,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"clickhouse_version": "24.8.14.39",
|
||||
"recorded_at": "2026-09-07T00:40:02.745Z",
|
||||
"recorded_at": "2026-09-07T01:07:17.084Z",
|
||||
"not_null": {
|
||||
"evaluated": 118,
|
||||
"violations": 5
|
||||
|
||||
@@ -39,7 +39,7 @@
|
||||
* refuses a session missing any of them.
|
||||
*
|
||||
* Evidence: evidence/bi-p014/duckdb-sandbox.json, written by
|
||||
* tests/integration/duckdb-provider.integration.test.ts, which runs the bypass
|
||||
* tests/duckdb/duckdb-provider.live.test.ts, which runs the bypass
|
||||
* corpus with LAYER 1 REMOVED and requires layer 2 to refuse it anyway.
|
||||
*/
|
||||
import path from 'node:path';
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* BI-P014 — the parts provable without a DuckDB, plus the pin on the parts
|
||||
* that are not.
|
||||
*
|
||||
* The live suite (tests/integration/duckdb-provider.integration.test.ts) only
|
||||
* The live suite (tests/duckdb/duckdb-provider.live.test.ts) only
|
||||
* runs when DUCKDB_PYTHON is set. On a runner without it, it contributes
|
||||
* nothing — so the recorded evidence is committed and THIS file, which never
|
||||
* skips, checks it. In particular it pins the finding that makes layer 2 real:
|
||||
|
||||
+14
-2
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* tests/integration/duckdb-provider.integration.test.ts
|
||||
* ======================================================
|
||||
* tests/duckdb/duckdb-provider.live.test.ts
|
||||
* ==========================================
|
||||
* BI-P014 — the DuckDB provider against a real DuckDB.
|
||||
*
|
||||
* Two things are proven here that a unit test cannot:
|
||||
@@ -22,6 +22,18 @@
|
||||
*
|
||||
* Gated on DUCKDB_PYTHON, an interpreter with the `duckdb` package. With it
|
||||
* set and DuckDB broken, this suite FAILS; it does not degrade to a skip.
|
||||
*
|
||||
* ── WHY THIS IS NOT UNDER tests/integration/ ──────────────────────────────
|
||||
* It was, and CI's integration job failed — correctly. That job provides a
|
||||
* live OpenSearch and a live ClickHouse and then asserts that NOTHING in
|
||||
* tests/integration/ skipped, on the stated principle that "a skipped suite
|
||||
* must never masquerade as a green integration gate". A DuckDB suite needing
|
||||
* a Python interpreter that job does not provide is not one of the suites that
|
||||
* job is a gate for, so it skipped, and the guard fired.
|
||||
*
|
||||
* Moving the file respects that guard rather than weakening it. Loosening the
|
||||
* guard to tolerate skips would have traded a real gate for one green job —
|
||||
* which is the trade this estate keeps regretting.
|
||||
*/
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import fs from 'node:fs';
|
||||
Vendored
+1
-1
@@ -14,7 +14,7 @@ To rebuild the Iceberg fixture locally:
|
||||
|
||||
```
|
||||
python -m venv .venv && .venv/bin/pip install duckdb pyarrow 'pyiceberg[sql-sqlite]'
|
||||
DUCKDB_PYTHON=.venv/bin/python pnpm vitest run tests/integration/duckdb-provider.integration.test.ts
|
||||
DUCKDB_PYTHON=.venv/bin/python pnpm vitest run tests/duckdb/duckdb-provider.live.test.ts
|
||||
```
|
||||
|
||||
`iceberg_scan` is pointed at an EXACT metadata JSON file rather than a table
|
||||
|
||||
Reference in New Issue
Block a user