diff --git a/CHANGELOG.md b/CHANGELOG.md index b439aa6e2..b9098d5dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,8 @@ Each entry that ships in a published release links to the PR that introduced it. - **The `eql_v2` schema and its entire surface are removed — EQL now ships only the self-contained `eql_v3` encrypted-domain surface.** Dropped with no `eql_v3` replacement: the `eql_v2_encrypted` composite column type and its operators (`=`, `<>`, `~~`/`~~*` `LIKE`/`ILIKE`, containment `@>`/`<@`, ORE comparisons), database-side configuration management (`eql_v2_configuration`, `add_search_config`, `add_column`, `migrate_config`, `diff_config`, `create_encrypted_columns`), the `encryptindex` migration machinery, boolean operators on the encrypted column type, operator-class-on-column indexing, and `GROUP BY` / `grouped_value` on the encrypted column type. Searchable-encryption capabilities (equality, ordered range, `MIN`/`MAX`, encrypted-JSONB document containment and path access) are all provided by the `eql_v3` encrypted-domain families and document surface. Why: `eql_v3` is now fully self-contained — it owns its own SEM index-term types (`eql_v3.hmac_256`, `eql_v3.ore_block_256`, `eql_v3.ore_cllw`, `eql_v3.bloom_filter`) and installs into a database with no `eql_v2` present — and the encryption client (CipherStash Proxy / ProtectJS) now owns the configuration model the database-side `eql_v2` functions previously provided. This is a major (3.0.0) public-API break; callers using the `eql_v2` schema migrate to the `eql_v3` encrypted-domain types. The user-facing reference documentation and tutorials have been migrated to teach only the `eql_v3` surface — the `eql_v2`-based examples, the database-side configuration guide, and the operator-class-on-column indexing recipe are removed. No per-capability migration guide is provided for the dropped capabilities (they have no `eql_v3` equivalent). ### Fixed +- **`ore_block_256` opclass-path helpers converted from `LANGUAGE sql` to plpgsql — restores v2-level ordered-scan performance.** `eql_v3_internal.jsonb_array_to_bytea_array` and `eql_v3_internal.jsonb_array_to_ore_block_256` were `LANGUAGE sql` for inlineability, but their only caller chain (`ore_block_256(val)`, plpgsql, feeding the btree operator class) can never inline SQL functions — every compared value paid the per-call SQL-function executor instead, measured at 3.5× the per-call cost of the logic-identical plpgsql form. Release benchmarks put the end-to-end cost at +43% on ORE ordered index scans vs EQL 2.3 (`0.513 → 0.736 ms` at 1M rows) and +36% on the composite bloom+ORE-order shape (`16.6 → 22.7 ms`); with the plpgsql form both scenarios return to (or beat) the v2 numbers — `0.553 ms` and `13.97 ms` respectively, validated A→B→A on a live 1M-row bench database. Semantics are unchanged: NULL/non-array inputs still return NULL, and the empty-`ob` COALESCE (#262) is preserved. The `eql-inline-critical` markers are retained so the pin_search_path pass keeps both functions unpinned — a `SET search_path` clause on plpgsql forces per-call configuration switching in the same hot path. Full attribution and experiment data: cipherstash/benches#23 (`v3-regressions-report.md`). ([#353](https://github.com/cipherstash/encrypt-query-language/issues/353)) + - **`=` / `<>` on `eql_v2.ore_block_u64_8_256` now declare a `COMMUTATOR`, so equality joins over the ORE term no longer raise.** Both operators set `COMMUTATOR` to themselves (equality and inequality are symmetric, so each is its own commutator). Why: without it the planner raised `could not find commutator for operator` the first time an `ore_block_u64_8_256` equality was used as a join / mergejoin qualifier (e.g. via the inlined `eql_v3.integer_ord_ore` equality wrappers, since the operators carry `MERGES`). This only enables previously-erroring join plans — it cannot change which rows match or their ordering. ([#239](https://github.com/cipherstash/encrypt-query-language/pull/239)) - **The `eql_v3` ORE block comparator now orders ciphertexts of any block count, not just 8.** `eql_v3.compare_ore_block_256_term` derives the block count `N` from the term length (`octet_length = 49·N + 16`) instead of hardcoding 8, so encrypted types whose native ORE width exceeds 8 blocks — `numeric` (14) and `timestamp` (12) — order, range-query, `ORDER BY`, and `MIN`/`MAX` correctly instead of silently mis-ordering. Malformed terms (length not `49·N + 16` for `N ≥ 1`) now raise instead of returning a bogus comparison. The self-contained `eql_v3` SEM type was renamed `eql_v3.ore_block_u64_8_256 → eql_v3.ore_block_256` to reflect that it is width-agnostic (the `eql_v2` type is unchanged). No effect on existing 8-block types (a no-op for `N = 8`). ([#241](https://github.com/cipherstash/encrypt-query-language/issues/241), [#276](https://github.com/cipherstash/encrypt-query-language/pull/276)) diff --git a/src/v3/common.sql b/src/v3/common.sql index 8fe4da2ff..a81ea3724 100644 --- a/src/v3/common.sql +++ b/src/v3/common.sql @@ -18,32 +18,40 @@ --! --! @note Returns NULL if input is JSON null --! @note Each array element is hex-decoded to bytea ---! @note Inlinable `LANGUAGE sql` IMMUTABLE form (no `SET search_path`) so the ---! planner can fold this per-encrypted-value helper into the calling query. ---! This deliberately diverges from the v2 plpgsql equivalent (intentionally ---! left unchanged): the `CASE WHEN jsonb_typeof(val) = 'array'` guard only ---! evaluates the set-returning `jsonb_array_elements_text` for an array, so a ---! non-array JSON scalar returns NULL here instead of raising "cannot extract ---! elements from a scalar". Both callers only ever pass an array or JSON null ---! (`val->'ob'`), so the divergence is unreachable in practice; JSON null and ---! empty array still return NULL exactly as before. +--! @note plpgsql, not `LANGUAGE sql` (issue #353). This helper's ONLY caller +--! chain is `ore_block_256(val)` -> `jsonb_array_to_ore_block_256(val)` — +--! both reached exclusively from plpgsql and btree operator-class support +--! contexts, where SQL functions can NEVER be inlined and instead pay the +--! per-call SQL-function executor (measured 3.5x the per-call cost of the +--! plpgsql equivalent; +43% on ORE ordered scans end-to-end). plpgsql +--! caches its plan across calls. The non-array guard preserves the v3 +--! behaviour (returns NULL for a non-array scalar; the v2 plpgsql original +--! raised) — both callers only ever pass an array or JSON null (`val->'ob'`), +--! so the divergence stays unreachable in practice; JSON null and empty +--! array still return NULL exactly as before. CREATE FUNCTION eql_v3_internal.jsonb_array_to_bytea_array(val jsonb) RETURNS bytea[] IMMUTABLE AS $$ - SELECT CASE WHEN jsonb_typeof(val) = 'array' - THEN ( - SELECT array_agg(decode(value::text, 'hex')::bytea) - FROM jsonb_array_elements_text(val) AS value - ) - ELSE NULL - END; -$$ LANGUAGE sql; +DECLARE + result bytea[]; +BEGIN + IF val IS NULL OR jsonb_typeof(val) != 'array' THEN + RETURN NULL; + END IF; + SELECT array_agg(decode(value::text, 'hex')::bytea) + INTO result + FROM jsonb_array_elements_text(val) AS value; + RETURN result; +END; +$$ LANGUAGE plpgsql; ---! @internal Mark this hand-written helper inline-critical so the post-install ---! pin_search_path pass leaves it unpinned (no `SET search_path`), preserving ---! SQL-function inlining. It takes a bare `jsonb` arg (not a jsonb-backed ---! encrypted DOMAIN), so the structural skip in tasks/pin_search_path_v3.sql does ---! not recognise it; this marker is the documented manual opt-in. +--! @internal Keep the inline-critical marker so the post-install +--! pin_search_path pass leaves this unpinned: a `SET search_path` clause on a +--! plpgsql function forces per-call configuration switching — measurable on a +--! helper invoked per compared value in the ore_block_256 opclass hot path. +--! It takes a bare `jsonb` arg (not a jsonb-backed encrypted DOMAIN), so the +--! structural skip in tasks/pin_search_path_v3.sql does not recognise it; +--! this marker is the documented manual opt-in. COMMENT ON FUNCTION eql_v3_internal.jsonb_array_to_bytea_array(jsonb) IS - 'eql-inline-critical: per-encrypted-value ORE helper; must stay inlinable (unpinned search_path)'; + 'eql-inline-critical: per-encrypted-value ORE opclass-path helper; must stay unpinned (SET search_path adds per-call overhead)'; diff --git a/src/v3/sem/ore_block_256/functions.sql b/src/v3/sem/ore_block_256/functions.sql index e4098ecc5..45f31fdce 100644 --- a/src/v3/sem/ore_block_256/functions.sql +++ b/src/v3/sem/ore_block_256/functions.sql @@ -17,14 +17,15 @@ --! @internal --! @param val jsonb Array of hex-encoded ORE block terms --! @return eql_v3_internal.ore_block_256 ORE block composite, or NULL if input is null ---! @note Inlinable `LANGUAGE sql` IMMUTABLE form (no `SET search_path`) so the ---! planner can fold this per-encrypted-value helper into the calling query. ---! This deliberately diverges from the v2 plpgsql equivalent (intentionally ---! left unchanged): the `CASE WHEN jsonb_typeof(val) = 'array'` guard only ---! evaluates the array path for an array, so a non-array JSON scalar returns ---! NULL here instead of raising. The sole caller (`ore_block_256`) only reaches ---! this when `has_ore_block_256(val)` is true, which now requires `val->'ob'` ---! to be a JSON array, so the non-array branch is unreachable in practice. +--! @note plpgsql, not `LANGUAGE sql` (issue #353). The sole caller +--! (`ore_block_256`) is itself plpgsql, so this function is NEVER reached +--! from an inlinable context — as `LANGUAGE sql` it paid the per-call +--! SQL-function executor on every compared value in the opclass hot path +--! (measured: +43% on ORE ordered scans vs the plpgsql form). The +--! non-array guard preserves the v3 behaviour (returns NULL for a +--! non-array scalar; the v2 plpgsql original raised); the caller only +--! reaches this when `has_ore_block_256(val)` is true, which requires +--! `val->'ob'` to be a JSON array, so that branch stays unreachable. --! An empty array (`ob: []`, what encrypting the empty string `""` produces) --! yields a non-NULL composite with an EMPTY `terms` array — NOT NULL terms. --! The `COALESCE` is load-bearing: `array_agg` over zero rows returns NULL, and @@ -36,25 +37,33 @@ CREATE FUNCTION eql_v3_internal.jsonb_array_to_ore_block_256(val jsonb) RETURNS eql_v3_internal.ore_block_256 IMMUTABLE AS $$ - SELECT CASE WHEN jsonb_typeof(val) = 'array' - THEN ROW(COALESCE( - ( - SELECT array_agg(ROW(b)::eql_v3_internal.ore_block_256_term) - FROM unnest(eql_v3_internal.jsonb_array_to_bytea_array(val)) AS b - ), - ARRAY[]::eql_v3_internal.ore_block_256_term[] - ))::eql_v3_internal.ore_block_256 - ELSE NULL - END; -$$ LANGUAGE sql; +DECLARE + terms eql_v3_internal.ore_block_256_term[]; +BEGIN + IF val IS NULL OR jsonb_typeof(val) != 'array' THEN + RETURN NULL; + END IF; + SELECT array_agg(ROW(b)::eql_v3_internal.ore_block_256_term) + INTO terms + FROM unnest(eql_v3_internal.jsonb_array_to_bytea_array(val)) AS b; + -- plpgsql pitfall: `SELECT INTO ` assigns the + -- select-list columns FIELD-WISE into the variable — return the row + -- constructor directly instead. The COALESCE stays load-bearing for the + -- empty-`ob` case (issue #262): array_agg over zero rows yields NULL, and + -- the comparator needs an EMPTY terms array, not NULL terms. + RETURN ROW(COALESCE(terms, ARRAY[]::eql_v3_internal.ore_block_256_term[]))::eql_v3_internal.ore_block_256; +END; +$$ LANGUAGE plpgsql; ---! @internal Mark this hand-written helper inline-critical so the post-install ---! pin_search_path pass leaves it unpinned (no `SET search_path`), preserving ---! SQL-function inlining. It takes a bare `jsonb` arg (not a jsonb-backed ---! encrypted DOMAIN), so the structural skip in tasks/pin_search_path_v3.sql does ---! not recognise it; this marker is the documented manual opt-in. +--! @internal Keep the inline-critical marker so the post-install +--! pin_search_path pass leaves this unpinned: a `SET search_path` clause on a +--! plpgsql function forces per-call configuration switching — measurable on a +--! helper invoked per compared value in the ore_block_256 opclass hot path. +--! It takes a bare `jsonb` arg (not a jsonb-backed encrypted DOMAIN), so the +--! structural skip in tasks/pin_search_path_v3.sql does not recognise it; +--! this marker is the documented manual opt-in. COMMENT ON FUNCTION eql_v3_internal.jsonb_array_to_ore_block_256(jsonb) IS - 'eql-inline-critical: per-encrypted-value ORE helper; must stay inlinable (unpinned search_path)'; + 'eql-inline-critical: per-encrypted-value ORE opclass-path helper; must stay unpinned (SET search_path adds per-call overhead)'; --! @brief Extract ORE block index term from JSONB payload diff --git a/tests/sqlx/tests/encrypted_domain/family/inlinability.rs b/tests/sqlx/tests/encrypted_domain/family/inlinability.rs index 2a9480edc..142c3a98e 100644 --- a/tests/sqlx/tests/encrypted_domain/family/inlinability.rs +++ b/tests/sqlx/tests/encrypted_domain/family/inlinability.rs @@ -96,23 +96,17 @@ async fn no_encrypted_domain_inline_critical_function_is_pinned(pool: PgPool) -> /// regresses to Seq Scan — this test fails instead. /// /// `jsonb_array_to_bytea_array(jsonb)` and -/// `jsonb_array_to_ore_block_256(jsonb)` are included here: both take a -/// bare `jsonb` arg (not a jsonb-backed encrypted DOMAIN), so the structural -/// skip in tasks/pin_search_path_v3.sql does not recognise them — they are kept -/// unpinned by the `eql-inline-critical` COMMENT marker instead. This test -/// asserts the unpinned + inlinable-SQL state directly; the companion -/// `eql_v3_sem_inline_critical_functions_carry_marker` test below asserts the -/// marker itself, so an edit that drops the marker (or a pin_search_path_v3.sql -/// refactor that stops honouring it) fails CI even though both checks live in -/// separate tests. +/// `jsonb_array_to_ore_block_256(jsonb)` are NOT in this test's inlinable-SQL +/// set: their only caller chain (`ore_block_256(val)`, plpgsql, feeding the +/// btree operator class) can never inline a SQL function, so they are plpgsql +/// by design (issue #353) and guarded by the dedicated +/// `eql_v3_ore_block_256_opclass_helpers_are_plpgsql_and_unpinned` test below. #[sqlx::test] async fn eql_v3_sem_inline_critical_functions_are_unpinned(pool: PgPool) -> Result<()> { let rows: Vec<(String,)> = sqlx::query_as( r#" WITH expected(proname, pronargs, arg0, arg1) AS ( VALUES - ('jsonb_array_to_bytea_array', 1, 'jsonb'::regtype, 0::oid), - ('jsonb_array_to_ore_block_256', 1, 'jsonb'::regtype, 0::oid), ('ore_cllw', 1, 'jsonb'::regtype, 0::oid), ('has_ore_cllw', 1, 'jsonb'::regtype, 0::oid), ('meta_data', 1, 'jsonb'::regtype, 0::oid), @@ -145,9 +139,7 @@ async fn eql_v3_sem_inline_critical_functions_are_unpinned(pool: PgPool) -> Resu 'hmac_256', 'bloom_filter', 'ore_cllw', - 'has_ore_cllw', - 'jsonb_array_to_bytea_array', - 'jsonb_array_to_ore_block_256') + 'has_ore_cllw') AND p.proargtypes[0] = 'jsonb'::regtype) OR e.proname IS NOT NULL ) @@ -171,17 +163,17 @@ async fn eql_v3_sem_inline_critical_functions_are_unpinned(pool: PgPool) -> Resu Ok(()) } -/// Companion guard for the two bare-`jsonb` per-encrypted-value helpers -/// (`jsonb_array_to_bytea_array`, `jsonb_array_to_ore_block_256`). The -/// unpinned state asserted above is only DURABLE because each helper carries an -/// `eql-inline-critical` COMMENT marker that `tasks/pin_search_path_v3.sql` honours -/// (it skips pinning functions whose `pg_description` matches -/// `'eql-inline-critical%'`). Neither helper is caught by the structural -/// jsonb-domain skip, so the marker is the ONLY thing keeping them unpinned — -/// an edit that removes the marker, or a pin_search_path_v3.sql refactor that drops -/// the marker handling, would silently re-pin them and break inlining. This test -/// asserts the marker is present (and the helpers are SQL/IMMUTABLE) so that -/// failure surfaces here. +/// Companion guard: the unpinned state asserted above is only DURABLE for +/// bare-`jsonb` helpers because each carries an `eql-inline-critical` COMMENT +/// marker that `tasks/pin_search_path_v3.sql` honours (it skips pinning +/// functions whose `pg_description` matches `'eql-inline-critical%'`). These +/// helpers are not caught by the structural jsonb-domain skip, so the marker +/// is the ONLY thing keeping them unpinned — an edit that removes the marker, +/// or a pin_search_path_v3.sql refactor that drops the marker handling, would +/// silently re-pin them and break inlining. This test asserts the marker is +/// present (and the helpers are SQL/IMMUTABLE). The two plpgsql +/// `jsonb_array_to_*` opclass-path helpers are guarded separately below +/// (issue #353). #[sqlx::test] async fn eql_v3_sem_inline_critical_helpers_carry_marker(pool: PgPool) -> Result<()> { // Each expected helper must appear with a present inline-critical marker @@ -191,8 +183,6 @@ async fn eql_v3_sem_inline_critical_helpers_carry_marker(pool: PgPool) -> Result r#" WITH expected(proname, pronargs, arg0, arg1) AS ( VALUES - ('jsonb_array_to_bytea_array', 1, 'jsonb'::regtype, 0::oid), - ('jsonb_array_to_ore_block_256', 1, 'jsonb'::regtype, 0::oid), ('ore_cllw', 1, 'jsonb'::regtype, 0::oid), ('has_ore_cllw', 1, 'jsonb'::regtype, 0::oid), ('meta_data', 1, 'jsonb'::regtype, 0::oid), @@ -238,6 +228,73 @@ async fn eql_v3_sem_inline_critical_helpers_carry_marker(pool: PgPool) -> Result Ok(()) } +/// Dedicated guard for the two `ore_block_256` opclass-path helpers +/// (`jsonb_array_to_bytea_array`, `jsonb_array_to_ore_block_256`) — issue #353. +/// +/// Their only caller chain is `ore_block_256(val)` (plpgsql) feeding the btree +/// operator class: neither plpgsql callers nor opclass support contexts can +/// EVER inline a SQL function, so as `LANGUAGE sql` these paid the per-call +/// SQL-function executor on every compared value (measured 3.5x the plpgsql +/// per-call cost; +43% end-to-end on ORE ordered index scans — see +/// cipherstash/benches#23). They are therefore plpgsql BY DESIGN, and must +/// stay: (a) plpgsql — a revert to LANGUAGE sql reintroduces the regression; +/// (b) IMMUTABLE; (c) UNPINNED, via the `eql-inline-critical` marker — a +/// `SET search_path` clause on plpgsql forces per-call configuration +/// switching in the same hot path. +#[sqlx::test] +async fn eql_v3_ore_block_256_opclass_helpers_are_plpgsql_and_unpinned(pool: PgPool) -> Result<()> { + let offenders: Vec<( + String, + Option, + Option, + Option, + Option, + )> = sqlx::query_as( + r#" + WITH expected(proname) AS ( + VALUES ('jsonb_array_to_bytea_array'), ('jsonb_array_to_ore_block_256') + ) + SELECT e.proname, + l.lanname::text, + p.provolatile::text, + d.description, + EXISTS ( + SELECT 1 FROM unnest(coalesce(p.proconfig, '{}'::text[])) c + WHERE c LIKE 'search_path=%' + ) AS pinned + FROM expected e + LEFT JOIN pg_catalog.pg_proc p + ON p.proname = e.proname + AND p.pronamespace = 'eql_v3_internal'::regnamespace + AND p.pronargs = 1 + AND p.proargtypes[0] = 'jsonb'::regtype + LEFT JOIN pg_catalog.pg_language l ON l.oid = p.prolang + LEFT JOIN pg_catalog.pg_description d + ON d.objoid = p.oid AND d.classoid = 'pg_proc'::regclass + WHERE p.oid IS NULL + OR l.lanname IS DISTINCT FROM 'plpgsql' + OR p.provolatile IS DISTINCT FROM 'i' + OR d.description IS NULL + OR d.description NOT LIKE 'eql-inline-critical%' + OR EXISTS ( + SELECT 1 FROM unnest(coalesce(p.proconfig, '{}'::text[])) c + WHERE c LIKE 'search_path=%' + ) + ORDER BY e.proname + "#, + ) + .fetch_all(&pool) + .await?; + + assert!( + offenders.is_empty(), + "ore_block_256 opclass-path helpers must be plpgsql + IMMUTABLE + \ + marker-unpinned (issue #353) — offenders (proname, lang, volatility, \ + marker, pinned): {offenders:#?}" + ); + Ok(()) +} + #[sqlx::test] async fn every_inline_critical_eligible_domain_has_inline_critical_functions( pool: PgPool,