Refuse offsets finer than a day; recode skips zero coordinates

A bare number is an interval in seconds to Postgres, so "30" was
accepted as a date offset, cast back to the same date, and a shifted
recode took rows out of a cell and put them back into it. Offsets are
now refused unless they are a whole number of days.

Recode no longer writes a take-out/put-back pair for coordinates that
already net to zero on value and units, and refuses, as scale does,
when nothing in the selection is left to move.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-25 00:14:00 -04:00
parent 709c9d0395
commit 82fa560cc8
3 changed files with 42 additions and 8 deletions

View File

@ -961,10 +961,16 @@ column.
which is possible but is inference, not recovery — deleting and replaying which is possible but is inference, not recovery — deleting and replaying
those log entries is the honest repair those log entries is the honest repair
- **Zero-row operations report success.** Scale refuses with "Nothing to - **Zero-row clones report success.** Scale refuses with "Nothing to scale…"
scale…" when its slice matches nothing; recode and clone commit an empty log and recode with "Nothing to recode…" when there is nothing to write (recode
entry and return `rows_affected: 0`. A recode of a rep whose rows are all also skips coordinates that already net to zero, rather than writing an empty
`reference` looked like it worked and did nothing pair for each); clone still commits an empty log entry and returns
`rows_affected: 0`
- **A date offset must be whole days.** `assertInterval()` refuses anything
finer, because every shifted date is cast back to a date — and Postgres reads
a bare number as *seconds*, so `"30"` used to be accepted and move nothing.
`30 days` is not a month either: a row on 1 October lands on the 31st; use
`1 month` to move a month into the next
- **The change log does not show an entry's id**, so there is no way to name - **The change log does not show an entry's id**, so there is no way to name
one when asking about it one when asking about it
- **Territory is read onto the session at login**, so granting or changing one - **Territory is read onto the session at login**, so granting or changing one

View File

@ -439,6 +439,11 @@ ilog AS (
{{exclude_clause}} {{exclude_clause}}
GROUP BY GROUP BY
${groupCols([...dims, ...dateCols])} ${groupCols([...dims, ...dateCols])}
-- a coordinate whose earlier adjustments already net it to zero has nothing to
-- move; writing it anyway is two empty rows per coordinate. Rare on a first
-- recode, but a second recode over cells a first one emptied would rewrite
-- every one of them
HAVING ${[effectiveValue, effectiveUnits].filter(Boolean).map(c => `sum(${q(c)}) <> 0`).join(' OR ')}
) )
,neg AS ( ,neg AS (
INSERT INTO {{fc_table}} (${insertCols}) INSERT INTO {{fc_table}} (${insertCols})

View File

@ -97,17 +97,30 @@ module.exports = function(pool) {
// Postgres to parse it alone first, where the failure is cheap and can name the // Postgres to parse it alone first, where the failure is cheap and can name the
// field it came from. Negative intervals are valid and useful -- '-90 days' // field it came from. Negative intervals are valid and useful -- '-90 days'
// pulls a plan back a quarter -- so this checks validity, not sign. // pulls a plan back a quarter -- so this checks validity, not sign.
// A date offset has to be a whole number of days. Every date it shifts is
// cast back to a date, so anything finer is rounded away -- and Postgres
// reads a bare number as seconds, so "30" meant thirty seconds, moved
// nothing, and a shifted recode took rows out of a cell and put them back
// into the same one.
async function assertInterval(value, res) { async function assertInterval(value, res) {
const example = `Try something like "30 days", "1 month", "-90 days" or "0 days".`;
let whole;
try { try {
await pool.query(`SELECT $1::interval`, [value]); whole = (await pool.query(
return true; `SELECT date_trunc('day', $1::interval) = $1::interval AS whole`, [value]
)).rows[0].whole;
} catch { } catch {
res.status(400).json({ error: `"${value}" is not a valid interval. ${example}` });
return false;
}
if (!whole) {
res.status(400).json({ res.status(400).json({
error: `"${value}" is not a valid interval. Try something like ` error: `"${value}" is not a whole number of days, so it would not move any date `
+ `"4 months", "1 year", "-90 days" or "0 days".` + `(a number with no unit is read as seconds). ${example}`
}); });
return false; return false;
} }
return true;
} }
// A slice is only meaningful if at least one of its keys is a filterable // A slice is only meaningful if at least one of its keys is a filterable
@ -977,6 +990,16 @@ module.exports = function(pool) {
} }
allRows.push(...result.rows); allRows.push(...result.rows);
} }
// Every coordinate in the selection nets to zero -- nothing to move.
// Refused like scale's no-op rather than committing an empty log
// entry that reads as a recode that happened.
if (allRows.length === 0) {
await client.query('ROLLBACK');
return res.status(400).json({
error: 'Nothing to recode — the selection nets to zero, or holds only rows recode cannot touch'
});
}
await client.query('COMMIT'); await client.query('COMMIT');
committed = true; committed = true;
// one log id per unit: apply_mode 'each' writes an entry per slice // one log id per unit: apply_mode 'each' writes an entry per slice