From 885c9abe838be03141238b0a08e4ab8a45ac64c0 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Thu, 17 Sep 2026 22:42:57 -0400 Subject: [PATCH] Put the ordering prefix in the stored label, not in an expression Perspective orders column groups by the value string, so "01 - Actual" is the only way an arbitrary order can be expressed. That prefix now lives in pf.log.label, typed by whoever names the segment, rather than being built from a seq column by client-side expressions. pf_segment and pf_bucket read label first, and the expressions are shared between /agg and /data instead of being spelled out in each -- they have to agree, and they had drifted apart in whitespace already. The synthetic values lose their parentheses and their ordinals, except the adjustment fallback: '(adjustment)' sorted *before* '01 - ...', since '(' is 0x28 and digits begin at 0x30, so it becomes '99 - Adjustments' to sit last. Labelling an adjustment's own log row overrides that, which is how one kind of adjustment splits out from the rest. '(unlabeled load)' becomes plain 'Unlabeled', which needs no ordinal -- letters already follow digits. The load routes carry label and bucket onto the log row, so the fields the segment form has been offering since af9e6de are no longer a silent no-op. startEdit now reads them back, which it never did: editing a segment for any other reason blanked both. Existing sources need Generate SQL re-run -- the load templates are stored. Co-Authored-By: Claude Opus 5 (1M context) --- lib/sql_generator.js | 84 +++++++++++++++++++++++++++++---------- routes/operations.js | 39 ++++++++---------- ui/src/views/Baseline.jsx | 8 +++- 3 files changed, 87 insertions(+), 44 deletions(-) diff --git a/lib/sql_generator.js b/lib/sql_generator.js index 8ff165f..bae5a24 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -8,9 +8,60 @@ // Tokens baked in at generation time: column names, source schema.table // Tokens substituted at request time: {{fc_table}}, {{where_clause}}, {{exclude_clause}}, // {{version_id}}, {{logid}}, {{pf_user}}, {{note}}, +// {{label}}, {{bucket}}, // {{params}}, {{slice}}, {{date_from}}, {{date_to}}, // {{value_incr}}, {{units_incr}}, {{set_clause}}, {{scale_factor}} +// What the pivot shows for a row's segment and its bucket. +// +// The ordering prefix is part of the stored text, not computed here. Perspective +// orders column groups by the value string, so "01 - Actual" is the only way an +// arbitrary order can be expressed -- and l.label is where a person types it. +// Nothing derives it, which is deliberate: an earlier design built the prefix from +// a separate seq column, as Perspective expressions on the client, and the prefix +// then existed only inside the pivot -- so every other reader disagreed with it, +// and a label that could not be expressed in ExprTK's printable-ASCII-per-byte +// string scanner could not be ordered at all. Stored text has neither problem. +// +// The single exception is the adjustment fallback, whose 99 keeps unlabelled +// adjustments last. Labelling an adjustment's log row overrides it, which is how +// one kind of adjustment is split out from the rest -- l.label rather than tag or +// note, so a segment name stays separable from adjustment commentary (pf_note). +// +// Exported because /data builds its own statement in routes/operations.js while +// /agg is generated here, and the two have to agree. +const ADJUSTMENT_SEGMENT = '99 - Adjustments'; + +// An unlabelled load reads 'Unlabeled' and carries no prefix, so it sorts after +// everything numbered -- letters follow digits in ASCII. The old '(unlabeled load)' +// sorted *first*, since '(' is 0x28 and digits begin at 0x30. +const LOAD_SEGMENT = `COALESCE(NULLIF(l.label, ''), NULLIF(l.tag, ''), NULLIF(l.note, ''), 'Unlabeled')`; + +const SEGMENT_EXPR = `CASE WHEN l.operation IN ('baseline','reference') + THEN ${LOAD_SEGMENT} + ELSE COALESCE(NULLIF(l.label, ''), '${ADJUSTMENT_SEGMENT}') + END`; + +// What the row counts towards. A load falls back to its own name until it is +// bucketed; an adjustment falls back to 'Forecast', because that is what an +// adjustment is -- exclude_iters keeps operations off the reference segments, so +// there is no adjustment that is not part of the forecast. +const BUCKET_EXPR = `COALESCE(NULLIF(l.bucket, ''), + CASE WHEN l.operation IN ('baseline','reference') + THEN ${LOAD_SEGMENT} + ELSE 'Forecast' + END)`; + +const NOTE_EXPR = `CASE WHEN l.operation IN ('baseline','reference') + THEN NULL + ELSE COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, '')) + END`; + +// Every pf.log column the two expressions above read, for /agg's GROUP BY: they +// are functionally dependent on pf_logid, which is in the grain, but Postgres +// will not infer that. +const LABEL_GROUP_COLS = ['l.operation', 'l.label', 'l.tag', 'l.note', 'l.bucket']; + // wrap a column name in double quotes for safe use in SQL function q(name) { return `"${name}"`; } @@ -194,26 +245,16 @@ function generateSQL(source, colMeta) { return ` SELECT ${grainSelect('t.')} - ,CASE WHEN l.operation IN ('baseline','reference') - THEN COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, ''), '(unlabeled load)') - ELSE '(adjustment)' END AS pf_segment - ,COALESCE(NULLIF(l.bucket, ''), - CASE WHEN l.operation IN ('baseline','reference') - THEN COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, ''), '(unlabeled load)') - ELSE 'Forecast' END) AS pf_bucket - ,CASE WHEN l.operation IN ('baseline','reference') - THEN NULL - ELSE COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, '')) END AS pf_note - ,l.operation AS pf_op + ,${SEGMENT_EXPR} AS pf_segment + ,${BUCKET_EXPR} AS pf_bucket + ,${NOTE_EXPR} AS pf_note + ,l.operation AS pf_op FROM {{fc_table}} t LEFT JOIN pf.log l ON l.id = t.pf_logid GROUP BY ${grain.groupCols('t.').join('\n ,')} - ,l.operation - ,l.tag - ,l.note - ,l.bucket`.trim(); + ,${LABEL_GROUP_COLS.join('\n ,')}`.trim(); } // grain columns + pf_gkey + summed measures, in the leading-comma style the @@ -262,8 +303,9 @@ GROUP BY return ` WITH ilog AS ( - INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note) - VALUES ({{version_id}}, '{{pf_user}}', 'baseline', NULL, '{{params}}'::jsonb, '{{note}}') + INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note, label, bucket) + VALUES ({{version_id}}, '{{pf_user}}', 'baseline', NULL, '{{params}}'::jsonb, '{{note}}', + NULLIF('{{label}}', ''), NULLIF('{{bucket}}', '')) RETURNING id ) ,ins AS ( @@ -282,8 +324,9 @@ SELECT count(*) AS rows_affected FROM ins`.trim(); return ` WITH ilog AS ( - INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note) - VALUES ({{version_id}}, '{{pf_user}}', 'reference', NULL, '{{params}}'::jsonb, '{{note}}') + INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note, label, bucket) + VALUES ({{version_id}}, '{{pf_user}}', 'reference', NULL, '{{params}}'::jsonb, '{{note}}', + NULLIF('{{label}}', ''), NULLIF('{{bucket}}', '')) RETURNING id ) ,ins AS ( @@ -544,4 +587,5 @@ function esc(val) { return String(val).replace(/'/g, "''"); } -module.exports = { generateSQL, grainOf, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; +module.exports = { generateSQL, grainOf, + SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, ADJUSTMENT_SEGMENT, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; diff --git a/routes/operations.js b/routes/operations.js index 067e5be..3e6bfc7 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -1,6 +1,7 @@ const express = require('express'); const { tableFromArrays, tableToIPC } = require('apache-arrow'); -const { applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc } = require('../lib/sql_generator'); +const { applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, + SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, ADJUSTMENT_SEGMENT } = require('../lib/sql_generator'); const { sessionUser } = require('../lib/auth'); const { fcTable } = require('../lib/utils'); @@ -334,21 +335,9 @@ module.exports = function(pool) { await client.query(` DECLARE pf_cur CURSOR FOR SELECT t.* - ,CASE WHEN l.operation IN ('baseline','reference') - THEN COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, ''), '(unlabeled load)') - ELSE '(adjustment)' END AS pf_segment - -- What the row counts towards. A load falls back to its own - -- name until it is labelled; an adjustment falls back to - -- 'Forecast', because that is what an adjustment is -- exclude_iters - -- keeps operations off the reference segments, so there is no - -- adjustment that is not part of the forecast. - ,COALESCE(NULLIF(l.bucket, ''), - CASE WHEN l.operation IN ('baseline','reference') - THEN COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, ''), '(unlabeled load)') - ELSE 'Forecast' END) AS pf_bucket - ,CASE WHEN l.operation IN ('baseline','reference') - THEN NULL - ELSE COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, '')) END AS pf_note + ,${SEGMENT_EXPR} AS pf_segment + ,${BUCKET_EXPR} AS pf_bucket + ,${NOTE_EXPR} AS pf_note FROM ${tbl} t LEFT JOIN pf.log l ON l.id = t.pf_logid @@ -420,7 +409,7 @@ module.exports = function(pool) { // load baseline rows from source table — additive, no delete router.post('/versions/:id/baseline', async (req, res) => { - const { where_clause, date_offset, note, filters, raw_where } = req.body; + const { where_clause, date_offset, note, filters, raw_where, label, bucket } = req.body; const pf_user = sessionUser(req); const dateOffset = date_offset || '0 days'; if (!await assertInterval(dateOffset, res)) return; @@ -438,6 +427,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), + label: esc(label || ''), + bucket: esc(bucket || ''), params: esc(paramsJson), filter_clause: filterClause, date_offset: esc(dateOffset) @@ -457,7 +448,7 @@ module.exports = function(pool) { router.put('/versions/:id/baseline/:logid', async (req, res) => { const versionId = parseInt(req.params.id); const logid = parseInt(req.params.logid); - const { where_clause, date_offset, note, filters, raw_where } = req.body; + const { where_clause, date_offset, note, filters, raw_where, label, bucket } = req.body; const pf_user = sessionUser(req); const dateOffset = date_offset || '0 days'; if (!await assertInterval(dateOffset, res)) return; @@ -501,6 +492,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), + label: esc(label || ''), + bucket: esc(bucket || ''), params: esc(paramsJson), filter_clause: filterClause, date_offset: esc(dateOffset) @@ -565,7 +558,7 @@ module.exports = function(pool) { // load reference rows from source table (additive — does not clear prior reference rows) router.post('/versions/:id/reference', async (req, res) => { - const { where_clause, date_offset, note, filters, raw_where } = req.body; + const { where_clause, date_offset, note, filters, raw_where, label, bucket } = req.body; const pf_user = sessionUser(req); const dateOffset = date_offset || '0 days'; const filterClause = (raw_where || where_clause || '').trim() || 'TRUE'; @@ -582,6 +575,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), + label: esc(label || ''), + bucket: esc(bucket || ''), params: esc(paramsJson), filter_clause: filterClause, date_offset: esc(dateOffset) @@ -666,7 +661,7 @@ module.exports = function(pool) { await client.query('COMMIT'); committed = true; const opLabel = (req.body.tag || '').trim() || note || null; - const rows = allRows.map(r => ({ ...r, pf_segment: '(adjustment)', pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'scale' })); + const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'scale' })); res.json({ rows, rows_affected: rows.length, @@ -726,7 +721,7 @@ module.exports = function(pool) { await client.query('COMMIT'); committed = true; const opLabel = (req.body.tag || '').trim() || note || null; - const rows = allRows.map(r => ({ ...r, pf_segment: '(adjustment)', pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'recode' })); + const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'recode' })); res.json({ rows, rows_affected: rows.length, slices_applied: units.length }); } finally { if (!committed) try { await client.query('ROLLBACK'); } catch {} @@ -821,7 +816,7 @@ module.exports = function(pool) { await client.query('COMMIT'); committed = true; const opLabel = (req.body.tag || '').trim() || note || null; - const rows = allRows.map(r => ({ ...r, pf_segment: '(adjustment)', pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'clone' })); + const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'clone' })); res.json({ rows, rows_affected: rows.length, slices_applied: units.length }); } finally { if (!committed) try { await client.query('ROLLBACK'); } catch {} diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index 6df4e77..783c6f0 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -242,8 +242,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio where_clause: clause, note: description || segNote, date_offset: offsetStr, - ...(segBucket.trim() ? { bucket: segBucket.trim() } : {}), - ...(segLabel.trim() ? { label: segLabel.trim() } : {}), + label: segLabel.trim(), + bucket: segBucket.trim(), ...(useRaw ? { raw_where: clause } : { filters }), } setSubmitting(true) @@ -279,6 +279,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio const params = entry.params || {} setSegType(entry.operation) setSegNote(entry.note || '') + setSegLabel(entry.label || '') + setSegBucket(entry.bucket || '') setDescription('') setOffset(params.date_offset || '0 days') const groups = normalizeFilters(params.filters) @@ -308,6 +310,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio setShowAddForm(false) setDescription('') setSegNote('') + setSegLabel('') + setSegBucket('') setOffsetYr(0) setOffsetMo(0) setUseRaw(false)