Query with OR condition on array

[ryanb]

Context

Using the jsonb backend with PostgreSQL, I am trying to find all records missing translations of the current locale. In ActiveRecord you can pass an array of values to perform an OR condition.

Product.i18n.where(name: ['', nil])

Expected Behavior

In a normal string column this would find all products whose name is empty or nil.

Actual Behavior

This looks for a product whose name matches ['', nil] json data.

Possible Fix

My current solution is this:

Product.i18n.where(name: '').or(Product.i18n.where(name: nil))

I understand that since this is a jsonb data type ActiveRecord treats the query condition differently, however it would be nice if Mobility kept the behavior of a string column. BTW great gem!

[shioyama]

Thanks for the idea! This is definitely something I could imagine people wanting.

One edge case that we'd need to consider is where people are actually storing non-string translated data in a jsonb backend. In this case the result that Mobility searches for names that match ['', nil] would actually be correct. I agree though that the vast majority would want/expect the result you proposed here.

Implementation-wise, passing an array to query on translations produces quite different things for each backend (I hadn't really considered this very carefully TBH). For jsonb, as you noted, Mobility will query for a matching array. For KeyValue (and Table since the logic is similar), it actually works as expected as long as the array does not contain nil, which is treated separately (i.e. Post.i18n.where(title: nil) does some magic to make this work that does not happen with an array).

So I think if this were to be implemented, it would need to be something whose behavior is specified consistently across backends in specs, but where implementation varies per backend. Jsonb shouldn't be too hard. Your solution with or produces this SQL:

SELECT "products".* FROM "products" WHERE (("products"."name" @> '{"en":""}') OR (NOT ("products"."title" ? 'en')))

I'm not sure if this is optimal or not (my memory of jsonb is a bit rusty), but it seems reasonable to me.

So long story short: implementing this just for jsonb wouldn't be too hard (and hstore since it's similar), and along with that adding specs to see what level of consistency there is with other backends.

Another aside, but Mobility makes an attempt not to ever store blank strings (converting them to nil in the presence plugin), so I don't think you should have to be doing that kind of OR query unless you're doing something tricky in your SQL. If Mobility is accidentally leaving behind blank values, then that's a bug that should be fixed 😄

[ryanb]

Another aside, but Mobility makes an attempt not to ever store blank strings (converting them to nil in the presence plugin), so I don't think you should have to be doing that kind of OR query unless you're doing something tricky in your SQL. If Mobility is accidentally leaving behind blank values, then that's a bug that should be fixed

That is very good to know, thanks! I probably won't need this feature now but still nice to have.

One edge case that we'd need to consider is where people are actually storing non-string translated data in a jsonb backend.

Can you check the translates ..., type: setting for this? Not certain if jsonb backend even uses this setting but I think it would be useful to track the type.

[shioyama]

Another aside, but Mobility makes an attempt not to ever store blank strings (converting them to nil in the presence plugin), so I don't think you should have to be doing that kind of OR query unless you're doing something tricky in your SQL. If Mobility is accidentally leaving behind blank values, then that's a bug that should be fixed

That is very good to know, thanks! I probably won't need this feature now but still nice to have.

About this one, see #92. It may actually be disabled if you are setting Mobility.default_options as a hash.

Can you check the translates ..., type: setting for this? Not certain if jsonb backend even uses this setting but I think it would be useful to track the type.

Yes this is actually a common misunderstanding (need to improve the readme), but only the (default) KeyValue backend uses this setting. But you're right that it might be useful to have for this case. Good idea! Thanks, I'll think about it.

[shioyama]

While working on a fix for this, I discovered that the query in this issue:

Product.i18n.where(name: ['', nil])

will not work correctly with the Table or KeyValue backends either, although the reason is different. Array-valued arguments will match correctly as long as there are no nils. When there are nils, however, Mobility should use an outer join in this case, to join the translations table (otherwise it cannot check if there are nil translations), but currently it uses an inner join and misses them (it only uses an outer join when all values are nil, not considering array values).

My fix in #128 fixes this issue as well as the one posted here for jsonb/hstore backends.

[shioyama]

@ryanb This is fixed on master now, would you like to give it a try? 😄