rom-sql icon indicating copy to clipboard operation
rom-sql copied to clipboard

string::lower() comparison with an array fails

Open renatolond opened this issue 4 years ago • 7 comments

Describe the bug

When using string:lower() in a where statement, if the comparison is with an array, the comparison fails.

To Reproduce

Assuming a table "table" with a string field "field", in any repository do:

root.where { string::lower(field).is([“value1”, “value2”]) }

Fails with

Caused by PG::UndefinedFunction: ERROR: operator does not exist: text = record
LINE 1: …" FROM “table” WHERE (LOWER(“table”.“field”) = ('value1…
HINT: No operator matches the given name and argument types. You might need to add explicit type casts.

Expected behavior

The comparison is expected to generate a query with (LOWER("table"."field") in ('value1', 'value2')) and the query should work

Your environment

  • Affects my production application: NO
  • Ruby version: 2.6.6
  • OS: Linux

renatolond avatar May 27 '20 07:05 renatolond

For the record, this is what we should support:

root.where { lower(field).in(["value1", "value2"]) }

Return types should not be required in where blocks after all.

solnic avatar May 27 '20 09:05 solnic

@solnic I disagree, return type is required to determine the list of available functions/operators. It's not required for common operators such as IN or =

flash-gordon avatar May 27 '20 10:05 flash-gordon

@flash-gordon not sure if I understand - do you mean that we should stop handling function building via method_missing and go the explicit route with a list of available functions?

solnic avatar May 28 '20 08:05 solnic

@solnic the list of available operators is determined by the expression type: https://github.com/rom-rb/rom-sql/blob/f3ef5817462c4915f37d0523ea2a2ba95c36a9de/lib/rom/sql/type_extensions.rb For instance json_returning_function(field).json_operator(args) will be a method missing (or mb some other) error whereas json::json_returning_function(field).json_operator(args) will work just fine. It's consistent and I think we should keep it that way.

flash-gordon avatar May 28 '20 10:05 flash-gordon

@flash-gordon OK fair enough 😄 so it seems like this bug is caused by the lack of in method defined on ROM::SQL::Function

solnic avatar May 29 '20 08:05 solnic

Just to add a little comment, the method that seems to support either = or in in some other parts seem to be is, is it intentional to split it into .is and .in ? (see https://github.com/rom-rb/rom-sql/blob/4611a760d594c6e3f2ee471462bc8add27df293f/spec/unit/function_spec.rb#L74-L75 )

renatolond avatar May 29 '20 09:05 renatolond

@renatolond actually, it's not intentional - ROM::SQL::Attribute#is uses Sequels =~ operator which supports array arg too (just like in).

This should be unified somehow, because ROM::SQL::Function#is doesn't do the same thing 🙈

solnic avatar May 30 '20 09:05 solnic