From 03a9b37fc3c71d469f01a59477a53f5ddc34684b Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 23 Apr 2026 11:27:36 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20BigQuery=20finops=20SQL=20=E2=80=94=20co?= =?UTF-8?q?rrect=20INFORMATION=5FSCHEMA=20columns=20+=20multi-region=20sup?= =?UTF-8?q?port?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `finops_query_history` was failing 100% on BigQuery with `Unrecognized name: error_message at [11:5]`. Telemetry traced it to one session looping 76 times over 2.5 hours. Three separate bugs in the BQ query-history template plus a broader region-locking issue across the whole finops module. Fixes in `packages/opencode/src/altimate/native/finops/query-history.ts` BIGQUERY_HISTORY_SQL: - `error_message` does not exist as a top-level column in INFORMATION_SCHEMA.JOBS. Replace with `error_result.message AS error_message`. - `error_result.reason AS error_code` in place of the previous hardcoded NULL (more useful and matches the struct that actually exists). - `total_rows` is a PARTITIONS column, not JOBS. Replace with `CAST(NULL AS INT64) AS rows_produced` so the downstream summary loop doesn't error. - BQ's `state` returns `'DONE'`, not `'SUCCESS'`. `getQueryHistory()` counts any `execution_status != 'SUCCESS'` as a failure, so bare `state AS execution_status` was flagging every completed BQ job as failed. Derive `CASE WHEN error_result IS NULL THEN 'SUCCESS' ELSE 'FAILED' END` to match the other warehouse templates. Multi-region support across all 5 finops modules: - All BQ `INFORMATION_SCHEMA` queries were hardcoded to `` `region-US.INFORMATION_SCHEMA.*` ``, making the tools unusable for non-US BigQuery projects. Replace with `{region}` placeholder and interpolate from the connection's configured `location` at runtime. - New shared helper `finops/bq-utils.ts` exposes `sanitizeBqRegion` (allowlist `[a-z0-9-]`, trim hyphens, cap at 64 chars, fall back to `us`), `interpolateBqRegion` (uses `replaceAll` so future JOIN-across-views templates stay safe), and `bqRegionFor(warehouse)`. - Covers query-history, credit-analyzer (3 templates), warehouse-advisor (2 templates), role-access, and unused-resources. Snowflake and Databricks paths are untouched. The new `bqRegion?` parameter is optional on every build helper and only consumed inside `whType === "bigquery"` branches; regression tests assert Snowflake and Databricks SQL still contains `QUERY_HISTORY` / `system.query.history` and never `region-`. Tests: - New `schema-finops-dbt.test.ts` assertions: column-level regression guards (no bare `error_message`/`total_rows`/`state as execution_status`), a table-driven test that no finops BQ template contains `region-US`, region-threading tests for all 5 build helpers, sanitizer coverage (injection, hyphen-trim, length cap, non-string), Snowflake/Databricks regression guards. 75 pass (up from 66). - New `finops-bigquery-e2e.test.ts`: skipIf-gated against `ALTIMATE_CODE_CONN_BIGQUERY_TEST`, mirrors the Snowflake pattern. Verifies shape, no SQL parse errors, derived `execution_status`, error_count math, and a bad-region graceful-failure path. 10/10 pass against real BigQuery (US region). - Full altimate suite: 2917/2917 pass (was 2908). Typecheck clean. Reviewed via /consensus:code-review by GPT-5.4, Gemini 3.1 Pro, and DeepSeek V3.2 — all three APPROVE on round 1 after incorporating their MAJOR findings (missing tests, missed files, sanitizer hardening). Closes #738 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/altimate/native/finops/bq-utils.ts | 57 +++++ .../altimate/native/finops/credit-analyzer.ts | 36 ++- .../altimate/native/finops/query-history.ts | 26 ++- .../src/altimate/native/finops/role-access.ts | 10 +- .../native/finops/unused-resources.ts | 6 +- .../native/finops/warehouse-advisor.ts | 22 +- .../test/altimate/finops-bigquery-e2e.test.ts | 221 ++++++++++++++++++ .../test/altimate/schema-finops-dbt.test.ts | 151 ++++++++++++ 8 files changed, 496 insertions(+), 33 deletions(-) create mode 100644 packages/opencode/src/altimate/native/finops/bq-utils.ts create mode 100644 packages/opencode/test/altimate/finops-bigquery-e2e.test.ts diff --git a/packages/opencode/src/altimate/native/finops/bq-utils.ts b/packages/opencode/src/altimate/native/finops/bq-utils.ts new file mode 100644 index 0000000000..a61ab1afb9 --- /dev/null +++ b/packages/opencode/src/altimate/native/finops/bq-utils.ts @@ -0,0 +1,57 @@ +/** + * Shared BigQuery helpers for the finops module. + * + * INFORMATION_SCHEMA queries on BigQuery must be region-qualified — + * `` `region-.INFORMATION_SCHEMA.` ``. The finops tools read + * the connection's configured `location` (e.g. "us", "eu", "us-central1") and + * interpolate it into each SQL template via a `{region}` placeholder. + * + * Kept in its own file so the sanitize + interpolate logic doesn't drift across + * query-history, credit-analyzer, warehouse-advisor, role-access, and + * unused-resources. + */ + +import * as Registry from "../connections/registry" + +/** + * Sanitize a BigQuery region/location string for safe interpolation into a + * region-qualified INFORMATION_SCHEMA reference. The result is always safe to + * inject into `` `region-.INFORMATION_SCHEMA.X` `` — the allow-list + * `[a-z0-9-]` cannot close the backtick context or introduce SQL delimiters. + * + * Transformations: + * - lowercase + trim + * - strip anything outside [a-z0-9-] + * - trim leading/trailing hyphens (BQ region names never start or end with -) + * - cap length at 64 chars (BQ region names are short; this guards against + * pathological inputs) + * - fall back to "us" on empty input (historical default) + */ +export function sanitizeBqRegion(location: unknown): string { + const raw = typeof location === "string" ? location : "" + const cleaned = raw + .toLowerCase() + .trim() + .replace(/[^a-z0-9-]/g, "") + .replace(/^-+|-+$/g, "") + .slice(0, 64) + return cleaned || "us" +} + +/** + * Substitute the `{region}` placeholder in a BQ SQL template with the sanitized + * region for a given warehouse config. Uses replaceAll so future templates that + * reference multiple region-qualified views (e.g. JOINs) are handled safely. + */ +export function interpolateBqRegion(sqlTemplate: string, bqRegion?: unknown): string { + return sqlTemplate.replaceAll("{region}", sanitizeBqRegion(bqRegion)) +} + +/** + * Resolve the BigQuery `location` for a registered warehouse. Returns undefined + * when the warehouse is not BigQuery or has no location set — callers pass the + * result through `sanitizeBqRegion`, which defaults to "us". + */ +export function bqRegionFor(warehouse: string): unknown { + return Registry.getConfig(warehouse)?.location +} diff --git a/packages/opencode/src/altimate/native/finops/credit-analyzer.ts b/packages/opencode/src/altimate/native/finops/credit-analyzer.ts index f8d22cc27b..368ee95dd5 100644 --- a/packages/opencode/src/altimate/native/finops/credit-analyzer.ts +++ b/packages/opencode/src/altimate/native/finops/credit-analyzer.ts @@ -5,6 +5,7 @@ */ import * as Registry from "../connections/registry" +import { bqRegionFor, interpolateBqRegion } from "./bq-utils" import type { CreditAnalysisParams, CreditAnalysisResult, @@ -80,7 +81,7 @@ SELECT 0 as credits_cloud, COUNT(*) as query_count, AVG(total_bytes_billed) / 1099511627776.0 * 5.0 as avg_credits_per_query -FROM \`region-US.INFORMATION_SCHEMA.JOBS\` +FROM \`region-{region}.INFORMATION_SCHEMA.JOBS\` WHERE creation_time >= TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL ? DAY) AND job_type = 'QUERY' AND state = 'DONE' @@ -97,7 +98,7 @@ SELECT 0 as total_cloud_credits, COUNT(DISTINCT DATE(creation_time)) as active_days, AVG(total_bytes_billed) / 1099511627776.0 * 5.0 as avg_daily_credits -FROM \`region-US.INFORMATION_SCHEMA.JOBS\` +FROM \`region-{region}.INFORMATION_SCHEMA.JOBS\` WHERE creation_time >= TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL ? DAY) AND job_type = 'QUERY' AND state = 'DONE' @@ -115,7 +116,7 @@ SELECT 0 as rows_produced, total_bytes_billed / 1099511627776.0 * 5.0 as credits_used, start_time -FROM \`region-US.INFORMATION_SCHEMA.JOBS\` +FROM \`region-{region}.INFORMATION_SCHEMA.JOBS\` WHERE creation_time >= TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL ? DAY) AND job_type = 'QUERY' AND state = 'DONE' @@ -191,7 +192,7 @@ function getWhType(warehouse: string): string { } function buildCreditUsageSql( - whType: string, days: number, limit: number, warehouseFilter?: string, + whType: string, days: number, limit: number, warehouseFilter?: string, bqRegion?: unknown, ): { sql: string; binds: any[] } | null { if (whType === "snowflake") { const binds: any[] = [-days] @@ -203,7 +204,10 @@ function buildCreditUsageSql( } } if (whType === "bigquery") { - return { sql: BIGQUERY_CREDIT_USAGE_SQL, binds: [days, limit] } + return { + sql: interpolateBqRegion(BIGQUERY_CREDIT_USAGE_SQL, bqRegion), + binds: [days, limit], + } } if (whType === "databricks") { return { sql: DATABRICKS_CREDIT_USAGE_SQL, binds: [days, limit] } @@ -211,12 +215,15 @@ function buildCreditUsageSql( return null } -function buildCreditSummarySql(whType: string, days: number): { sql: string; binds: any[] } | null { +function buildCreditSummarySql(whType: string, days: number, bqRegion?: unknown): { sql: string; binds: any[] } | null { if (whType === "snowflake") { return { sql: SNOWFLAKE_CREDIT_SUMMARY_SQL, binds: [-days] } } if (whType === "bigquery") { - return { sql: BIGQUERY_CREDIT_SUMMARY_SQL, binds: [days] } + return { + sql: interpolateBqRegion(BIGQUERY_CREDIT_SUMMARY_SQL, bqRegion), + binds: [days], + } } if (whType === "databricks") { return { sql: DATABRICKS_CREDIT_SUMMARY_SQL, binds: [days] } @@ -224,12 +231,15 @@ function buildCreditSummarySql(whType: string, days: number): { sql: string; bin return null } -function buildExpensiveSql(whType: string, days: number, limit: number): { sql: string; binds: any[] } | null { +function buildExpensiveSql(whType: string, days: number, limit: number, bqRegion?: unknown): { sql: string; binds: any[] } | null { if (whType === "snowflake") { return { sql: SNOWFLAKE_EXPENSIVE_SQL, binds: [-days, limit] } } if (whType === "bigquery") { - return { sql: BIGQUERY_EXPENSIVE_SQL, binds: [days, limit] } + return { + sql: interpolateBqRegion(BIGQUERY_EXPENSIVE_SQL, bqRegion), + binds: [days, limit], + } } if (whType === "databricks") { return { sql: DATABRICKS_EXPENSIVE_SQL, binds: [days, limit] } @@ -295,9 +305,10 @@ export async function analyzeCredits(params: CreditAnalysisParams): Promise= TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL ? DAY) ORDER BY creation_time DESC LIMIT ? @@ -146,6 +153,7 @@ function buildHistoryQuery( limit: number, user?: string, warehouseFilter?: string, + bqRegion?: unknown, ): { sql: string; binds: any[] } | null { if (whType === "snowflake") { const binds: any[] = [-days] @@ -161,7 +169,10 @@ function buildHistoryQuery( return { sql: POSTGRES_HISTORY_SQL.replace("{limit}", String(Math.floor(Number(limit)))), binds: [] } } if (whType === "bigquery") { - return { sql: BIGQUERY_HISTORY_SQL, binds: [days, limit] } + return { + sql: interpolateBqRegion(BIGQUERY_HISTORY_SQL, bqRegion), + binds: [days, limit], + } } if (whType === "databricks") { return { sql: DATABRICKS_HISTORY_SQL, binds: [days, limit] } @@ -202,8 +213,9 @@ export async function getQueryHistory(params: QueryHistoryParams): Promise { const whType = getWhType(params.warehouse) const limit = params.limit ?? 100 + const bqRegion = whType === "bigquery" ? bqRegionFor(params.warehouse) : undefined - const built = buildGrantsSql(whType, params.role, params.object_name, limit) + const built = buildGrantsSql(whType, params.role, params.object_name, limit, bqRegion) if (!built) { return { success: false, diff --git a/packages/opencode/src/altimate/native/finops/unused-resources.ts b/packages/opencode/src/altimate/native/finops/unused-resources.ts index 8c44f90e96..a6de251df9 100644 --- a/packages/opencode/src/altimate/native/finops/unused-resources.ts +++ b/packages/opencode/src/altimate/native/finops/unused-resources.ts @@ -5,6 +5,7 @@ */ import * as Registry from "../connections/registry" +import { bqRegionFor, interpolateBqRegion } from "./bq-utils" import type { UnusedResourcesParams, UnusedResourcesResult, @@ -90,7 +91,7 @@ SELECT size_bytes, TIMESTAMP_MILLIS(last_modified_time) as last_altered, creation_time as created -FROM \`region-US.INFORMATION_SCHEMA.TABLE_STORAGE\` +FROM \`region-{region}.INFORMATION_SCHEMA.TABLE_STORAGE\` WHERE NOT deleted AND last_modified_time < UNIX_MILLIS(TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL ? DAY)) ORDER BY size_bytes DESC @@ -186,7 +187,8 @@ export async function findUnusedResources(params: UnusedResourcesParams): Promis } } else if (whType === "bigquery") { try { - const result = await connector.execute(BIGQUERY_UNUSED_TABLES_SQL, limit, [days, limit]) + const sql = interpolateBqRegion(BIGQUERY_UNUSED_TABLES_SQL, bqRegionFor(params.warehouse)) + const result = await connector.execute(sql, limit, [days, limit]) unusedTables = rowsToRecords(result) } catch (e) { errors.push(`Could not query unused tables: ${e}`) diff --git a/packages/opencode/src/altimate/native/finops/warehouse-advisor.ts b/packages/opencode/src/altimate/native/finops/warehouse-advisor.ts index dc6c1166a5..5352f48f75 100644 --- a/packages/opencode/src/altimate/native/finops/warehouse-advisor.ts +++ b/packages/opencode/src/altimate/native/finops/warehouse-advisor.ts @@ -5,6 +5,7 @@ */ import * as Registry from "../connections/registry" +import { bqRegionFor, interpolateBqRegion } from "./bq-utils" import type { WarehouseAdvisorParams, WarehouseAdvisorResult, @@ -58,7 +59,7 @@ SELECT 0 as avg_queue_load, MAX(period_slot_ms / 1000.0) as peak_queue_load, COUNT(*) as sample_count -FROM \`region-US.INFORMATION_SCHEMA.JOBS_TIMELINE\` +FROM \`region-{region}.INFORMATION_SCHEMA.JOBS_TIMELINE\` WHERE period_start >= TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL {days} DAY) GROUP BY reservation_id ORDER BY avg_concurrency DESC @@ -73,7 +74,7 @@ SELECT APPROX_QUANTILES(TIMESTAMP_DIFF(end_time, start_time, MILLISECOND), 100)[OFFSET(95)] / 1000.0 as p95_time_sec, AVG(total_bytes_billed) as avg_bytes_scanned, SUM(total_bytes_billed) / 1099511627776.0 * 5.0 as total_credits -FROM \`region-US.INFORMATION_SCHEMA.JOBS\` +FROM \`region-{region}.INFORMATION_SCHEMA.JOBS\` WHERE creation_time >= TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL {days} DAY) AND job_type = 'QUERY' AND state = 'DONE' @@ -127,16 +128,20 @@ function getWhType(warehouse: string): string { return wh?.type || "unknown" } -function buildLoadSql(whType: string, days: number): string | null { +function buildLoadSql(whType: string, days: number, bqRegion?: unknown): string | null { if (whType === "snowflake") return SNOWFLAKE_LOAD_SQL.replace("{days}", String(days)) - if (whType === "bigquery") return BIGQUERY_LOAD_SQL.replace("{days}", String(days)) + if (whType === "bigquery") { + return interpolateBqRegion(BIGQUERY_LOAD_SQL.replace("{days}", String(days)), bqRegion) + } if (whType === "databricks") return DATABRICKS_LOAD_SQL.replace(/{days}/g, String(days)) return null } -function buildSizingSql(whType: string, days: number): string | null { +function buildSizingSql(whType: string, days: number, bqRegion?: unknown): string | null { if (whType === "snowflake") return SNOWFLAKE_SIZING_SQL.replace("{days}", String(days)) - if (whType === "bigquery") return BIGQUERY_SIZING_SQL.replace("{days}", String(days)) + if (whType === "bigquery") { + return interpolateBqRegion(BIGQUERY_SIZING_SQL.replace("{days}", String(days)), bqRegion) + } if (whType === "databricks") return DATABRICKS_SIZING_SQL.replace(/{days}/g, String(days)) return null } @@ -217,9 +222,10 @@ function generateSizingRecommendations( export async function adviseWarehouse(params: WarehouseAdvisorParams): Promise { const whType = getWhType(params.warehouse) const days = params.days ?? 14 + const bqRegion = whType === "bigquery" ? bqRegionFor(params.warehouse) : undefined - const loadSql = buildLoadSql(whType, days) - const sizingSql = buildSizingSql(whType, days) + const loadSql = buildLoadSql(whType, days, bqRegion) + const sizingSql = buildSizingSql(whType, days, bqRegion) if (!loadSql || !sizingSql) { return { diff --git a/packages/opencode/test/altimate/finops-bigquery-e2e.test.ts b/packages/opencode/test/altimate/finops-bigquery-e2e.test.ts new file mode 100644 index 0000000000..3ba766b9ed --- /dev/null +++ b/packages/opencode/test/altimate/finops-bigquery-e2e.test.ts @@ -0,0 +1,221 @@ +/** + * FinOps Integration Tests — BigQuery + * + * Tests query execution and response shape for all finops functions against + * real BigQuery INFORMATION_SCHEMA views. Skips if ALTIMATE_CODE_CONN_BIGQUERY_TEST + * is not set. Requires the service account to have + * `bigquery.jobs.listAll` (roles/bigquery.resourceAdmin or + * roles/bigquery.jobUser + INFORMATION_SCHEMA view) in the target region. + * + * Tests accept success OR a graceful permission error — the goal is to verify + * the SQL is valid (no "Unrecognized name" errors), binds are correct, and the + * response has the right shape. This catches regressions like the + * `error_message` / `total_rows` column typos that shipped in v0.5.21. + * + * Run: + * export ALTIMATE_CODE_CONN_BIGQUERY_TEST='{"type":"bigquery","project":"your-project","credentials_path":"/absolute/path/to/key.json","location":"us"}' + * bun test test/altimate/finops-bigquery-e2e.test.ts --timeout 120000 + */ + +import { describe, expect, test, beforeAll, afterAll } from "bun:test" + +process.env.ALTIMATE_TELEMETRY_DISABLED = "true" + +import * as Registry from "../../src/altimate/native/connections/registry" +import { getQueryHistory } from "../../src/altimate/native/finops/query-history" +import { analyzeCredits, getExpensiveQueries } from "../../src/altimate/native/finops/credit-analyzer" +import { adviseWarehouse } from "../../src/altimate/native/finops/warehouse-advisor" + +const BQ_CONFIG = process.env.ALTIMATE_CODE_CONN_BIGQUERY_TEST +const HAS_BIGQUERY = !!BQ_CONFIG +const WH = "bigquery_finops_e2e" + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Accept success OR a graceful error (permission / region mismatch / no jobs + * in window). The key invariant: no SQL parse errors — those indicate a + * template regression and must fail the test. + */ +function expectValidResult(result: { success: boolean; error?: string }) { + if (!result.success) { + expect(result.error).toBeDefined() + expect(typeof result.error).toBe("string") + // SQL parse errors from BQ should never occur — they mean the template is broken + expect(result.error).not.toMatch(/Unrecognized name|Syntax error|unbound|invalid identifier/i) + } +} + +// --------------------------------------------------------------------------- +// Suite +// --------------------------------------------------------------------------- + +describe.skipIf(!HAS_BIGQUERY)("BigQuery FinOps Integration", () => { + beforeAll(async () => { + Registry.reset() + Registry.setConfigs({ [WH]: JSON.parse(BQ_CONFIG!) }) + }, 30000) + + afterAll(() => { + Registry.reset() + }) + + // ------------------------------------------------------------------------- + // Query History — the main regression surface + // ------------------------------------------------------------------------- + describe("query_history", () => { + let result: Awaited> + + beforeAll(async () => { + result = await getQueryHistory({ warehouse: WH, days: 7, limit: 10 }) + }, 60000) + + test("returns valid shape (no SQL parse errors)", () => { + expectValidResult(result) + if (result.success) { + expect(Array.isArray(result.queries)).toBe(true) + expect(typeof result.summary).toBe("object") + expect(typeof result.summary.query_count).toBe("number") + expect(result.warehouse_type).toBe("bigquery") + } + }) + + test("respects limit parameter", () => { + if (result.success) { + expect(result.queries.length).toBeLessThanOrEqual(10) + } + }) + + test("query rows have expected columns when data exists", () => { + if (result.success && result.queries.length > 0) { + const row = result.queries[0] + expect(row).toHaveProperty("query_id") + expect(row).toHaveProperty("query_text") + expect(row).toHaveProperty("execution_status") + expect(row).toHaveProperty("execution_time_sec") + // The regression columns — they must exist and be accessible even if NULL + expect(row).toHaveProperty("error_message") + expect(row).toHaveProperty("error_code") + expect(row).toHaveProperty("rows_produced") + } + }) + + test("execution_status is derived correctly (never contains 'DONE')", () => { + // BQ's raw `state` column returns 'DONE' for every finished job. Our template + // maps error_result IS NULL → 'SUCCESS' / 'FAILED' so the summary error + // count in getQueryHistory() works. A row reading 'DONE' means the mapping + // regressed back to a bare `state as execution_status`. + if (result.success && result.queries.length > 0) { + for (const row of result.queries) { + expect(row.execution_status).not.toBe("DONE") + expect(["SUCCESS", "FAILED"]).toContain(String(row.execution_status)) + } + } + }) + + test("summary error_count uses the derived status (SUCCESS vs FAILED)", () => { + if (result.success) { + // error_count is computed from rows where execution_status !== 'SUCCESS'; + // if the mapping is wrong, every row would read as FAILED and the count + // would equal queries.length. + const errCount = Number(result.summary.error_count ?? 0) + expect(errCount).toBeLessThanOrEqual(result.queries.length) + } + }) + }) + + // ------------------------------------------------------------------------- + // Credit / Cost Analysis — BQ templates share the same region qualifier bug + // surface as query history. + // ------------------------------------------------------------------------- + describe("credit_analyzer", () => { + let creditsResult: Awaited> + let expensiveResult: Awaited> + + beforeAll(async () => { + ;[creditsResult, expensiveResult] = await Promise.all([ + analyzeCredits({ warehouse: WH, days: 14, limit: 10 }), + getExpensiveQueries({ warehouse: WH, days: 14, limit: 10 }), + ]) + }, 60000) + + test("analyzeCredits returns valid shape (no SQL parse errors)", () => { + expectValidResult(creditsResult) + if (creditsResult.success) { + expect(Array.isArray(creditsResult.daily_usage)).toBe(true) + expect(Array.isArray(creditsResult.warehouse_summary)).toBe(true) + expect(typeof creditsResult.total_credits).toBe("number") + expect(creditsResult.days_analyzed).toBe(14) + expect(Array.isArray(creditsResult.recommendations)).toBe(true) + } + }) + + test("getExpensiveQueries returns valid shape (no SQL parse errors)", () => { + expectValidResult(expensiveResult) + if (expensiveResult.success) { + expect(Array.isArray(expensiveResult.queries)).toBe(true) + expect(typeof expensiveResult.query_count).toBe("number") + expect(expensiveResult.days_analyzed).toBe(14) + } + }) + + test("expensive query rows have expected columns when data exists", () => { + if (expensiveResult.success && expensiveResult.queries.length > 0) { + const row = expensiveResult.queries[0] + expect(row).toHaveProperty("query_id") + expect(row).toHaveProperty("query_preview") + expect(row).toHaveProperty("execution_time_sec") + expect(row).toHaveProperty("bytes_scanned") + } + }) + }) + + // ------------------------------------------------------------------------- + // Warehouse Advisor — BQ uses JOBS_TIMELINE + JOBS, both region-qualified. + // ------------------------------------------------------------------------- + describe("warehouse_advisor", () => { + let result: Awaited> + + beforeAll(async () => { + result = await adviseWarehouse({ warehouse: WH, days: 14 }) + }, 60000) + + test("returns valid shape (no SQL parse errors)", () => { + expectValidResult(result) + if (result.success) { + expect(Array.isArray(result.warehouse_load)).toBe(true) + expect(Array.isArray(result.warehouse_performance)).toBe(true) + expect(Array.isArray(result.recommendations)).toBe(true) + expect(result.days_analyzed).toBe(14) + } + }) + }) + + // ------------------------------------------------------------------------- + // Region sanity — tolerate a bogus region by surfacing a graceful error + // rather than crashing. Supports the multi-region story. + // ------------------------------------------------------------------------- + describe("region handling", () => { + const BAD_REGION_WH = "bigquery_bad_region" + + beforeAll(() => { + const baseConfig = JSON.parse(BQ_CONFIG!) + Registry.setConfigs({ + [WH]: baseConfig, + [BAD_REGION_WH]: { ...baseConfig, location: "mars-central99" }, + }) + }) + + test("invalid region returns graceful error, not an unhandled crash", async () => { + const r = await getQueryHistory({ warehouse: BAD_REGION_WH, days: 1, limit: 1 }) + // Either the dataset-not-found error (graceful) or success (if BQ is + // tolerant) — both are fine. The key is: no unhandled throw. + if (!r.success) { + expect(r.error).toBeDefined() + expect(typeof r.error).toBe("string") + } + }, 60000) + }) +}) diff --git a/packages/opencode/test/altimate/schema-finops-dbt.test.ts b/packages/opencode/test/altimate/schema-finops-dbt.test.ts index cad71b41e8..3c915a265e 100644 --- a/packages/opencode/test/altimate/schema-finops-dbt.test.ts +++ b/packages/opencode/test/altimate/schema-finops-dbt.test.ts @@ -28,6 +28,7 @@ import { SQL_TEMPLATES as HistoryTemplates } from "../../src/altimate/native/fin import { SQL_TEMPLATES as AdvisorTemplates } from "../../src/altimate/native/finops/warehouse-advisor" import { SQL_TEMPLATES as UnusedTemplates } from "../../src/altimate/native/finops/unused-resources" import { SQL_TEMPLATES as RoleTemplates } from "../../src/altimate/native/finops/role-access" +import { sanitizeBqRegion, interpolateBqRegion } from "../../src/altimate/native/finops/bq-utils" import { ensureUpstreamSelector } from "../../src/altimate/native/dbt/runner" import { parseManifest } from "../../src/altimate/native/dbt/manifest" import { mapType } from "../../src/altimate/native/local/schema-sync" @@ -154,6 +155,156 @@ describe("FinOps: SQL template generation", () => { expect(built?.binds).toContain(14) }) + test("BigQuery history SQL uses error_result struct, not bare error_message / error_code", () => { + // INFORMATION_SCHEMA.JOBS exposes errors via error_result STRUCT + // (reason, location, message). Earlier versions selected bare `error_message` + // and `error_code` identifiers, which BQ rejects with + // "Unrecognized name: error_message" — don't regress. + const sql = HistoryTemplates.BIGQUERY_HISTORY_SQL + expect(sql).toContain("error_result.message as error_message") + expect(sql).toContain("error_result.reason as error_code") + expect(sql).not.toMatch(/^\s*error_message,\s*$/m) + expect(sql).not.toMatch(/^\s*NULL as error_code,?\s*$/m) + }) + + test("BigQuery history SQL does not reference total_rows (not a JOBS column)", () => { + // `total_rows` exists in INFORMATION_SCHEMA.PARTITIONS, not JOBS. Selecting + // it would throw "Unrecognized name: total_rows" once error_message is fixed. + const sql = HistoryTemplates.BIGQUERY_HISTORY_SQL + expect(sql).not.toMatch(/\btotal_rows\b/) + expect(sql).toContain("as rows_produced") + }) + + test("BigQuery history SQL derives execution_status from error_result (not bare state)", () => { + // BQ's `state` column returns 'DONE', not 'SUCCESS'. getQueryHistory() + // counts rows whose execution_status !== 'SUCCESS' as errors, so a bare + // `state as execution_status` would flag every completed BQ job as failed. + const sql = HistoryTemplates.BIGQUERY_HISTORY_SQL + expect(sql).toMatch(/CASE WHEN error_result IS NULL THEN 'SUCCESS'/) + expect(sql).not.toMatch(/^\s*state as execution_status,\s*$/m) + }) + + test("BigQuery history SQL defaults to region-us when no location provided", () => { + const built = HistoryTemplates.buildHistoryQuery("bigquery", 7, 100) + expect(built?.sql).toContain("`region-us.INFORMATION_SCHEMA.JOBS`") + expect(built?.sql).not.toContain("{region}") + }) + + test("BigQuery history SQL threads explicit region (EU)", () => { + const built = HistoryTemplates.buildHistoryQuery("bigquery", 7, 100, undefined, undefined, "EU") + expect(built?.sql).toContain("`region-eu.INFORMATION_SCHEMA.JOBS`") + }) + + test("BigQuery history SQL threads multi-part region (us-central1)", () => { + const built = HistoryTemplates.buildHistoryQuery("bigquery", 7, 100, undefined, undefined, "us-central1") + expect(built?.sql).toContain("`region-us-central1.INFORMATION_SCHEMA.JOBS`") + }) + + test("sanitizeBqRegion strips unsafe characters and falls back to 'us'", () => { + expect(sanitizeBqRegion("US")).toBe("us") + expect(sanitizeBqRegion("EU")).toBe("eu") + expect(sanitizeBqRegion("us-central1")).toBe("us-central1") + expect(sanitizeBqRegion("asia-east1")).toBe("asia-east1") + expect(sanitizeBqRegion("europe-west2")).toBe("europe-west2") + // Injection attempts — backtick, semicolon, comment markers must be stripped + expect(sanitizeBqRegion("us`; DROP TABLE")).toBe("usdroptable") + expect(sanitizeBqRegion("us/*x*/")).toBe("usx") + // Empty / non-string inputs fall back to "us" + expect(sanitizeBqRegion("")).toBe("us") + expect(sanitizeBqRegion(undefined)).toBe("us") + expect(sanitizeBqRegion(null)).toBe("us") + expect(sanitizeBqRegion(42)).toBe("us") + }) + + test("sanitizeBqRegion trims leading/trailing hyphens", () => { + // BQ region names never start or end with a hyphen; strip them so odd + // input like "-us-" becomes "us" instead of producing `region--us-`. + expect(sanitizeBqRegion("-us-")).toBe("us") + expect(sanitizeBqRegion("--eu-west1--")).toBe("eu-west1") + expect(sanitizeBqRegion("---")).toBe("us") + }) + + test("sanitizeBqRegion caps length at 64 chars", () => { + // Guard against pathological inputs — BQ region names are short (<30). + const long = "a".repeat(300) + const out = sanitizeBqRegion(long) + expect(out.length).toBeLessThanOrEqual(64) + expect(out).toMatch(/^a+$/) + }) + + test("interpolateBqRegion replaces every {region} placeholder", () => { + // Defensive against future templates that reference multiple INFORMATION_SCHEMA + // views (e.g. JOIN JOBS with JOBS_TIMELINE). replaceAll, not replace. + const tpl = "FROM `region-{region}.X` JOIN `region-{region}.Y`" + expect(interpolateBqRegion(tpl, "eu")).toBe("FROM `region-eu.X` JOIN `region-eu.Y`") + }) + + test("all BQ finops SQL templates use {region} placeholder (no hardcoded region-US)", () => { + // Catches the exact regression Gemini flagged: hardcoded region-US in + // role-access.ts and unused-resources.ts was shipped for months because + // the unit test suite only covered query-history. + const templates: Array<[string, string]> = [ + ["query-history BQ", HistoryTemplates.BIGQUERY_HISTORY_SQL], + ["credit-analyzer usage", CreditTemplates.BIGQUERY_CREDIT_USAGE_SQL], + ["credit-analyzer summary", CreditTemplates.BIGQUERY_CREDIT_SUMMARY_SQL], + ["credit-analyzer expensive", CreditTemplates.BIGQUERY_EXPENSIVE_SQL], + ["warehouse-advisor load", AdvisorTemplates.BIGQUERY_LOAD_SQL], + ["warehouse-advisor sizing", AdvisorTemplates.BIGQUERY_SIZING_SQL], + ["role-access grants", RoleTemplates.BIGQUERY_GRANTS_SQL], + ["unused-resources tables", UnusedTemplates.BIGQUERY_UNUSED_TABLES_SQL], + ] + for (const [name, sql] of templates) { + expect(sql, `${name} must use {region} placeholder`).toContain("{region}") + expect(sql, `${name} must not contain hardcoded region-US`).not.toContain("region-US") + } + }) + }) + + describe("BQ region threading (all finops modules)", () => { + test("credit-analyzer usage / summary / expensive SQL threads region", () => { + for (const [name, builder] of [ + ["buildCreditUsageSql", (r: string) => CreditTemplates.buildCreditUsageSql("bigquery", 7, 50, undefined, r)], + ["buildCreditSummarySql", (r: string) => CreditTemplates.buildCreditSummarySql("bigquery", 7, r)], + ["buildExpensiveSql", (r: string) => CreditTemplates.buildExpensiveSql("bigquery", 7, 20, r)], + ] as const) { + const eu = builder("EU") + expect(eu?.sql, `${name}(EU)`).toContain("`region-eu.INFORMATION_SCHEMA.JOBS`") + expect(eu?.sql, `${name}(EU) fully substituted`).not.toContain("{region}") + + // Default (undefined) falls back to us + const def = builder("") + expect(def?.sql, `${name}(default)`).toContain("`region-us.INFORMATION_SCHEMA.JOBS`") + } + }) + + test("warehouse-advisor load / sizing SQL threads region", () => { + const load = AdvisorTemplates.buildLoadSql("bigquery", 14, "asia-east1") + expect(load).toContain("`region-asia-east1.INFORMATION_SCHEMA.JOBS_TIMELINE`") + expect(load).not.toContain("{region}") + + const sizing = AdvisorTemplates.buildSizingSql("bigquery", 14, "EU") + expect(sizing).toContain("`region-eu.INFORMATION_SCHEMA.JOBS`") + expect(sizing).not.toContain("{region}") + }) + + test("role-access grants SQL threads region for BigQuery", () => { + const built = RoleTemplates.buildGrantsSql("bigquery", undefined, undefined, 100, "us-central1") + expect(built?.sql).toContain("`region-us-central1.INFORMATION_SCHEMA.OBJECT_PRIVILEGES`") + expect(built?.sql).not.toContain("{region}") + }) + + test("non-BigQuery warehouses ignore bqRegion parameter — Snowflake unchanged", () => { + const sf = HistoryTemplates.buildHistoryQuery("snowflake", 7, 100, undefined, undefined, "eu") + expect(sf?.sql).not.toContain("region-") + expect(sf?.sql).toContain("QUERY_HISTORY") + }) + + test("non-BigQuery warehouses ignore bqRegion parameter — Databricks unchanged", () => { + const db = HistoryTemplates.buildHistoryQuery("databricks", 7, 100, undefined, undefined, "eu") + expect(db?.sql).not.toContain("region-") + expect(db?.sql).toContain("system.query.history") + }) + test("builds Databricks history SQL", () => { const built = HistoryTemplates.buildHistoryQuery("databricks", 7, 50) expect(built?.sql).toContain("system.query.history")