fix(mapper): make the late native fallback for unresolved value nodes fail closed (CIP-3715)

[freshtonic]

Closes CIP-3715.

Important

Stacked on #439 (CIP-3700, base branch james/cip-3700-eql-mapper-inference-hardening-unconstrained-limitfetch). It must land after #439, or be retargeted to main once #439 merges. The dependency is real: without #439's LIMIT/OFFSET/FETCH Native pins, removing the fallback would break LIMIT $1 and friends.

The problem

Unifier::resolve_unresolved_value_nodes resolved any ast::Value node still untyped after inference to Native — fail-open one layer below the CIP-3699/CIP-3700 inference gaps. A literal or param in a clause that inference never visited was silently typed Native and could skip encryption; a future missed AST field over an encrypted column would sail through the same way. Its Result was also discarded (let _ =) at the call site in EqlMapper::resolve, so even unification failures there were swallowed.

The audit

Instrumented the fallback and ran the full mapper suite. Every shape that reached it, and what owns it now:

Shape reaching the fallback	Now typed by
WHERE true, WHERE $1	Native pin on Select::selection (boolean contexts are always native)
HAVING <literal>	Native pin on Select::having
JOIN … ON true (incl. ASOF match conditions)	Native pin on join ON constraints
ORDER BY 1, GROUP BY 1 ordinals (incl. under DISTINCT, and after set operations where no single projection resolves them)	Native pin on literal sort/group keys — the literal reaches the database as a plain constant regardless of which projected column the ordinal selects; the redirected Ord/Eq bound on the projected column is unchanged
SELECT 'lit', SELECT $1, CASE results in a projection, SELECT ARRAY[1,2,3], SELECT 1 inside EXISTS, unreferenced derived-table columns	Still defaulted to Native at resolve time, but the defaulting is now scoped: only type variables reachable from a Query/Statement node's type (i.e. from some projection) qualify. These values relate to nothing — they flow to the client or are discarded — so they cannot be EQL, which is what makes the default sound.

Fail closed

Anything else unresolved at resolve time is now TypeError::UnresolvedValue, naming the value, and the error propagates out of EqlMapper::resolve instead of being swallowed.

Shapes verified (by probe) to now fail closed instead of silently passing — left for CIP-3699, which owns these inference gaps; do not expect them to type-check until it lands:

• window frame bounds (ROWS BETWEEN 1 PRECEDING AND CURRENT ROW)
• aggregate FILTER (WHERE …) clauses

