search

alecdotninja / active_record_distinct_on

Support for `DISTINCT ON` statements when querying with ActiveRecord
MIT License
34 stars 11 forks source link

addressed Arel::Visitors::UnsupportedVisitError #12

Closed masukomi closed 3 years ago

masukomi commented 4 years ago

we were getting Arel::Visitors::UnsupportedVisitError Exception: Unsupported argument type: String

this fixes that by wrapping all the fields with arel_attribute(field)

alecdotninja commented 4 years ago

@masukomi Thank you for taking the time to fix this! :tada:

I'm sorry for the delay in review. I have pushed a new commit to master which should fix the CI issues on this branch. Would you mind rebasing?

Also, are you able/willing to provide a test that fails without the fix to avoid future regressions?

masukomi commented 4 years ago

rebased with new commit.

Also, are you able/willing to provide a test that fails without the fix to avoid future regressions?

I don't think i actually understand what causes the problem well enough to set up the appropriate world state in order to invoke this behavior in a test. :(

also, it's been a while since i was focusing on this and the world state of the bug has fallen out of my brain. ;)

AlexWayfer commented 3 years ago

I have the same issue. Code is like:

Procedure.where(Procedure.arel_table[:name].matches("#{query_param}%"))
  .union(
    Procedure.where(Procedure.arel_table[:name].matches("%#{query_param}%"))
  )
  .distinct_on(:id)
  .all
masukomi commented 3 years ago

thinking that as this issue has been confirmed, and there's been no movement to write a test in a full year it might make sense to just merge this fix without the test, rather than leaving it broken.

alecdotninja commented 3 years ago

Thank you for your work here, @masukomi . This is now released in v1.1.0.