diff --git a/lib/sql_generator.js b/lib/sql_generator.js index 02a0921..445f699 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -512,13 +512,53 @@ function applyTokens(sql, tokens) { // build a SQL WHERE clause string from a slice object // only dimension columns are included; unrecognised keys are silently skipped -function buildWhere(slice, dimCols) { +// pf_segment and pf_bucket are not columns on the forecast table -- they are +// computed at read time from the row's pf.log entry -- so a slice naming one +// cannot be compared directly. It resolves to a set of log ids instead, which is +// exact: the name lives on the log row, and every forecast row carries the +// pf_logid that points at it. +// +// Without this they were dropped from the slice, and clicking a single bucket's +// cell scaled every bucket at that dimension intersection while the panel showed +// only the one clicked. +const COMPUTED_SLICE_COLS = { pf_segment: SEGMENT_EXPR, pf_bucket: BUCKET_EXPR }; + +function computedSlicePredicate(col, val, versionId) { + if (versionId == null) { + const err = new Error(`Cannot filter on ${col} without a version`); + err.status = 500; + throw err; + } + const vals = (Array.isArray(val) ? val : [val]).map(v => `'${esc(v)}'`).join(', '); + return `pf_logid IN ( + SELECT l.id + FROM pf.log l${VERSION_JOIN} + WHERE l.version_id = ${parseInt(versionId)} + AND ${COMPUTED_SLICE_COLS[col]} IN (${vals}) +)`; +} + +function buildWhere(slice, dimCols, versionId) { if (!slice || Object.keys(slice).length === 0) return 'TRUE'; const allowed = new Set(dimCols); const parts = []; for (const [col, val] of Object.entries(slice)) { + if (COMPUTED_SLICE_COLS[col]) { + parts.push(computedSlicePredicate(col, val, versionId)); + continue; + } + // A pf_ key this does not understand is refused rather than skipped. + // Skipping is how a selection silently widened: the operation ran against + // everything the dropped key would have excluded. pf_iter is the one + // exception -- the client strips it deliberately, since two cells that + // differ only by iter band are the same slice. + if (col.startsWith('pf_') && col !== 'pf_iter') { + const err = new Error(`Slice names ${col}, which cannot be filtered on`); + err.status = 400; + throw err; + } if (!allowed.has(col)) continue; if (Array.isArray(val)) { const escaped = val.map(v => esc(v)); @@ -535,13 +575,13 @@ function buildWhere(slice, dimCols) { // A union of slices cannot be flattened into one IN list per column: slices // {Region:East, State:NY} and {Region:West, State:CA} would become // Region IN (East,West) AND State IN (NY,CA), which also matches East/CA. -function buildWhereAny(slices, dimCols) { +function buildWhereAny(slices, dimCols, versionId) { const list = (slices || []).filter(s => s && Object.keys(s).length > 0); if (list.length === 0) return 'TRUE'; - if (list.length === 1) return buildWhere(list[0], dimCols); + if (list.length === 1) return buildWhere(list[0], dimCols, versionId); const groups = list - .map(s => buildWhere(s, dimCols)) + .map(s => buildWhere(s, dimCols, versionId)) .filter(w => w !== 'TRUE'); // any slice that reduced to TRUE selects everything, so the union does too @@ -627,6 +667,6 @@ function esc(val) { return String(val).replace(/'/g, "''"); } -module.exports = { generateSQL, grainOf, +module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, VERSION_JOIN, ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, UNLABELED_LOAD, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; diff --git a/routes/operations.js b/routes/operations.js index adaf42b..28cac9b 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -1,6 +1,6 @@ const express = require('express'); const { tableFromArrays, tableToIPC } = require('apache-arrow'); -const { applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, +const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, VERSION_JOIN, ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator'); const { sessionUser } = require('../lib/auth'); @@ -45,9 +45,10 @@ module.exports = function(pool) { // rather than distribute an amount, so for them this only decides whether the // work lands as one log entry or several. function sliceUnits(slices, ctx, applyMode) { + const vid = ctx.version.id; return applyMode === 'each' - ? slices.map(sl => ({ slices: [sl], where: buildWhere(sl, ctx.filterCols) })) - : [{ slices, where: buildWhereAny(slices, ctx.filterCols) }]; + ? slices.map(sl => ({ slices: [sl], where: buildWhere(sl, ctx.filterCols, vid) })) + : [{ slices, where: buildWhereAny(slices, ctx.filterCols, vid) }]; } // The offset is interpolated into the statement as an interval literal, so a @@ -73,7 +74,7 @@ module.exports = function(pool) { // otherwise reduce to TRUE and apply the operation to the whole version. // Refuse rather than let a malformed selection rewrite every row. function assertSelective(slices, ctx) { - const allowed = new Set(ctx.filterCols); + const allowed = new Set([...ctx.filterCols, ...Object.keys(COMPUTED_SLICE_COLS)]); slices.forEach((sl, i) => { const hits = Object.keys(sl).filter(k => allowed.has(k)); if (hits.length === 0) { diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 566ff1a..3803b6c 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -299,7 +299,16 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se const hasExcl = excl.rows > 0 && (excl.value !== 0 || excl.units !== 0) const onTotal = hasExcl && targetBasis !== 'adjustable' // 'selected total' is the default const grand = { value: total.value + excl.value, units: total.units + excl.units } - const exclName = (currentTotals?.excludedIters || []).join(' / ') || 'excluded' + // Named by the segments themselves where we have them -- "02 - Prior Year" + // reads as a thing a forecaster recognises, where "reference" names only the + // iter band that happens to exclude it. + const exclName = (currentTotals?.excluded?.names || []).join(' · ') + || (currentTotals?.excludedIters || []).join(' / ') + || 'excluded' + + // Everything in the selection is immovable. Worth saying outright: the panel + // otherwise prints a row of zeros and leaves the reason to be worked out. + const nothingToAdjust = hasExcl && !total.value && !total.units // the basis decides which line the editable rows are measured from const basisOf = (key) => { @@ -443,12 +452,15 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se {/* Rows the pivot shows but operations cannot write. Listed so the panel's figures reconcile with what the grid displays. */} {hasExcl && ( - - - {exclName} · fixed + + + {exclName} + + final + {measures.map(m => ( - + {m.key === 'price' ? fmtNum(priceOf(excl), m.dp) : fmtNum(excl[m.key], m.dp)} ))} @@ -468,6 +480,19 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se )} + {/* Zeros in the Adjustable row are a true answer to the wrong question: + they say how much can move, not why none of it can. Spell it out + where the eye already is, rather than leaving the edit rows to fail + silently below. */} + {nothingToAdjust && ( + + + Nothing in this selection can be adjusted — all of it is {exclName}, + loaded as {(currentTotals?.excludedIters || []).join(' / ') || 'reference'}. + + + )} + {rule}{measures.map(m =>
)} {/* the edit — three equivalent ways to say the same thing */} diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index c014557..dc8f382 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -12,6 +12,11 @@ import '@perspective-dev/viewer/inline' import '@perspective-dev/viewer-datagrid' import '@perspective-dev/viewer/themes' +// Slice keys that are not col_meta columns: computed from pf.log when the rows are +// served, so they are real columns in the loaded table but have to be resolved back +// to log ids server-side. Mirrors COMPUTED_SLICE_COLS in lib/sql_generator.js. +const COMPUTED_SLICE_COLS = new Set(['pf_segment', 'pf_bucket']) + const LAYOUT_KEY = (vid) => `pf_layout_v${vid}` // last-used layout (auto restore) const LAYOUTS_KEY = (vid) => `pf_layouts_v${vid}` // named layout list @@ -406,6 +411,13 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio async function totalsFor(sliceObj) { const filters = [ + // pf_segment and pf_bucket are computed server-side but are ordinary + // string columns in the loaded table, so here they filter directly. They + // have to be applied, or the ledger totals a wider selection than the + // operation will write. + ...Object.entries(sliceObj) + .filter(([col]) => COMPUTED_SLICE_COLS.has(col)) + .map(([col, val]) => [col, '==', String(val)]), ...Object.entries(sliceObj) .filter(([col]) => dimNames.has(col)) .map(([col, val]) => [col, '==', val]), @@ -424,7 +436,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // rows the pivot shows but operations cannot write (usually 'reference'). // Kept separate rather than filtered away: the grid total includes them, // so the panel has to account for them or the two disagree. - const excluded = { value: 0, units: 0, rows: 0 } + const excluded = { value: 0, units: 0, rows: 0, names: new Set() } for (const r of rows) { const k = r.pf_iter || '?' const val = valueCol ? (parseFloat(r[valueCol]) || 0) : 0 @@ -434,6 +446,10 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio excluded.value += val excluded.units += uni excluded.rows += 1 + // Name them by what they are, not by the iter band that happens to + // exclude them: "02 - Prior Year" means something to a forecaster, + // "reference" is the mechanism. + if (r.pf_segment) excluded.names.add(String(r.pf_segment)) continue } @@ -505,10 +521,12 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio value: acc.value + (ps.excluded?.value || 0), units: acc.units + (ps.excluded?.units || 0), rows: acc.rows + (ps.excluded?.rows || 0), - }), { value: 0, units: 0, rows: 0 }) + names: new Set([...acc.names, ...(ps.excluded?.names || [])]), + }), { value: 0, units: 0, rows: 0, names: new Set() }) setCurrentTotals({ - byIter, byEntry, total, excluded, valueCol, unitsCol, perSlice, + byIter, byEntry, total, valueCol, unitsCol, perSlice, + excluded: { ...excluded, names: [...excluded.names].sort() }, excludedIters: [...excludeIters], }) } catch { @@ -1330,6 +1348,10 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const dateCols = new Set(colMetaRef.current.filter(c => c.role === 'date').map(c => c.cname)) const out = {} for (const [k, v] of Object.entries(raw)) { + // Not col_meta columns, but real ones here and resolvable server-side to the + // set of pf.log ids that carry the name. Dropping them is what let a click on + // one bucket's cell scale every bucket at that intersection. + if (COMPUTED_SLICE_COLS.has(k)) { out[k] = v; continue } if (dimCols.has(k)) { out[k] = v; continue } if (dateCols.has(k)) { const ms = Number(v)