Unexpected warning when using the SQL TRIM function with less than 4 parameters

Description

The ANSI SQL TRIM function is defined as:

However, the first three arguments to the function are optional. Therefore, all the following calls to this function are valid:

However, any time this function is called from HQL with less than the maximum allowed of 4 parameters, the following warning message is seen in the logs, generated from the class org.hibernate.dialect.function.TemplateRenderer:

For example, the following Spring Data JPA repository method (which generates an HQL query) leads to this error:

This warning message has no impact on the actual execution of the code, which works as expected. However, WARN log level seems excessive for this case. The following portion of the code generates the warning:

At the very least, the severity level for this message should be changed to INFO. A proper check should know the minimum and maximum number of mandatory arguments allowed for the SQL function and compare the supplied number of arguments with this range. If the number of arguments is outside this range, the method must halt the operation by throwing an exception, instead of just logging a message and continuing.

Have attached a Maven project as a sample. Just run the test cases to see the aforementioned warning message being displayed.

Environment

Any OS

Activity

Show:
Steve Ebersole
April 11, 2017, 4:32 PM

I think the parameter one may not be addressable until 6.0. TBH though I am not sure it is necessarily the EXTRACT function being complained about. The warning states that only 1 argument was found, but it seems like that translation ought to get 2 arguments ("YEAR", "FROM"). Could you step into that and make sure it is the EXTRACT function where this occurs?

I think a related improvement would to also include the function name in the warning.

As for the level, I agree. I think this should be DEBUG so far as this being something that should be addressed during development. So we should make this DEBUG, but it also seems to me that there ought to be way to earmark log messages as kind of development-time warning. Or do we just mark it as DEBUG and move on? , , ?

Andrea Boriero
April 12, 2017, 10:37 AM

I think marking as DEBUG (or even INFO) is fine, for sure we have to add the function name to the log message.

As a major change may be we can add the min number of arguments the function requires, if the passed arguments are less than the min we can throw an exception.

Chris Cranford
April 12, 2017, 12:51 PM

I concur with Andrea, marking it with a more appropriate logging level should be sufficient. Regarding the exception piece which Andrea mentioned, I suspect the database will do this for us should it determine the provided number of arguments are incorrect resulting in a SQLException anyway.

Steve Ebersole
April 12, 2017, 12:57 PM

The concern with "the database will do this for us" is that we often map certain "HQL functions" to differently named SQL functions - might be confusing.

Chris Cranford
April 12, 2017, 1:21 PM

, I understand and I agree with changing the logging level and adding the function to the message. What I am not sure about is the suggestion for throwing an exception when the supplied number of arguments are outside of [min,max]. Perhaps the reason why we didn't previously is we couldn't support a range-based argument check properly, hence why we merely logged the warning. I'm just suggesting we consider all use cases before adding the exception-case.

Assignee

Unassigned

Reporter

Manish J

Fix versions

None

Labels

backPortable

None

Suitable for new contributors

None

Requires Release Note

None

Pull Request

None

backportDecision

None

Components

Affects versions

Priority

Minor
Configure