when i use isIn(emptyList) where condition, it would not render to sql

[clcy1243]

code example

select(s -> s.where(id, isIn(new ArrayList<>()));

will render like this

select * from table

it should be like

select * from table where id in ()

and mysql where throw exception when execute this sql, it's what my want,
so i think there is a bug,
i found this code at org.mybatis.dynamic.sql.where.render.WhereConditionVisitor<T>

    public Optional<FragmentAndParameters> visit(AbstractListValueCondition<T> condition) {
        FragmentCollector fc = (FragmentCollector)condition.mapValues(this::toFragmentAndParameters).collect(FragmentCollector.collect());
        return fc.isEmpty() ? Optional.empty() : FragmentAndParameters.withFragment(condition.renderCondition(this.columnName(), fc.fragments())).withParameters(fc.parameters()).buildOptional();
    }

if list value condition accept an empty list, fc.isEmpty() will be true, this condition where be deleted

could anyone fix this?

[jeffgbutler]

I don't agree this is a bug - I think it is actually a good thing that the library won't generate invalid SQL. I think it would be best to check to see if the list is empty before trying to execute the statement. If it is a possibility in your app that the list could be empty, then do you really want to rely on causing a database exception as a kind of validation?

Honestly it wouldn't be too hard to provide an option to render these conditions even when they are empty. But I'm not convinced that is a good idea.

Thoughts???

[clcy1243]

I don't agree this is a bug - I think it is actually a good thing that the library won't generate invalid SQL. I think it would be best to check to see if the list is empty before trying to execute the statement. If it is a possibility in your app that the list could be empty, then do you really want to rely on causing a database exception as a kind of validation?

Honestly it wouldn't be too hard to provide an option to render these conditions even when they are empty. But I'm not convinced that is a good idea.

Thoughts???

if i want ignore invalid condition, i will use IsInWhenPresent but not IsIn, i think there is IsInWhenPresent method, so IsIn mean it will render this condition even if it is invalid

[clcy1243]

there is two method do same thing with diff name, may be there should has one method named IsInRequired?

[jeffgbutler]

IsInWhenPresent filters null values out of the list. It will still not render if the list ends up being empty.

[clcy1243]

with 1.3.7 generator, i use Example, when i use andIdIn(emptyList), will throw exception,
so i should change my code to use mybatis-dynamic-sql,
when i use isIn , i should valid my param

[clcy1243]

there is a problem, when i build sql with not valid param, it where change more data than what i want it modify,
and i should check param each step now

[jeffgbutler]

I get it. I really do think you should be validating, but I understand it could be a change to how you're working now and not doing it would cause undesirable side effects.

I've been experimenting a bit. I can enable you to write a condition that will force the exception with a very simple code change. I'll push something to address this shortly.

[clcy1243]

Thanks for understanding

[clcy1243]

Here is 4:00am, I will try it when I waked up.

[jeffgbutler]

See documentation here: https://github.com/mybatis/mybatis-dynamic-sql/blob/master/src/site/markdown/docs/conditions.md#optionality-with-the-in-conditions

I will release the new version in the next few days.

[clcy1243]

i code like this

    public static <T> IsIn<T> isInRequired(Collection<T> values) {
        if (CollectionUtils.isEmpty(values)) {
            throw new ZhqcException(WmsResponseEnum.CUSTOMIZE_MSG, "param could not be empty at where in condition");
        }
        return IsIn.of(values);
    }

to fix my codes now.

ur idea is so good, I don't try to expland where codition before,
I think i can use mybatis-dynamic-sql do more amazing things than before.

[jeffgbutler]

@clcy1243 I like your solution a lot! This is much better than forcing the library to generate invalid SQL. I'm going to remove the change I made and use your example in the docs.

[zhenchuan9r]

I would like to view this as a bug.
It is very contradicting normal understanding of the declaration of the where clause.
It is at least a very unfriendly design.
But thanks for changing the design to a normal track.

[jeffgbutler]

@zhenchuan9r If you have found a bug, please open a new issue. It is ineffective to comment on an issue that has been closed for three years.

[zhenchuan9r]

@zhenchuan9r If you have found a bug, please open a new issue. It is ineffective to comment on an issue that has been closed for three years.

I encountered the same problem as this issue.
As this issue has already been resolved, it is not necessary for me to open a new issue.