(ON CONFLICT — also CIP-3699's — already errors earlier, on scope resolution.)

One behavioural edge outside the test suite: DISTINCT ON (<literal>) previously slipped through via the fallback and now errors. PostgreSQL has no ordinal resolution there, so nothing meaningful is lost; flagging it in case CIP-3699 wants to own it alongside the other clause pins.

Changes

• packages/eql-mapper/src/inference/unifier/mod.rs — fallback replaced with projection-reachability-scoped defaulting + fail-closed error; unit test for the fail-closed path.
• packages/eql-mapper/src/inference/infer_type_impls/select.rs — Native pins for WHERE/HAVING/join ON and GROUP BY literal keys.
• packages/eql-mapper/src/inference/infer_type_impls/query_statement.rs — Native pin for ORDER BY literal keys.
• packages/eql-mapper/src/inference/type_error.rs — new UnresolvedValue variant.
• packages/eql-mapper/src/eql_mapper.rs — propagate the error instead of let _ =.
• packages/eql-mapper/src/lib.rs — six new tests: WHERE-placeholder-is-native, HAVING/ON constant conditions, encrypted column as bare WHERE condition rejected, UNION ALL … ORDER BY 1, unreferenced derived-table literal column.
• CHANGELOG.md — Fixed entry under Unreleased.

As a side effect of the WHERE pin, WHERE enc_col (a bare encrypted column as a boolean condition) is now rejected by the mapper instead of being forwarded for PostgreSQL to reject.

Verification

• mise run check — clean (fmt, clippy, compile).
• cargo test -p eql-mapper — 146 passed (139 pre-existing + 7 new), 0 failed.
• cargo test -p cipherstash-proxy -- --test-threads=1 — 121 passed; the 3 doc-test failures are pre-existing on clean main.
• Integration tests not run: this is a mapper-internal change with no wire-protocol or encryption-path surface; the mapper suite is the authority on statement typing.

Summary by CodeRabbit

• Bug Fixes

  • Improved query type validation to prevent untyped values from being accepted unexpectedly.
  • Added clear errors for values that cannot be safely resolved.
  • Fixed handling of literals and parameters in comparisons, filters, joins, grouping, ordering, and aggregate conditions.
  • Bare encrypted boolean conditions are now rejected where native values are required.

• Tests

  • Added coverage for literal-only predicates, shared parameters, joins, ordering, grouping, and derived queries.

[coderabbitai]

Warning

Review limit reached

@freshtonic, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 19 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e0134cf9-4568-4dfe-aa7d-8cbbf9281cdd

📥 Commits

Reviewing files that changed from the base of the PR and between ecc6b0e and a80d8c8.

📒 Files selected for processing (13)

• CHANGELOG.md
• packages/eql-mapper/src/eql_mapper.rs
• packages/eql-mapper/src/inference/infer_type_impls/expr.rs
• packages/eql-mapper/src/inference/infer_type_impls/query_statement.rs
• packages/eql-mapper/src/inference/infer_type_impls/select.rs
• packages/eql-mapper/src/inference/type_error.rs
• packages/eql-mapper/src/inference/unifier/mod.rs
• packages/eql-mapper/src/lib.rs
• packages/eql-mapper/src/transformation_rules/helpers.rs
• packages/eql-mapper/src/transformation_rules/mod.rs
• packages/eql-mapper/src/transformation_rules/rewrite_eql_any_all_ops.rs
• packages/eql-mapper/src/transformation_rules/rewrite_eql_comparison_ops.rs
• packages/eql-mapper/src/type_checked_statement.rs

📝 Walkthrough

Walkthrough

The EQL mapper now applies native typing to predicates and ordinal expressions, defers comparison operand grounding, and rejects unresolved values outside projections. Resolution errors propagate to callers. Regression tests cover literals, parameters, joins, ordering, projections, and encrypted boolean conditions.

Changes

EQL type inference

Layer / File(s)	Summary
Native constraints for expressions packages/eql-mapper/src/inference/type_error.rs, packages/eql-mapper/src/inference/infer_type_impls/expr.rs, packages/eql-mapper/src/inference/infer_type_impls/select.rs, packages/eql-mapper/src/inference/infer_type_impls/query_statement.rs	Comparison operands can be natively groundable when unresolved. WHERE, HAVING, join ON, AS OF, and ordinal GROUP BY/ORDER BY expressions receive native constraints.
Fail-closed unresolved value resolution packages/eql-mapper/src/inference/unifier/mod.rs, packages/eql-mapper/src/eql_mapper.rs	The unifier tracks safely defaultable variables and projection-reachable variables. Other unresolved values return TypeError::UnresolvedValue, and resolve propagates the error.
Regression coverage and release documentation packages/eql-mapper/src/inference/unifier/mod.rs, packages/eql-mapper/src/lib.rs, CHANGELOG.md	Tests cover literal predicates, shared parameters, boolean conditions, joins, encrypted conditions, ordinal ordering, and derived-table projections. The changelog documents the stricter behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• cipherstash/proxy#415: Both modify EQL mapper type checking for unresolved values and encrypted literals.
• cipherstash/proxy#432: This change extends related EQL v3 inference and unifier handling.

Suggested reviewers: tobyhede

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the fail-closed fix for unresolved value nodes and matches the main changes in the pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch james/cip-3715-unifiers-late-native-fallback-for-unresolved-value-nodes-is

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[freshtonic]

Comment-only review.\n\nI did not find a defect in the mapper changes themselves. The projection-reachability defaulting is scoped conservatively, the previously implicit boolean/ordinal cases are now constrained explicitly, and the new error is propagated.\n\nOne current-head issue remains: all four PostgreSQL CI jobs are failing in with a error (the server expects , while tokio-postgres is given a Rust ). This appears inherited from the stacked base rather than introduced by this diff, but the stack needs to be updated or the failure otherwise resolved before merge.

[freshtonic]

Correction to my preceding review (shell formatting stripped identifiers): the four failing jobs fail in update_with_param_reused_for_storage_and_query with WrongType; PostgreSQL expects eql_v3_text_search while tokio-postgres is given a Rust String.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/eql-mapper/src/inference/infer_type_impls/expr.rs`:
- Around line 238-251: Extend the native-default eligibility marking beyond the
generic BinaryOp branch to every comparison path in the expression inference
implementation, including Expr::InList, Expr::Between, Expr::IsDistinctFrom,
Expr::AnyOp/Expr::AllOp, and simple CASE comparisons. Reuse
mark_natively_groundable for each operand type unified by Eq or Ord constraints,
preserving encrypted-type grounding behavior, and add regression tests covering
the listed literal comparison forms.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 731c150d-aeca-4b8e-82df-eb1d44a04876

📥 Commits

Reviewing files that changed from the base of the PR and between e2198c7 and 5b43968.

📒 Files selected for processing (8)

• CHANGELOG.md
• packages/eql-mapper/src/eql_mapper.rs
• packages/eql-mapper/src/inference/infer_type_impls/expr.rs
• packages/eql-mapper/src/inference/infer_type_impls/query_statement.rs
• packages/eql-mapper/src/inference/infer_type_impls/select.rs
• packages/eql-mapper/src/inference/type_error.rs
• packages/eql-mapper/src/inference/unifier/mod.rs
• packages/eql-mapper/src/lib.rs