Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Array contains operator support #67

Merged
merged 3 commits into from Feb 18, 2013

Conversation

Projects
None yet
2 participants
Contributor

jagregory commented Feb 18, 2013

Hello,

I've added support for the Postgres array contains operator @> (see: Array Functions and Operators). I followed the existing patterns to implement this, tests included.

The only thing I'm not entirely happy about is that the ActiveRecord query extension is called array_contains because contains was already taken by the INET feature. Having array_contains and overlap seems inconsistent, but perhaps that's something we could address later.

If you're happy with this change and how I've done it, I'll do the same for the array is contained by operator <@.

Cheers,
James

jagregory added some commits Feb 18, 2013

Array contains operator ( @> ) support
1. Add support for the Postgres array contains operator. This operator
is like overlap, but expects all of the provided elements to be in the
array.

    MyModel.where.array_contains(tags: [1, 2])

2. Moved the array nodes into their own file

@danmcclain danmcclain and 1 other commented on an outdated diff Feb 18, 2013

docs/querying.md
+elements of an array are within another.
+
+```sql
+ARRAY[1,2,3] @> ARRAY[3,4]
+-- f
+
+ARRAY[1,2,3] @> ARRAY[2,3]
+-- t
+```
+
+Postgres_ext extends the `ActiveRecord::Relation.where` method by
+adding an `array_contains` method. To make a contains query, you
+can do:
+
+```ruby
+User.where.array_contains(:nick_names => ['Bob', 'Fred'])
@danmcclain

danmcclain Feb 18, 2013

Contributor

I would have this method be named contains instead of array_contains, since it is actually used for the Range datatype as well. I've been meaning to rename array_overlap to just overlap (and have in the querying API, I need to go back and change the Arel predicate definition)

@jagregory

jagregory Feb 18, 2013

Contributor

I would've preferred that too, but contains is already taken by the INET implementation. Unless I'm missing something obvious, there's a naming conflict there. I would much prefer this to be named contains if possible.

If there's a solution I'm not aware of, let me know and I'll fix it up.

@danmcclain

danmcclain Feb 18, 2013

Contributor

For now, why not check the column type in the WhereClause#contains method and call the corresponding Arel method for now. Ultimately, when I rename the array_overlap Arel method, I'll investigate fixing the contains visitor to handle both.

Remove explicit array_contains where extension, use contains instead
Removed the array_contains where extension and changed the existing
contains extension to check the type of the column. It will use either
array_contains or the INET contains depending on whether the column is
an array or not.
Contributor

jagregory commented Feb 18, 2013

@danmcclain I've made the changes we discussed earlier, so the contains where extension now checks the type of the column to choose the arel method (and I've removed the array_contains extension).

danmcclain added a commit that referenced this pull request Feb 18, 2013

@danmcclain danmcclain merged commit dae3222 into DavyJonesLocker:master Feb 18, 2013

1 check passed

default The Travis build passed
Details
Contributor

danmcclain commented Feb 18, 2013

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment