feat: add eql_v2.lints() inlinability lint (#194)#195
Conversation
Adds a Splinter-shaped lint that reports operator implementation functions
which the Postgres planner cannot inline. Non-inlinable functions in
operator implementations silently degrade encrypted-query performance:
the documented functional indexes (hmac_256 / bloom_filter / ste_vec)
never engage, and customers seq-scan at scale.
Categories reported:
- inlinability_language (impl is not LANGUAGE sql)
- inlinability_volatility (impl is VOLATILE)
- inlinability_set_clause (impl has SET clauses, e.g. SET search_path)
- inlinability_secdef (impl is SECURITY DEFINER)
- inlinability_transitive (inlinable impl calls a non-inlinable function;
the chain blocks at depth 1)
Usage:
SELECT severity, category, object_name, message
FROM eql_v2.lints()
WHERE severity = 'error'
ORDER BY category, object_name;
Smoke-tested against the current released EQL surface and surfaces
exactly the operators that Phase 1 (#193) needs to fix:
- = / <> / ~~ / ~~* / -> / ->> / @> / <@ on eql_v2_encrypted
- All < <= > >= comparison operators
- The ore_block_u64_8_256_* operators (VOLATILE)
The Phase 1 PR is stacked on top of this commit; the lint becomes
clean for the operators it touches.
This is v1: operator implementations only. Future scope (separate PRs):
index-size budget, operator coverage, STRICT extractor checks,
build-time static analyzer.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR introduces ChangesInlinability Lint Function & Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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 `@src/lint/lints.sql`:
- Around line 6-13: The `@brief` lists "Single-statement SELECT body" as an
inlining precondition but the current implementation (the four UNION ALL
branches) never inspects the function body (prosrc), causing false-clean
results; either update the docstring/@brief to mark that this check is not yet
implemented, or implement the check in the lint by examining prosrc (or
pg_get_functiondef) to ensure the function body is a single SELECT — e.g.,
conservative heuristic: flag when prosrc does not begin with "SELECT" (ignoring
whitespace/comments) or contains a semicolon before the last character — and add
this check to the same logic that builds the existing UNION ALL branches so the
lint actually rejects multi-statement/CTE bodies.
🪄 Autofix (Beta)
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
Run ID: f55ce2b8-4c00-4bc7-ac1f-45b975ad76dd
📒 Files selected for processing (2)
src/lint/lints.sqltests/sqlx/tests/lint_tests.rs
freshtonic
left a comment
There was a problem hiding this comment.
This is awesome. Also if CipherStash fails we can all go consult as leet Postgres internals consultants.
Per CodeRabbit on #195: the docstring listed five inlining preconditions but the lint only checks four. Add an explicit @note so callers aren't misled into reading a clean lint result as a guarantee that every `LANGUAGE sql` function in the EQL surface inlines. The check itself is left for a follow-up — implementing it cleanly needs `prosrc` parsing or `pg_get_functiondef` reflection.
Summary
Resolves #194. Adds a Splinter-shaped lint that reports operator implementation functions which the Postgres planner cannot inline. Non-inlinable functions in operator implementations silently degrade encrypted-query performance: the documented functional indexes (hmac_256 / bloom_filter / ste_vec) never engage, and customers seq-scan at scale without a clear diagnostic.
What's in this PR
src/lint/lints.sql— defineseql_v2.lints(), aLANGUAGE sql STABLEfunction returning(severity, category, object_name, message)rows. Picked up by the existingmise run buildpipeline (-- REQUIRE: src/schema.sqldirective routes it through dependency ordering).tests/sqlx/tests/lint_tests.rs— basic Rust tests asserting the lint runs, returns rows, and emits well-known severity/category values.Categories reported
inlinability_languageLANGUAGE sql(plpgsql etc.)inlinability_volatilityVOLATILEinlinability_set_clauseSETclauses (e.g.SET search_path = ...)inlinability_secdefSECURITY DEFINERinlinability_transitiveUsage
CI gating later (separate PR): fail the build if the query returns non-empty.
Validation
Smoke-tested against a fresh install of this branch's EQL build. The lint correctly identifies every operator implementation in scope for #193 (
=,<>,~~,~~*,@>,<@oneql_v2_encrypted) plus the comparison operators and JSONB extractors that are out of scope for Phase 1 (#193) but worth flagging.The Phase 1 PR is stacked on this branch (#193) and uses the lint output to drive its fixes — circular validation: the lint catches what Phase 1 fixes; after Phase 1 lands, the lint reports zero violations on the targeted operators (and continues to flag the out-of-scope ones, which is correct).
Future scope (separate PRs)
.sqlsource files in this repo before install. Easier to integrate with the doxygen / comment-annotation work already in flight.eql_v2.lints()into the EQL CI workflow.bench_text_eql_idxfailure in perf(bench): add @cipherstash/bench for index-engagement validation stack#424.--! @lint-allow, see issue.Summary by CodeRabbit
New Features
Tests