When we use isIn(emptyList) in where condition, it should not query full table.

[jesusslim]

It's been posted here two years ago. But I don't think isInRequired or withListEmptyCallback is a good solution.Because if we can remember to use isInRequired,of course we can also remember to check empty list before isIn.

I can understand that you never want to build invalid sql, but in my opinion, once we use isIn, whatever param we pass, you should never change our sql, we don't want fixing, if you think our sql is invalid just throw exception, not change it to another wrong sql.

If we pass empty list, the sql should be in (),you can't change it to query full table, because it will be a disaster in production if we have a table with big data, there will be lots of slow queries. And if we use it in delete, it will delete all rows, it's a real disaster because you changed our sql wrong for "valid sql".

And for most programmers, they don't know this issue, so every time in code review, we have to take time to check if they have checked empty list or not before isIn.And it's like a time bomb if someone forget to check empty list before use isIn.

I think it will be a good solution if there is a config setting to decision throw exception or query full table when isIn empty list.

[jeffgbutler]

@jesusslim I understand the concern and I agree that the current solutions are a bit lacking. The problem is more that just IsIn - most conditions are optional. There are many cases when a WHERE clause could be removed from the rendered SQL.

One thing I will not compromise is about invalid SQL - I will not allow the library to generate invalid SQL. I've actually been adding checks for this in various places.

I think the best solution is - as you say - some kind of global configuration mechanism that would cause an exception to be thrown in the case of a "disappearing" where clause.

My initial thought is to do something similar to Log4J where you could set a default behavior and then it could be overridden for individual statements. I haven't fully thought this through but I am interested in your thoughts about this.

Configuration isn't so easy as there is no owning context for the statements. What is needed is a method of configuring each statement - probably by adding something to the DSL. So we could make the default behavior to throw an exception if no where clause will render, but provide a DSL mechanism to disable the exception if it was needed for some reason.

[jeffgbutler]

@jesusslim see the new docs page for details on how I've addressed this issue and let me know what you think.

https://github.com/mybatis/mybatis-dynamic-sql/blob/master/src/site/markdown/docs/configuration.md

[Roderland]

@jeffgbutler @jesusslim
I think yes, there is no owning context for the statements, So I usually handle it myself.
Here are some examples, I am using the old version below:

        /* [1] this is a general query case:
            I need to query all the data because the user has no input criteria
         */
        select(c -> c.where(age, isInWhenPresent(request.getAgeList()))); // I think it works well!

        /* [2] this is a preQuery case:
            I filter some username by a condition (In most cases, it is filtered first in another table),
            after filtering, the matching usernameList is empty, so no data should be queried in the end
         */
        List<String> usernameList = filterUsernameBySomeConditions(request.getUsernameList());
        select(c -> c.where(username, isInWhenPresent(usernameList))); // not as expected！

        /* [3] In fact, a mixture of the two */
        List<Integer> ageList = request.getAgeList();
        List<String> usernameList = filterUsernameBySomeConditions(request.getUsernameList());
        select(c -> c.where(age, isInWhenPresent(ageList)).and(username, isInWhenPresent(usernameList))); 
        // ... so what do you think it is

        /* So I usually handle it myself */
        List<Integer> ageList = request.getAgeList();
        List<String> usernameList = filterUsernameBySomeConditions(request.getUsernameList());
        if (CollectionUtils.isEmpty(usernameList)) {
            return ;
        }
        select(c -> c.where(age, isInWhenPresent(ageList)).and(username, isInWhenPresent(usernameList)));

[jeffgbutler]

@Roderland Thanks for the excellent writeup!

Starting in version 1.4.1, your case [1] will throw an exception if ageList is empty. In this case you do want to query all rows in the table so you will need to configure the statement to allow the non-rendering where clause:

select(c -> c.where(age, isInWhenPresent(request.getAgeList()))
   .configureStatement(cfg -> cfg.setNonRenderingWhereClauseAllowed(true)))

Also in version 1.4.1, your case [2] will throw an exception if usernameList is empty. In this case I think you don't want to query all rows in the table so the exception may be appropriate. But your case [3] is the best way to handle it.

Your case [3] is a good example of doing parameter validation before calling the query - that is always the best thing to do. With that case you do not need to configure the statement to allow a non-rendering where clause because you check for values in usernameList. Even if ageList is empty, the where clause will still render.

[jeffgbutler]

This change is now released in version 1.4.1. Thanks for all the feedback.