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) <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-17 22:42:57 -04:00
parent 893a395529
commit 885c9abe83
3 changed files with 87 additions and 44 deletions

View File

@ -8,9 +8,60 @@
// Tokens baked in at generation time: column names, source schema.table // Tokens baked in at generation time: column names, source schema.table
// Tokens substituted at request time: {{fc_table}}, {{where_clause}}, {{exclude_clause}}, // Tokens substituted at request time: {{fc_table}}, {{where_clause}}, {{exclude_clause}},
// {{version_id}}, {{logid}}, {{pf_user}}, {{note}}, // {{version_id}}, {{logid}}, {{pf_user}}, {{note}},
// {{label}}, {{bucket}},
// {{params}}, {{slice}}, {{date_from}}, {{date_to}}, // {{params}}, {{slice}}, {{date_from}}, {{date_to}},
// {{value_incr}}, {{units_incr}}, {{set_clause}}, {{scale_factor}} // {{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 // wrap a column name in double quotes for safe use in SQL
function q(name) { return `"${name}"`; } function q(name) { return `"${name}"`; }
@ -194,26 +245,16 @@ function generateSQL(source, colMeta) {
return ` return `
SELECT SELECT
${grainSelect('t.')} ${grainSelect('t.')}
,CASE WHEN l.operation IN ('baseline','reference') ,${SEGMENT_EXPR} AS pf_segment
THEN COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, ''), '(unlabeled load)') ,${BUCKET_EXPR} AS pf_bucket
ELSE '(adjustment)' END AS pf_segment ,${NOTE_EXPR} AS pf_note
,COALESCE(NULLIF(l.bucket, ''), ,l.operation AS pf_op
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
FROM {{fc_table}} t FROM {{fc_table}} t
LEFT JOIN pf.log l LEFT JOIN pf.log l
ON l.id = t.pf_logid ON l.id = t.pf_logid
GROUP BY GROUP BY
${grain.groupCols('t.').join('\n ,')} ${grain.groupCols('t.').join('\n ,')}
,l.operation ,${LABEL_GROUP_COLS.join('\n ,')}`.trim();
,l.tag
,l.note
,l.bucket`.trim();
} }
// grain columns + pf_gkey + summed measures, in the leading-comma style the // grain columns + pf_gkey + summed measures, in the leading-comma style the
@ -262,8 +303,9 @@ GROUP BY
return ` return `
WITH WITH
ilog AS ( ilog AS (
INSERT INTO pf.log (version_id, pf_user, operation, slice, params, 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}}') VALUES ({{version_id}}, '{{pf_user}}', 'baseline', NULL, '{{params}}'::jsonb, '{{note}}',
NULLIF('{{label}}', ''), NULLIF('{{bucket}}', ''))
RETURNING id RETURNING id
) )
,ins AS ( ,ins AS (
@ -282,8 +324,9 @@ SELECT count(*) AS rows_affected FROM ins`.trim();
return ` return `
WITH WITH
ilog AS ( ilog AS (
INSERT INTO pf.log (version_id, pf_user, operation, slice, params, 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}}') VALUES ({{version_id}}, '{{pf_user}}', 'reference', NULL, '{{params}}'::jsonb, '{{note}}',
NULLIF('{{label}}', ''), NULLIF('{{bucket}}', ''))
RETURNING id RETURNING id
) )
,ins AS ( ,ins AS (
@ -544,4 +587,5 @@ function esc(val) {
return String(val).replace(/'/g, "''"); 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 };

View File

@ -1,6 +1,7 @@
const express = require('express'); const express = require('express');
const { tableFromArrays, tableToIPC } = require('apache-arrow'); 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 { sessionUser } = require('../lib/auth');
const { fcTable } = require('../lib/utils'); const { fcTable } = require('../lib/utils');
@ -334,21 +335,9 @@ module.exports = function(pool) {
await client.query(` await client.query(`
DECLARE pf_cur CURSOR FOR DECLARE pf_cur CURSOR FOR
SELECT t.* SELECT t.*
,CASE WHEN l.operation IN ('baseline','reference') ,${SEGMENT_EXPR} AS pf_segment
THEN COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, ''), '(unlabeled load)') ,${BUCKET_EXPR} AS pf_bucket
ELSE '(adjustment)' END AS pf_segment ,${NOTE_EXPR} AS pf_note
-- 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
FROM ${tbl} t FROM ${tbl} t
LEFT JOIN pf.log l LEFT JOIN pf.log l
ON l.id = t.pf_logid ON l.id = t.pf_logid
@ -420,7 +409,7 @@ module.exports = function(pool) {
// load baseline rows from source table — additive, no delete // load baseline rows from source table — additive, no delete
router.post('/versions/:id/baseline', async (req, res) => { 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 pf_user = sessionUser(req);
const dateOffset = date_offset || '0 days'; const dateOffset = date_offset || '0 days';
if (!await assertInterval(dateOffset, res)) return; if (!await assertInterval(dateOffset, res)) return;
@ -438,6 +427,8 @@ module.exports = function(pool) {
version_id: ctx.version.id, version_id: ctx.version.id,
pf_user: esc(pf_user || ''), pf_user: esc(pf_user || ''),
note: esc(note || ''), note: esc(note || ''),
label: esc(label || ''),
bucket: esc(bucket || ''),
params: esc(paramsJson), params: esc(paramsJson),
filter_clause: filterClause, filter_clause: filterClause,
date_offset: esc(dateOffset) date_offset: esc(dateOffset)
@ -457,7 +448,7 @@ module.exports = function(pool) {
router.put('/versions/:id/baseline/:logid', async (req, res) => { router.put('/versions/:id/baseline/:logid', async (req, res) => {
const versionId = parseInt(req.params.id); const versionId = parseInt(req.params.id);
const logid = parseInt(req.params.logid); 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 pf_user = sessionUser(req);
const dateOffset = date_offset || '0 days'; const dateOffset = date_offset || '0 days';
if (!await assertInterval(dateOffset, res)) return; if (!await assertInterval(dateOffset, res)) return;
@ -501,6 +492,8 @@ module.exports = function(pool) {
version_id: ctx.version.id, version_id: ctx.version.id,
pf_user: esc(pf_user || ''), pf_user: esc(pf_user || ''),
note: esc(note || ''), note: esc(note || ''),
label: esc(label || ''),
bucket: esc(bucket || ''),
params: esc(paramsJson), params: esc(paramsJson),
filter_clause: filterClause, filter_clause: filterClause,
date_offset: esc(dateOffset) 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) // load reference rows from source table (additive — does not clear prior reference rows)
router.post('/versions/:id/reference', async (req, res) => { 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 pf_user = sessionUser(req);
const dateOffset = date_offset || '0 days'; const dateOffset = date_offset || '0 days';
const filterClause = (raw_where || where_clause || '').trim() || 'TRUE'; const filterClause = (raw_where || where_clause || '').trim() || 'TRUE';
@ -582,6 +575,8 @@ module.exports = function(pool) {
version_id: ctx.version.id, version_id: ctx.version.id,
pf_user: esc(pf_user || ''), pf_user: esc(pf_user || ''),
note: esc(note || ''), note: esc(note || ''),
label: esc(label || ''),
bucket: esc(bucket || ''),
params: esc(paramsJson), params: esc(paramsJson),
filter_clause: filterClause, filter_clause: filterClause,
date_offset: esc(dateOffset) date_offset: esc(dateOffset)
@ -666,7 +661,7 @@ module.exports = function(pool) {
await client.query('COMMIT'); await client.query('COMMIT');
committed = true; committed = true;
const opLabel = (req.body.tag || '').trim() || note || null; 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({ res.json({
rows, rows,
rows_affected: rows.length, rows_affected: rows.length,
@ -726,7 +721,7 @@ module.exports = function(pool) {
await client.query('COMMIT'); await client.query('COMMIT');
committed = true; committed = true;
const opLabel = (req.body.tag || '').trim() || note || null; 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 }); res.json({ rows, rows_affected: rows.length, slices_applied: units.length });
} finally { } finally {
if (!committed) try { await client.query('ROLLBACK'); } catch {} if (!committed) try { await client.query('ROLLBACK'); } catch {}
@ -821,7 +816,7 @@ module.exports = function(pool) {
await client.query('COMMIT'); await client.query('COMMIT');
committed = true; committed = true;
const opLabel = (req.body.tag || '').trim() || note || null; 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 }); res.json({ rows, rows_affected: rows.length, slices_applied: units.length });
} finally { } finally {
if (!committed) try { await client.query('ROLLBACK'); } catch {} if (!committed) try { await client.query('ROLLBACK'); } catch {}

View File

@ -242,8 +242,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
where_clause: clause, where_clause: clause,
note: description || segNote, note: description || segNote,
date_offset: offsetStr, date_offset: offsetStr,
...(segBucket.trim() ? { bucket: segBucket.trim() } : {}), label: segLabel.trim(),
...(segLabel.trim() ? { label: segLabel.trim() } : {}), bucket: segBucket.trim(),
...(useRaw ? { raw_where: clause } : { filters }), ...(useRaw ? { raw_where: clause } : { filters }),
} }
setSubmitting(true) setSubmitting(true)
@ -279,6 +279,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
const params = entry.params || {} const params = entry.params || {}
setSegType(entry.operation) setSegType(entry.operation)
setSegNote(entry.note || '') setSegNote(entry.note || '')
setSegLabel(entry.label || '')
setSegBucket(entry.bucket || '')
setDescription('') setDescription('')
setOffset(params.date_offset || '0 days') setOffset(params.date_offset || '0 days')
const groups = normalizeFilters(params.filters) const groups = normalizeFilters(params.filters)
@ -308,6 +310,8 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
setShowAddForm(false) setShowAddForm(false)
setDescription('') setDescription('')
setSegNote('') setSegNote('')
setSegLabel('')
setSegBucket('')
setOffsetYr(0) setOffsetYr(0)
setOffsetMo(0) setOffsetMo(0)
setUseRaw(false) setUseRaw(false)