containsIgnoreCase() behaviour change (maybe regression)

[McNetic]

Expected behavior

When updating an outdated application from Jooq 3.11.5 to Jooq 3.19.8 (I know, I know), I found that the method containsIgnoreCase() behaves differently on numeric fields now.

With 3.11.5, the behaviour was like this (varchar field is only for comparison, as it works as expected):

Field fieldVarchar = DSL.field("TESTVARCHAR", SQLDataType.VARCHAR);
Field fieldNumeric = DSL.field("TESTNUMERIC", SQLDataType.NUMERIC);
System.out.println(fieldVarchar.containsIgnoreCase("test"));
System.out.println(fieldNumeric.containsIgnoreCase("test"));

lower(TESTVARCHAR) like lower(('%' || 'test' || '%')) escape '!'
lower(cast(TESTNUMERIC as varchar)) like lower(('%' || 'test' || '%')) escape '!'

Actual behavior

With 3.19.8 (and also versions before that, did not check exactly since when), it does no longer work for numeric fields, as the rendered SQL is now this:

TESTVARCHAR ilike (('%' || replace(
  replace(
    replace('test', '!', '!!'),
    '%',
    '!%'
  ),
  '_',
  '!_'
)) || '%') escape '!'

(which is ok for the varchar field of course), and

cast(TESTNUMERIC as varchar) ilike (('%' || cast(null as varchar)) || '%') escape '!'

which is always true at least for Oracle databases, and does generally not work as before .

It is debatable wether it should work like it did before, so I leave it up to discussion if is is a bug/regression or not (background: in the application, it is used for query generation where the uses tables and columns are not known at compile time. It is certainly possible to check for types before blindly using containsIgnoreCase())

Steps to reproduce the problem

Execute the code snippet

Field fieldNumeric = DSL.field("TESTNUMERIC", SQLDataType.NUMERIC);
System.out.println(fieldNumeric.containsIgnoreCase("test"));

with a current jooq version.

jOOQ Version

jooQ 3.19.8

Database product and version

No database used for the demonstration.

Java Version

8 and 17

JDBC / R2DBC driver name and version (include name if unofficial driver)

[lukaseder]

Thank you for your report.

When doing this:

System.out.println(fieldNumeric.containsIgnoreCase("test"));

Is there any expectation with respect to the exact generated SQL?

The expression has no SQLDialect associated with it, so SQLDialect.DEFAULT applies. The DEFAULT dialect is mainly used for debugging purposes. Its goal is to render SQL as close to the jOOQ API calls as possible. But it cannot be reasonably used in any dialect specific environment. See also the Javadoc:
https://www.jooq.org/javadoc/latest/org.jooq/org/jooq/SQLDialect.html#DEFAULT

If you want to render an expression in the context of a SQLDialect, use DSLContext.render(QueryPart):
https://www.jooq.org/javadoc/latest/org.jooq/org/jooq/DSLContext.html#render(org.jooq.QueryPart)

There's a long-running "epic" task that attempts to simplify all of the rendered SQL in order to remove emulations from the SQLDialect.DEFAULT rendering:

• SQL generated with SQLDialect.DEFAULT should match closest supported parser syntax #9085

This would explain why there was a change.

Does your actual code also depend on QueryPart.toString() of arbitrary QueryPart expressions (which have no SQLDialect attached via the Attachable API)? Or is this just an artefact of creating a minimal example?

[McNetic]

Sorry if the report caused confusion because of being incomplete or misleading.
In our actual application, we use various Oracle-Dialects(depending on the client database version), and the constructed queries are directly executed via jooq; the generated sql is equal or similar enough to produce the same problem described here, so I chose the toString()-Representation to make the problem easily visible.

I am not sure how the behaviour should be, I just noted that in older versions, our code "worked", and in newer versions, the semantic of the generated code changed drastically, and it may not have been intended (I looked into the source code and I think the change stems from additional type conversions taking place under the hood).

[lukaseder]

I chose the toString()-Representation to make the problem easily visible.

Sure, that's what I assumed, and thus I asked what your actual code looks like. Because if it uses toString(), then this isn't a bug. But if there's a more subtle change of behaviour in a context where the SQLDialect should be available, then this is would be a bug.

So, if there's another example that doesn't use toString(), I'll be happy to look at it.

[lukaseder]

So, if there's another example that doesn't use toString(), I'll be happy to look at it.

Including, if you think that jOOQ's internals call toString(), that would also be a bug, but I'd need to be able to reproduce it first.

[McNetic]

Ok, i rewrote the simple example to better illustrate the change, and also, that it does rely on neither toString() nor a specific SQLDialect:

        Field fieldNumeric = DSL.field("TESTNUMERIC", SQLDataType.NUMERIC);
        DSLContext create = DSL.using(SQLDialect.POSTGRES);
        System.out.println(create.renderInlined(fieldNumeric.containsIgnoreCase("test")));
        create = DSL.using(SQLDialect.MYSQL);
        System.out.println(create.renderInlined(fieldNumeric.containsIgnoreCase("test")));

