Skip to content

Add support for @fields atstring#32

Merged
renatomassaro merged 1 commit intomainfrom
add-support-for-atfields
Mar 29, 2026
Merged

Add support for @fields atstring#32
renatomassaro merged 1 commit intomainfrom
add-support-for-atfields

Conversation

@renatomassaro
Copy link
Copy Markdown
Owner

@renatomassaro renatomassaro commented Mar 29, 2026

Summary by CodeRabbit

  • New Features

    • Added support for explicit field binding directives in SQL queries, allowing users to specify which fields should be extracted, particularly beneficial for complex JOIN scenarios with aliased tables.
  • Tests

    • Added comprehensive test coverage for field binding parsing, including wildcard selections, explicit column lists, and edge cases across different query types.

Just a hack to workaround the rudimentary SQL parser we have in place.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

The changes introduce support for a new -- @fields [...] SQL annotation directive that enables explicit field binding declarations in queries. A handler processes this directive, parses the specified fields, and updates the query's field bindings while preserving parameter bindings.

Changes

Cohort / File(s) Summary
Core feature implementation
lib/feeb/db/query.ex, lib/feeb/db/query/binding.ex
Added handle_atstring/4 handler to parse -- @fields [ directives via Binding.parse_atstring/1 and update query field bindings. Added early-return clause to parse_fields/3 that returns explicitly provided bindings directly, bypassing standard parsing logic.
Test query
priv/test/queries/test/chaos.sql
Added new :get_with_join test query demonstrating the -- @fields [:*] annotation for a SELECT statement with JOIN and aliased columns.
Test coverage
test/db/query/binding_test.exs, test/db/query_test.exs
Added comprehensive ExUnit tests validating parse_params/3, parse_fields/3, and parse_atstring/1 behavior with explicit bindings; extended chaos query validation to include the new :get_with_join query.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 A rabbit hops with glee today,
With @fields marking the way,
No guessing what columns to bind,
Explicit paths are far more kind! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add support for @fields atstring' directly and accurately describes the main change: implementing a new @fields atstring handler for parsing field bindings in SQL queries.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 add-support-for-atfields

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/db/query/binding_test.exs (1)

52-57: Consider adding edge case tests for malformed input.

The parse_atstring/1 function uses String.to_atom/1 which will create atoms for any string input. Consider whether tests for edge cases (e.g., empty input, whitespace-only fields) would be valuable to document expected behavior.

💡 Optional: additional edge case tests
test "handles edge cases" do
  # Single field
  assert [:id] == Binding.parse_atstring("id]")
  # Field with underscore
  assert [:user_id] == Binding.parse_atstring("user_id]")
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/db/query/binding_test.exs` around lines 52 - 57, Add unit tests covering
malformed and boundary inputs for Binding.parse_atstring/1: include cases for
empty string (""), whitespace-only strings ("   ]"), missing trailing bracket or
extra characters, single-field inputs ("id]"), fields with underscores
("user_id]"), and unexpected tokens so the test suite documents the function's
behavior when String.to_atom/1 is fed unusual values; update the
binding_test.exs describe "parse_atstring/1" block to assert the expected
outputs (or errors) for these edge cases so future changes to
Binding.parse_atstring are validated against them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/db/query/binding_test.exs`:
- Around line 52-57: Add unit tests covering malformed and boundary inputs for
Binding.parse_atstring/1: include cases for empty string (""), whitespace-only
strings ("   ]"), missing trailing bracket or extra characters, single-field
inputs ("id]"), fields with underscores ("user_id]"), and unexpected tokens so
the test suite documents the function's behavior when String.to_atom/1 is fed
unusual values; update the binding_test.exs describe "parse_atstring/1" block to
assert the expected outputs (or errors) for these edge cases so future changes
to Binding.parse_atstring are validated against them.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 837b9004-384b-4500-9cf1-0f46e7a7b1d1

📥 Commits

Reviewing files that changed from the base of the PR and between 2bc76b5 and 928c37d.

📒 Files selected for processing (5)
  • lib/feeb/db/query.ex
  • lib/feeb/db/query/binding.ex
  • priv/test/queries/test/chaos.sql
  • test/db/query/binding_test.exs
  • test/db/query_test.exs

@renatomassaro renatomassaro merged commit ee74494 into main Mar 29, 2026
3 checks passed
@renatomassaro renatomassaro deleted the add-support-for-atfields branch March 29, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant