From 2cb17766c96712d767db7045d5351aa5aabb5459 Mon Sep 17 00:00:00 2001 From: stefanbaxter Date: Tue, 13 Oct 2026 11:05:08 +0000 Subject: [PATCH] feat(cubejs): warn when update_window is set without incremental Cube honours refresh_key.update_window only with incremental: true. Without it every partition in the build range is rebuilt on each refresh tick. abler's BigQuery models had 20 such pre-aggregations since April and rebuilt ~270 partitions an hour on dev, closed months included, every 30 minutes for the busiest two. Cube's compiler is silent about this, so /api/v1/validate (editor save) and /api/v1/validate-in-branch (Model Management API, FraiOS) now return a warning on the update_window line. Non-blocking: valid is unchanged. Co-Authored-By: Claude Opus 5.5 --- services/cubejs/src/routes/validate.js | 7 +- .../cubejs/src/routes/validateInBranch.js | 6 +- .../__tests__/preAggregationWarnings.test.js | 72 +++++++++++++++++++ .../src/utils/preAggregationWarnings.js | 45 ++++++++++++ 4 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 services/cubejs/src/utils/__tests__/preAggregationWarnings.test.js create mode 100644 services/cubejs/src/utils/preAggregationWarnings.js diff --git a/services/cubejs/src/routes/validate.js b/services/cubejs/src/routes/validate.js index 5a05afb9..204f8c74 100644 --- a/services/cubejs/src/routes/validate.js +++ b/services/cubejs/src/routes/validate.js @@ -1,5 +1,7 @@ import { prepareCompiler } from "@cubejs-backend/schema-compiler"; +import { preAggregationWarnings } from "../utils/preAggregationWarnings.js"; + /** * In-memory schema file repository that satisfies the SchemaFileRepository * interface from @cubejs-backend/shared. Each file has { fileName, content }. @@ -68,7 +70,7 @@ export default async (req, res) => { try { const repo = new InMemorySchemaFileRepository(files); - const { compiler } = prepareCompiler(repo, { + const { compiler, cubeEvaluator } = prepareCompiler(repo, { allowNodeRequire: false, standalone: true, }); @@ -109,6 +111,9 @@ export default async (req, res) => { const errors = rawErrors.map(mapCompilerError); const warnings = rawWarnings.map(mapSyntaxWarning); + if (rawErrors.length === 0 && !compileError) { + warnings.push(...preAggregationWarnings(cubeEvaluator, files)); + } return res.json({ valid: errors.length === 0, diff --git a/services/cubejs/src/routes/validateInBranch.js b/services/cubejs/src/routes/validateInBranch.js index 0593d8ea..e9421090 100644 --- a/services/cubejs/src/routes/validateInBranch.js +++ b/services/cubejs/src/routes/validateInBranch.js @@ -1,4 +1,5 @@ import { prepareCompiler } from "@cubejs-backend/schema-compiler"; +import { preAggregationWarnings } from "../utils/preAggregationWarnings.js"; import { verifyAndProvision } from "../utils/directVerifyAuth.js"; import { emitModelEvent } from "../utils/eventEmitter.js"; @@ -283,7 +284,7 @@ export default async function validateInBranch(req, res) { // Compile. try { const repo = new InMemorySchemaFileRepository(files); - const { compiler } = prepareCompiler(repo, { + const { compiler, cubeEvaluator } = prepareCompiler(repo, { allowNodeRequire: false, standalone: true, }); @@ -322,6 +323,9 @@ export default async function validateInBranch(req, res) { } const warnings = rawWarnings.map(mapSyntaxWarning); + if (rawErrors.length === 0 && !compileError) { + warnings.push(...preAggregationWarnings(cubeEvaluator, files)); + } const result = { valid: errors.length === 0, errors, diff --git a/services/cubejs/src/utils/__tests__/preAggregationWarnings.test.js b/services/cubejs/src/utils/__tests__/preAggregationWarnings.test.js new file mode 100644 index 00000000..8924ee61 --- /dev/null +++ b/services/cubejs/src/utils/__tests__/preAggregationWarnings.test.js @@ -0,0 +1,72 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { prepareCompiler } from "@cubejs-backend/schema-compiler"; + +import { preAggregationWarnings } from "../preAggregationWarnings.js"; + +const MODEL = `cubes: + - name: orders + sql_table: public.orders + measures: + - name: count + type: count + dimensions: + - name: created_at + sql: created_at + type: time + pre_aggregations: + - name: not_incremental + measures: [count] + time_dimension: created_at + granularity: day + partition_granularity: month + refresh_key: + every: 30 minutes + update_window: 14 day + - name: incremental + measures: [count] + time_dimension: created_at + granularity: day + partition_granularity: month + refresh_key: + every: 1 hour + incremental: true + update_window: 7 day + - name: no_window + measures: [count] + refresh_key: + every: 1 hour +`; + +async function compile(content) { + const files = [{ fileName: "orders.yml", content }]; + const { compiler, cubeEvaluator } = prepareCompiler( + { localPath: () => "/", dataSchemaFiles: async () => files }, + { allowNodeRequire: false, standalone: true } + ); + await compiler.compile(); + return { cubeEvaluator, files }; +} + +describe("preAggregationWarnings", () => { + it("warns only on update_window without incremental, at its line", async () => { + const { cubeEvaluator, files } = await compile(MODEL); + const warnings = preAggregationWarnings(cubeEvaluator, files); + + assert.equal(warnings.length, 1); + assert.equal(warnings[0].severity, "warning"); + assert.equal(warnings[0].fileName, "orders.yml"); + assert.equal(warnings[0].startLine, 19); + assert.match(warnings[0].message, /orders\.not_incremental/); + assert.match(warnings[0].message, /incremental: true/); + }); + + it("is silent once incremental is set", async () => { + const fixed = MODEL.replace( + "every: 30 minutes\n", + "every: 30 minutes\n incremental: true\n" + ); + const { cubeEvaluator, files } = await compile(fixed); + assert.deepEqual(preAggregationWarnings(cubeEvaluator, files), []); + }); +}); diff --git a/services/cubejs/src/utils/preAggregationWarnings.js b/services/cubejs/src/utils/preAggregationWarnings.js new file mode 100644 index 00000000..eaf423c5 --- /dev/null +++ b/services/cubejs/src/utils/preAggregationWarnings.js @@ -0,0 +1,45 @@ +/** + * Model warnings Cube's compiler does not raise. + * + * Cube honours `refresh_key.update_window` only with `incremental: true`. + * Without it every partition in the build range is rebuilt on each refresh + * tick: on 2026-10-13 abler's BigQuery models rebuilt ~270 partitions an + * hour, closed months included, because of this. + * + * @param {object} cubeEvaluator from prepareCompiler, after compile() + * @param {Array<{fileName: string, content: string}>} files the compiled files + */ +export function preAggregationWarnings(cubeEvaluator, files) { + const contentByFile = new Map(files.map((f) => [f.fileName, f.content || ""])); + const warnings = []; + + for (const cubeName of cubeEvaluator.cubeNames()) { + const cube = cubeEvaluator.cubeFromPath(cubeName); + for (const [name, preAgg] of Object.entries(cube.preAggregations || {})) { + const refreshKey = preAgg.refreshKey || {}; + if (!refreshKey.updateWindow || refreshKey.incremental) continue; + + const line = locate(contentByFile.get(cube.fileName), name); + warnings.push({ + severity: "warning", + message: `${cubeName}.${name}: update_window has no effect without incremental: true, so every partition is rebuilt on each refresh. Add incremental: true to refresh_key.`, + fileName: cube.fileName || null, + startLine: line, + startColumn: line ? 1 : null, + endLine: line, + endColumn: null, + }); + } + } + return warnings; +} + +// Line of the update_window key after the pre-aggregation's name; null if not found. +function locate(content, preAggName) { + if (!content) return null; + const lines = content.split("\n"); + const start = lines.findIndex((l) => new RegExp(`\\b${preAggName}\\b`).test(l)); + if (start < 0) return null; + const hit = lines.findIndex((l, i) => i > start && /\b(update_window|updateWindow)\b/.test(l)); + return (hit < 0 ? start : hit) + 1; +}