eql: detect the ORE-unavailable case at install time, and name a remedy that exists - #907
eql: detect the ORE-unavailable case at install time, and name a remedy that exists#907coderdan wants to merge 1 commit into
Conversation
The EQL bundle skips the ORE btree operator class when the installing role cannot create one, and poisons every `_ord_ore` domain with a loud-failure CHECK in its place. That is a supported configuration -- but nothing said so where the choice between `types.*Ord` and `types.*OrdOre` is actually made, so operators discovered the trade at query time. - `eql preflight` probes whether the role can create an operator class and reports it as a non-blocking row. Probed, not inferred from `superuser`: RDS and Aurora let their admin role create one while cloud-hosted Supabase does not, so `rolsuper` is not evidence either way. The probe attempts the DDL in a transaction it always rolls back, so preflight stays read-only, and reports `unknown` when it could not ask rather than guessing. - `eql install` names the consequence and the remedy on its own line. - `eql status` reports the ORE state, so the answer survives the install output. - The remedy now names a type that exists. The previous wording pointed at the `_ord_ope` domains; the bundle creates those, but `@cipherstash/stack` ships no `types.*OrdOpe` factory, so it named a column type no schema author could declare. - The state machine, the catalogue probe and the copy move into one module shared by preflight, install, status, verify and validate, so the five cannot drift into disagreeing about the same catalogue fact. Claude-Session: https://claude.ai/code/session_01AwM5Cm5ddasXozb6stxPR1
🦋 Changeset detectedLatest commit: 83264da The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
freshtonic
left a comment
There was a problem hiding this comment.
Approve. The design decision this PR turns on — probing CREATE OPERATOR FAMILY rather than inferring from rolsuper — is right, well-argued, and live-tested on both arms including the no-leftover assertion that keeps preflight's read-only claim honest. CI green.
What I verified:
- The shared ORE module does what it promises:
classifyOreStateis pure with all four states unit-tested, the opclassEXISTSexpression that previously existed byte-identical inverify.tsandvalidate.tsnow has one home, and thereadOreState-agrees-with-verifyEqlSurfacelive test is exactly the guard that makes a cheap status read safe to ship alongside the full diff. - The
_ord_opecorrection is real and pinned: the bundle does createeql_v3_<t>_ord_opedomains butcolumns.tsships noOrdOpefactory, so the old remedy pointed at an undeclarable type. The regression test sweeping every state's message for_ord_ope/OrdOpeis the right shape of guard. - The never-blocks framing is carried consistently —
missinguntouched, the preflight row annotates with<- skips:not<- blocks,unknownis a genuine third state that never renders as either answer, and the tests pin each of those separately. eql status's ORE check is advisory, not a gate — a failed read warns and the rest of status still reports. Correctly gated oninstalledV3, since an empty database legitimately reads asincoherent-unpoisonedthrough the raw classifier.
One non-blocking nit on a comment: probeOperatorClassCreate says "no CREATE on public" lands in the null arm, but Postgres raises 42501 for schema-permission denials too, so that case actually classifies as a hard false. It's harmless in practice — a role without CREATE on public is already reported as a preflight blocker, and the bundle's own CREATE OPERATOR CLASS would fail identically for that role, so "not creatable" is the truthful answer for what install will experience — but the comment (and the PR description's list of unknown causes) slightly overstates what reaches null. Worth a one-line fix whenever the file is next touched.
Fixes #891. Stacked on #906 — review that first; this PR's diff is the last commit only.
The gap
The EQL bundle wraps
CREATE OPERATOR CLASSfor the ORE opclass in a guardedDOblock that swallowsinsufficient_privilege. Where the installing role cannot clear that gate the class is skipped and the bundle poisons every_ord_ore/_search_oredomain with an always-raisingeql_ore_unavailableCHECK instead.That is a supported configuration. The problem was when you found out: nothing said it where the choice between
types.*Ordandtypes.*OrdOreis actually made. #906 added the detection, but only as a parenthetical on one line of install output. The operator in the report learned about it when something on the ORE path errored at query time.What this adds
eql preflightpredicts it. A new non-blockingORE operator classrow —creatable/not creatable/unknown,canCreateOperatorClassin--json.It is probed, not inferred from
superuser, and that is the load-bearing decision here.CREATE OPERATOR CLASSis superuser-gated in stock PostgreSQL, but AWS RDS and Aurora let their admin role create one (rolsuper = f) while cloud-hosted Supabase does not. Predicting fromrolsuperwould tell an RDS operator their ORE domains are unavailable when they work — exactly the blanket claim about "managed Postgres" this change exists to stop making. So preflight asks the server:CREATE OPERATOR FAMILYshares the privilege gate withCREATE OPERATOR CLASSand needs no member operators, so it is the cheapest statement that tests it, and it runs inside a transaction that is always rolled back.42501is a real "no"; anything else (read-only replica, noCREATEonpublic, timeout) isunknown— never rendered as either answer.eql installnames the consequence and the remedy on its own line rather than in parentheses.eql statusreports the ORE state on a v3 install, so the answer survives past the install output without re-reading scrollback.A correction: the old remedy named a type that does not exist
The fallback message pointed at "the
_ord_opeordering domains". The bundle does createpublic.eql_v3_<t>_ord_ope— but@cipherstash/stackships notypes.*OrdOpefactory (packages/stack/src/eql/v3/columns.ts:165— out of scope), so that sentence sent a schema author to a column type they could not declare.Every command now says
types.*Ord(public.eql_v3_*_ord): the same CLLW-OPE ordering, with a factory behind it.ore.test.tspins that no state's message mentions_ord_opeorOrdOpeagain.One model, five commands
The state machine, the catalogue probe and the copy move into
packages/cli/src/installer/ore.ts, shared byeql preflight,eql install,eql status,eql verifyandeql validate. The opclassEXISTSexpression previously existed twice, byte-identical, inverify.tsandvalidate.ts; the two commands can no longer drift into disagreeing about the same catalogue fact.On the issue's premises
Two things in #891 are out of date against the pinned bundle, and the implementation follows the bundle:
_ord_oredomains remain present and usable as column types" (citingstash-encryption: the_ord_oredomains are not disabled on managed Postgres — only the opclass is skipped #759, verified on EQL 3.0.2) is no longer true on 3.0.4, which is what the CLI pins. The bundle now adds theeql_ore_unavailablepoison CHECK to all 20 ORE domains on the fallback path (cipherstash-encrypt.sql:59368-59425), so a write to one fails. The messaging added here says "unusable, writes fail a CHECK", not "usable but unindexable". Worth closingstash-encryption: the_ord_oredomains are not disabled on managed Postgres — only the opclass is skipped #759 against the current bundle rather than applying its suggested wording.stash eql installshould pick the OPE domains automatically in Supabase mode" — it already does, on every platform.V3Domain(init/types.ts) has no*OrdOremember and the interactive picker never offers one, so no scaffold path can emit the trap. What was missing was the reason, so the agent editing the user's real schema files afterwards does not reach for it: both placeholder templates now carry it.Coverage
installer/__tests__/ore.test.ts(new): all four state classifications, both healthy/damage severities, the_ord_operegression guard, the three-statedescribeOreCreatable, and that the opclass expression usesto_regtype(degrades on a database with no EQL) rather than a raising::regtypecast.db/__tests__/preflight.test.ts: the row never annotates as a blocker, andunknownnever renders asno.preflight.live.test.ts: the probe answerstruefor the superuser andfalsefor an unprivileged role (the 42501 arm is an answer, not a failure), andpg_opfamilyholds no leftover afterwards — the assertion that preflight is still read-only.verify.live.test.ts: the standalonereadOreStateagrees with the full surface diff on the same database, so the cheap readeql statususes cannot disagree witheql verify.pnpm --filter stash testgreen, including the live suites against the compose database.https://claude.ai/code/session_01AwM5Cm5ddasXozb6stxPR1