Skip to content

Table-qualify amount/expires_at scopes so they compose with joins#5

Open
rameerez wants to merge 1 commit into
mainfrom
fix/qualify-amount-scopes-for-joins
Open

Table-qualify amount/expires_at scopes so they compose with joins#5
rameerez wants to merge 1 commit into
mainfrom
fix/qualify-amount-scopes-for-joins

Conversation

@rameerez

@rameerez rameerez commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Problem

credits / debits (and not_expired / expired) use bare column predicates:

scope :credits, -> { where("amount > 0") }
scope :debits,  -> { where("amount < 0") }

A bare amount is ambiguous the moment a host joins another table that also has an amount column — and every wallet has an amount on both wallets_transactions and wallets_transfers. So a perfectly natural query like:

# "which of my transactions belong to a reversal transfer?"
wallet.transactions.joins(:transfer).merge(Wallets::Transfer.where(category: :reversal)).credits

raises PG::AmbiguousColumn (SQLite and MySQL raise the equivalent — ambiguous column name: amount).

We hit this in production (a real-money carpooling app): computing "lifetime earned, net of reversals" needs exactly that join, and we had to work around it host-side with a two-step id resolution instead of a clean .joins(:transfer).credits.

Fix

Route the four column-referencing raw-SQL scopes through arel_table[:col], so the generated SQL is always table-qualified ("wallets_transactions"."amount" > 0). Bonus: arel_table derives from the resolved (overridable) table_name, so the embedding/subclassing story qualifies correctly for free — no hardcoded table string.

Test

New regression in transaction_test.rb:

  • creates a transfer between two wallets (so both legs + a wallets_transfers.amount exist),
  • runs wallet.transactions.joins(:transfer).debits/.credits — which raised before the fix (verified: reverting the scopes reproduces SQLite3::SQLException: ambiguous column name: amount),
  • asserts each returns the correct leg,
  • asserts the generated SQL is table-qualified (DB-agnostic).

Full suite green (93 runs, 346 assertions).

`credits`/`debits`/`not_expired`/`expired` used bare `where("amount > 0")`
-style predicates. A bare column name is ambiguous the moment a host joins
another table carrying the same column — and every wallet has an `amount`
on BOTH wallets_transactions and wallets_transfers, so a natural query like

    wallet.transactions.joins(:transfer).credits

(find the transactions belonging to a category of transfer — e.g. reversals)
raised PG::AmbiguousColumn; SQLite and MySQL raise the equivalent.

Route the predicates through `arel_table[:col]` so the generated SQL is
always table-qualified. This also respects the embedding `table_name`
override for free (arel_table derives from the resolved table name), so
embedded/subclassed transaction tables qualify correctly too.

Regression test reproduces the ambiguity across the transfer join and
asserts the generated SQL is qualified (DB-agnostic).
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