Support inequality operators when parsing queries

[renatomassaro]

Very rudimentary but works great so far ✅

Summary by CodeRabbit

• Bug Fixes
  • Corrected parsing of key-value bindings in SQL statements, ensuring parameters map to the correct fields and improving reliability for SELECT queries with single or multiple placeholders.
• Tests
  • Added comprehensive automated tests for parameter binding across multiple SELECT query cases to improve coverage and prevent regressions.

[coderabbitai]

Walkthrough

Updated private SQL binding parsing in lib/feeb/db/query/binding.ex to split on " ?" and adjust atom extraction position. Added a new ExUnit test module validating parse_params/3 across several SELECT statements with varying placeholder counts. No public API changes.

Changes

Cohort / File(s)	Summary
Binding parsing logic lib/feeb/db/query/binding.ex	Updated parse_kv: split tokens on " ?" instead of " = ?", and extract the second-to-last token for atom conversion. No public API changes.
Tests for parse_params test/db/query/binding_test.exs	Added Feeb.DB.Query.BindingTest validating Binding.parse_params(:select, sql, []) across multiple SELECT SQL strings and expected bindings.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant Binding as Binding.parse_params/3
    participant Parser as parse_kv (private)

    Client->>Binding: parse_params(:select, sql, [])
    activate Binding
    Binding->>Parser: parse_kv(sql)
    note right of Parser: Tokenize by " ?"<br/>Extract key from second-to-last token
    Parser-->>Binding: [{:key, :placeholder}, ...]
    Binding-->>Client: bindings list
    deactivate Binding

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at each “ ?” I see,
Hop-splitting tokens with boundless glee.
Second-to-last? A tasty clue—
I nibble the key, the binding comes through.
Tests sprout like clover, green and bright—
All queries bound, we leap in delight! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title explicitly states the main change of adding support for inequality operators in the SQL parsing logic, which directly corresponds to the updated parse_kv implementation and accompanying tests.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch support-inequality-operators-in-query-parsing

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

lib/feeb/db/query/binding.ex (1)

45-52: Consider edge cases for robustness.

The current implementation assumes:

1. A space always precedes the ? placeholder (e.g., id = ? works, but id=? would extract the wrong token)
2. Simple column names without functions or complex expressions

While these assumptions are reasonable for typical SQL usage, consider whether documentation or validation would help prevent subtle bugs.

test/db/query/binding_test.exs (1)

8-14: Consider adding edge case tests for completeness.

While the current test cases cover common scenarios well, consider adding tests for edge cases such as:

• Operators without spaces (e.g., "select * from users where id=?")
• Multiple consecutive spaces
• Different whitespace characters (tabs, newlines)

This would help catch potential parsing issues early and document expected behavior for unusual inputs.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1409440 and ee2edee.

📒 Files selected for processing (2)

• lib/feeb/db/query/binding.ex (1 hunks)
• test/db/query/binding_test.exs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

test/db/query/binding_test.exs (1)

lib/feeb/db/query/binding.ex (1)

• parse_params (16-21)

lib/feeb/db/query/binding.ex (1)

lib/feeb/db/query.ex (1)

• defp handle_atstring(<<98, 105, 110, 100, 32, 91, raw_bindings::binary>>, qs, id, q) do (398-408)

🔇 Additional comments (3)

lib/feeb/db/query/binding.ex (2)

45-52: LGTM! Logic correctly supports inequality operators.

The change from splitting on " = ?" to " ?" combined with extracting Enum.at(-2) correctly handles various comparison operators (=, >, <, >=, <=). The logic successfully extracts the column name before the operator for all tested cases.

---

45-52: parse_params callers scope verified
Only one invocation in lib/feeb/db/query.ex remains; no additional callers affected by the updated parse_kv logic.

test/db/query/binding_test.exs (1)

1-20: LGTM! Good test coverage for the new inequality operator support.

The test cases effectively validate the updated parse_params/3 behavior across various operators (=, >=, >, <, <=) and combinations with AND/OR clauses. The parameterized test structure using Enum.each is clean and maintainable.