stmt.Identifier.In removes WHERE clause if value is nil or empty slice

[yansal]

This is not really a bug report, it's more a feedback about a behavior that was unexpected and caused a potentially harmful bug.

To reproduce:

lk.Select("columns").From("table").Where(lk.Condition("column").In(ids))

If ids is a nil or empty slice, loukoum builds a query with no WHERE clause, that is, it will select all rows in the table, while the user almost certainly wants to select zero row.

[yansal]

A few more points:

1. There is the same behavior with UPDATE and DELETE, not just SELECT
2. There is the same behavior if the IN is coupled with an AND or an OR: the WHERE clause disappear.

lk.Select("column").From("table").Where(lk.Condition("first").In(empty).And(lk.Condition("second").Equal(1)))

generates the following SQL:

SELECT column FROM table

[novln]

You can fix this behavior here: https://github.com/ulule/loukoum/blob/dev/stmt/expression.go#L140

[icholy]

I think it's a problem with terminology. With the current convention, when an Expression.IsEmpty returns true, then that part of the query expression is omitted. Array.IsEmpty returns true when it contains zero elements, but it should still be emitted. A quick fix here is to have Array.IsEmpty always return true, but this is confusing. I propose we rename Expression.IsEmpty Expression.IsNil. Then change Array.IsNil to always return false.

[icholy]

Actually, that won't help because this is not a valid query:

SELECT column FROM table WHERE (ID IN ())

Would a panic be the right thing to do here?

edit or maybe we just generated the invalid query and let the user deal with it?

[yansal]

lk.Select("columns").From("table").Where(lk.Condition("column").In(nil))

panics with panic: cannot use {<nil>}[<nil>] as loukoum Expression.

My 2 cents: in our case, I believe a panic or an invalid sql query would be better than a valid sql query without a WHERE clause.

[icholy]

Something like this then

// In performs a "in" condition.
func (identifier Identifier) In(value ...interface{}) In {
	array := NewArrayExpression(value...)
	if array.IsEmpty() {
		panic("loukoum: cannot have empty in expression")
	}
	return NewIn(identifier, array)
}

Or simply change the Array type.

// IsEmpty returns true if statement is undefined.
func (array Array) IsEmpty() bool {
	return false
}