-
Notifications
You must be signed in to change notification settings - Fork 2
Fix/tagged struct expr handling #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The catch-all IN clause handler was wrapping SubQuery expressions in
JSON_EACH(), producing invalid SQL like:
WHERE id IN (SELECT value FROM JSON_EACH(?))
This adds a specific pattern match for %Ecto.SubQuery{} before the
catch-all, generating proper inline subqueries:
WHERE id IN (SELECT s0.id FROM table AS s0 WHERE ...)
Also adds a general SubQuery expression handler that properly resolves
parent query aliases and combination queries.
This fixes compatibility with libraries like Oban that use subqueries
in UPDATE...WHERE id IN (subquery) patterns.
Ecto's query planner transforms {:type, _, [expr, type]} AST nodes
into %Ecto.Query.Tagged{} structs. The SQL generator only handled
the pre-planning tuple form, causing type-wrapped fragments (e.g.
type(fragment(...), :integer)) to fall through to the catch-all
expr clause which rendered a single '?' placeholder. This caused
parameter count mismatches with Hrana/Turso.
Fixes Oban Web JobQuery.limit_query crash on completed jobs page.
WalkthroughThe update modifies internal logic in the Ecto LibSQL adapter to enhance SQL generation for Changes
Sequence Diagram(s)Skipped (diagram criteria not met, as only one file and no three-component interaction). Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
…67 Adds in_expression_comprehensive_test.exs to catch regressions in: 1. Issue #63: JSON encoding of list parameters in IN clauses - Simple list parameters should expand to individual placeholders - ~w() sigils should be handled via %Ecto.Query.Tagged{} detection 2. PR #66: Subqueries in IN expressions (Oban pattern) - WHERE id IN (SELECT ...) should generate proper SQL subquery - Before fix: incorrectly wrapped in JSON_EACH() causing malformed JSON error - After fix: generates inline subquery with correct aliases 3. PR #67: Tagged struct and type-wrapped expressions - Post-planning %Ecto.Query.Tagged{} nodes must be handled - Type-wrapped fragments should not fall through to catch-all '?' placeholder - Before fix: parameter count mismatch with Hrana/Turso - After fix: correct parameter count and proper type handling Tests include: - Basic list parameter expansion - Empty lists and single-element lists - ~w() sigil handling (Oban Lite pattern) - Multiple IN clauses combined - Subqueries with WHERE, SELECT, complex filters - Type-cast expressions - Integration tests combining multiple patterns All 15 tests pass with the fixes in place. Relates to: #63, PR #65, PR #66, PR #67
Ecto's query planner transforms {:type, _, [expr, type]} AST nodes into %Ecto.Query.Tagged{} structs. The SQL generator only handled the pre-planning tuple form, causing type-wrapped fragments (e.g. type(fragment(...), :integer)) to fall through to the catch-all expr clause which rendered a single '?' placeholder. This caused parameter count mismatches with Hrana/Turso.
Fixes Oban Web JobQuery.limit_query crash on '$state' jobs page.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.