diff --git a/CLAUDE.md b/CLAUDE.md index e13fdea..607e990 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -556,6 +556,33 @@ Turning a region back into slices re-derives, per cell, the same filters Perspec `pf_iter` is not a col_meta column, so it is stripped when a slice is built: two cells differing only by iter band produce the same effective slice. Duplicates are collapsed before the request — without that, `apply_mode: each` would apply the same change twice. +**The pivot's own filter travels alongside, as `scope`.** A slice is +`{col: value}` and can only mean equality, so a view narrowed to +`sseas_e <= 2027` has no slice form at all — the filter is read off +`viewer.save()` and sent as `[col, op, value]` triples, which is also what the +ledger totals against, so the panel and the write agree. `SCOPE_OPS` in +`lib/sql_generator.js` is the whole allowed list, under **Perspective's** +operator names; anything outside it is refused rather than dropped, since a +scope silently discarded is a write wider than the panel that authorised it. + +The substring operators — `contains`, `begins with`, `ends with` and their +negations — map to **`ILIKE`**, not `LIKE`, because that is what the engine +does: `t_tscalar::contains` lowercases both sides before `std::string::find`. +On `partd`, `contains 'Keysto'` matches 16,864 rows case-insensitively and 35 +case-sensitively, so `LIKE` would have written a fortieth of what the grid +showed. Narrower is no safer than wider — either way it is not the set the +ledger counted. The term is literal in the engine, so `%`, `_` and `\` are +escaped out of it before it becomes a pattern. + +`matches` / `not matches` stay refused. They are RE2 in the engine, and +Postgres regexes are not RE2 — `\b` is a word boundary in one and a backspace +in the other — so a pattern accepted by both could quietly match a different +set of rows, which is worse than a 400. + +Every operator that needs a term refuses an empty one: an unfinished filter row +in the pivot arrives as `[col, '==', null]`, and `ILIKE '%%'` or `= ''` would +have read as "every row" or "no rows" rather than as the mistake it is. + **Limitation:** computed columns created by Perspective's split_by (e.g. Month, YearDate) don't map back to raw rows — only native dimension columns work for slice extraction. --- diff --git a/lib/sql_generator.js b/lib/sql_generator.js index 56d9817..1b6de06 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -626,19 +626,55 @@ function buildWhere(slice, dimCols, versionId) { // Perspective's operator names, not SQL's, since that is where these come from. // Anything outside this list is refused rather than ignored: a scope silently // dropped is a write that is wider than the panel that authorised it. +// +// The substring operators are the ones a text dimension is actually filtered +// with -- `partd contains 'Keysto'` picks a product family out of a description +// nobody would enumerate by hand -- and they are **case-insensitive** in the +// engine: `t_tscalar::contains` lowercases both sides before +// `std::string::find` (scalar.cpp), as do `begins_with` and `ends_with`. So the +// SQL is ILIKE, not LIKE. LIKE would be *narrower* than the grid that +// authorised the write, which is the failure this whole mechanism exists to +// prevent, and narrower is no safer than wider: it writes a set the ledger did +// not count either way. +// +// Their term is matched literally there, so LIKE's own metacharacters are +// escaped out of it by `likeTerm` -- a filter on `50%` means those three +// characters, not "50 followed by anything". +// +// Not identical in one corner: the engine folds case byte by byte, so ASCII +// only, where ILIKE follows the database collation. They differ only on +// non-ASCII letters. +// +// `matches` / `not matches` are deliberately still refused. They are RE2 in the +// engine and Postgres regexes are not RE2 -- `\b` is a word boundary in one and +// a backspace in the other -- so a pattern valid in both could quietly match a +// different set of rows, which is worse than answering 400. +const likeTerm = (val, pre, post) => + `'${esc(pre + String(val).replace(/[\\%_]/g, m => `\\${m}`) + post)}'`; + const SCOPE_OPS = { - '==': (c, v) => `${c} = ${v[0]}`, - '!=': (c, v) => `${c} != ${v[0]}`, - '>': (c, v) => `${c} > ${v[0]}`, - '>=': (c, v) => `${c} >= ${v[0]}`, - '<': (c, v) => `${c} < ${v[0]}`, - '<=': (c, v) => `${c} <= ${v[0]}`, - 'in': (c, v) => `${c} IN (${v.join(', ')})`, - 'not in': (c, v) => `${c} NOT IN (${v.join(', ')})`, - 'is null': (c) => `${c} IS NULL`, - 'is not null': (c) => `${c} IS NOT NULL`, + '==': (c, v) => `${c} = ${v[0]}`, + '!=': (c, v) => `${c} != ${v[0]}`, + '>': (c, v) => `${c} > ${v[0]}`, + '>=': (c, v) => `${c} >= ${v[0]}`, + '<': (c, v) => `${c} < ${v[0]}`, + '<=': (c, v) => `${c} <= ${v[0]}`, + 'in': (c, v) => `${c} IN (${v.join(', ')})`, + 'not in': (c, v) => `${c} NOT IN (${v.join(', ')})`, + 'is null': (c) => `${c} IS NULL`, + 'is not null': (c) => `${c} IS NOT NULL`, + 'contains': (c, v, raw) => `${c} ILIKE ${likeTerm(raw[0], '%', '%')}`, + 'not contains': (c, v, raw) => `${c} NOT ILIKE ${likeTerm(raw[0], '%', '%')}`, + 'begins with': (c, v, raw) => `${c} ILIKE ${likeTerm(raw[0], '', '%')}`, + 'not begins with': (c, v, raw) => `${c} NOT ILIKE ${likeTerm(raw[0], '', '%')}`, + 'ends with': (c, v, raw) => `${c} ILIKE ${likeTerm(raw[0], '%', '')}`, + 'not ends with': (c, v, raw) => `${c} NOT ILIKE ${likeTerm(raw[0], '%', '')}`, }; +// the operators that need a term, so an empty one is refused rather than +// becoming ILIKE '%%' -- which is every non-null row, i.e. no scope at all +const SCOPE_OPS_NEED_TERM = new Set(Object.keys(SCOPE_OPS).filter(op => !op.startsWith('is '))); + function buildScopeClause(scope, dimCols, versionId) { if (!Array.isArray(scope) || scope.length === 0) return ''; const allowed = new Set(dimCols); @@ -666,7 +702,11 @@ function buildScopeClause(scope, dimCols, versionId) { const err = new Error(`Unsupported filter operator "${op}" on ${col}`); err.status = 400; throw err; } - return fn(`"${col}"`, vals.map(v => `'${esc(String(v))}'`)); + if (SCOPE_OPS_NEED_TERM.has(op) && (vals.length === 0 || vals[0] === '' || vals[0] === null)) { + const err = new Error(`Filter "${col} ${op}" has no value`); + err.status = 400; throw err; + } + return fn(`"${col}"`, vals.map(v => `'${esc(String(v))}'`), vals); }); return parts.join('\nAND '); }