Output with jooq 3.11.5:

TESTNUMERIC ilike ('%' || 'test' || '%') escape '!'
lower(cast(TESTNUMERIC as char)) like lower(concat('%', 'test', '%')) escape '!'

Output with jooq 3.19.8:

cast(TESTNUMERIC as varchar) ilike (('%' || cast(null as varchar)) || '%') escape '!'
lower(cast(TESTNUMERIC as char)) like lower(concat('%', cast(null as char), '%')) escape '!'

EDIT: I can not easily test this with the oracle dialect, as we are still using java 8, and to use the current jooq pro I would have to download pro versions I currently don't use etc. But the rendered SQL (with the "null" where "'test'" was before) is the same; I found the issue by debugging the real application and inspecting the executed SQL.

[lukaseder]

I see now what you mean. The issue here is that in a comparison Field<T>.eq(T), the T value (bind value) will have the Field<T>'s type applied to it automatically. But a string value ("test") cannot be bound as a NUMERIC value, which is why you're seeing that null value there.

Normally, you'll get a compilation error because fieldNumeric is a Field<BigDecimal>.

To be honest, I'm not really sure what to do with this now. What would be a use-case of calling containsIgnoreCase() on a numeric field? E.g. looking for the digits %13% in a number like 161713415?

[lukaseder]

The issue here is that in a comparison Field<T>.eq(T), the T value (bind value) will have the Field<T>'s type applied to it automatically.

I don't think there's a single issue for this kind of change. But it's an expectation that users keep having for various jOOQ API. An example of such a change / fix is this:

• Update using multi-column set clause with DSL.row and custom type logs warn on deprecated static type registry #16627

It's mostly related to when Converter or Binding are involved, i.e. when doing a comparison Field<U>.eq(U) with U being a user type from a Converter<T, U>, then we must ensure that the Field<U>'s attached DataType<U> is used for the binding.

These things always assume that type safety applies first, which it normally does. Given your example uses rawtypes, I'm assuming you're doing some dynamic SQL where arbitrary fields can be compared with arbitrary values and arbitrary operators? In that case, I do believe it would be good to prevent certain combinations that don't really make sense.

Of course, it would be possible to take into account the possibility of purely string based operations being applied to non-string columns, but that would be a vast change throughout the jOOQ API, and would again lead to behavioural change similar to this one. I'm not sure if it's worth it...

[McNetic]

In our application, we have a "search all columns" text box on a lot of tables the application displays. Naturally, the user will want to search for quote numbers starting with "52" and things like that. So the conditions are generically generated for all columns.

As I said, we do not need to rely on this "feature". We can check the column type and build fitting conditions correspondingly.

Our original code probably originates from the fact that quote_no like '%52% is valid sql and behaves as expected for a numeric column quote_no. With the help of jooq, writing containsIgnoreCase() is arguably better readable as the intention is obvious; on the other hand, one cannot really assume that the semantics of LIKE is preserved if it is not used explicitly.

In the end, I think you will have to decide how jooq should behave in this case.

[McNetic]

An additional thought: Right now, there is SQL generated that executes, but definitely does not as intended. Maybe at least there should be a warning or an error in this case; however, I am not sure that is easily achievable.

[lukaseder]

Our original code probably originates from the fact that quote_no like '%52% is valid sql and behaves as expected for a numeric column quote_no.

It's not valid in all RDBMS. E.g. in PostgreSQL:

select 1234 like '%23'

This produces:

SQL Error [42883]: ERROR: operator does not exist: integer ~~ unknown

In the end, I think you will have to decide how jooq should behave in this case.

I'll definitely have to think about this a bit more. Apart from the regression in your case, there are some things to consider:

• like() is explicitly about strings, so to some extent, the previous behaviour could be expected. In fact, numericField.like("test") works as you were expecting, but that's wrong as can be seen in this issue: Converters applied to Field.like #12472
• contains() is not about strings, though (it works with arrays as well, thus accepting T, not String)
• containsIgnoreCase() on the other hand, is about strings again

An additional thought: Right now, there is SQL generated that executes, but definitely does not as intended. Maybe at least there should be a warning or an error in this case; however, I am not sure that is easily achievable.

Yes, there's a known issue where these implicit conversions of bind values and other things should throw instead of silently return null. Unfortunately, this will be an incompatible change, and as such, it will be hard to implement this correctly. Even a warning may not be what people want who prefer the current behaviour, and given that there are some static contexts (with no access to a Configuration), there's no easy way to introduce a Settings for this behaviour.

See also:

• DataType#convert(Object) returns null instead of throwing DataTypeException #4681