Properties .eq with null parameter causes crash

[DOFandersolsen]

Hello!

I have a query with the following code that tries to find an existing Observation with the matching fields, some of which may be null (SecondaryBehaviourId and DirectionId are often null, since the minimum requirement is PrimaryBehaviourId)

return dao.queryBuilder()
                //Id must not match
                .where(ObservationDao.Properties.Id.notEq(observationId))
                //But the rest must match
                .where(ObservationDao.Properties.CollectionId.eq(collectionId))
                .where(ObservationDao.Properties.PrimaryBehaviourId.eq(primaryBehaviourId))
                .where(ObservationDao.Properties.SecondaryBehaviourId.eq(secondaryBehaviourId))
                .where(ObservationDao.Properties.DirectionId.eq(directionId))
                .build()
                .unique();

However, when I run the code, I get the following error:

java.lang.IllegalArgumentException: the bind value at index 9 is null
                      at android.database.sqlite.SQLiteProgram.bindString(SQLiteProgram.java:164)
                      at android.database.sqlite.SQLiteProgram.bindAllArgsAsStrings(SQLiteProgram.java:200)
                      at android.database.sqlite.SQLiteDirectCursorDriver.query(SQLiteDirectCursorDriver.java:47)
                      at android.database.sqlite.SQLiteDatabase.rawQueryWithFactory(SQLiteDatabase.java:1316)
                      at android.database.sqlite.SQLiteDatabase.rawQuery(SQLiteDatabase.java:1255)
                      at org.greenrobot.greendao.database.StandardDatabase.rawQuery(StandardDatabase.java:32)
                      at org.greenrobot.greendao.query.Query.unique(Query.java:129)

From what I can tell, it is because .eq does not handle null-parameters.

My suggestion would be either:

• Check for null and throw an InvalidArgumentException if a null value is passed to .eq() in https://github.com/greenrobot/greenDAO/blob/master/DaoCore/src/main/java/org/greenrobot/greendao/Property.java Throwing an exception shows the developer that this function does not work with nulls, and the developer should ensure proper checking before quering.
• Change the .eq() function to check for null and call .isNull() if the value is a null, else use the existing code This eases the job of the developer but could cause problems because the developer doesn't notice he/she might be using wrong parameters.

[derentenpopel]

I've run into a similar problem and would really like @DOFandersolsen second approach.

[greenrobot-team]

Yeap, should probably update docs or throw as suggested.

@derentenpopel Just call .isNull() instead of .eq(value) if you see that value == null. -ut

[DOFandersolsen]

Wouldn't it be possible to do something like this?

https://github.com/greenrobot/greenDAO/blob/master/DaoCore/src/main/java/org/greenrobot/greendao/Property.java

/** Creates an "equal ('=')" condition  for this property. */
    public WhereCondition eq(Object value) {
        if (value == null) {
            return isNull();
        }
        return new PropertyCondition(this, "=?", value);
}

/** Creates an "not equal ('<>')" condition  for this property. */
    public WhereCondition notEq(Object value) {
        if (value == null) {
            return isNotNull();
        }
        return new PropertyCondition(this, "<>?", value);
}

[derentenpopel]

@greenrobot-team

Just call .isNull() instead of .eq(value) if you see that value == null

I have a setup similar to this and every other run ObjB.getA() returns null.

Query<ObjA> query = queryBuilder.where(
    ObjADao.Properties.ParamA.eq(null),
    ObjADao.Properties.ParamB.eq(null),
    ObjADao.Properties.ParamC.eq(null),
    ObjADao.Properties.ParamD.eq(null),
).build();

for (ObjB current : someArray) {
    query.setParameter(0, current.getA());
    query.setParameter(1, current.getB());
    query.setParameter(2, current.getC());
    query.setParameter(3, current.getD());

    ObjA obj = query.unique();

    //Do something with obj
}

So @DOFandersolsen suggestion would really make things easier for me.

[greenrobot-team]

While creating a fix as suggested by @DOFandersolsen I noticed that auto use of isNull/isNotNull is not done because it might break a query.setParameter() pattern. An edited example from our tests:

if (queryTemplate == null) {
    QueryBuilder<ToManyTargetEntity> queryBuilder = queryBuilder();
    queryBuilder.where(Properties.TargetJoinProperty.eq(null));
    queryTemplate = queryBuilder.build();
}
Query<ToManyTargetEntity> query = queryTemplate.forCurrentThread();
query.setParameter(0, targetJoinProperty);
return query.list();

Notice the template using a null value, then actually setting a value before executing the query. Auto-changing to isNull/isNotNull or throwing might break existing code that uses the above pattern. ☹️ -ut

[DOFandersolsen]

Hmm, that is a bummer..

I have an alternative idea that would not handle nulls automatically but instead tell the developer that he's doing something wrong instead of throwing a raw IllegalArgumentException.

The DAO should do a null-check when executing the Query's list-functions. If one of the parameters are null, then throw a NullPointerException if there is a .eq(...) / .neq(...) with a parameter that has not been changed from null to <Something>

This NullPointerException could have a more descriptive message like "Null-values cannot be used with parameter-based WhereConditions"

However, we're very close to being back to the original java.lang.IllegalArgumentException: the bind value at index 9 is null message, but this would still tell the developer that he/she is doing something that should NOT be done, and it's not the SQLite system that is messing up.

I can fully understand if you decide to drop the issue now that we can't handle it automatically, but I appreciate you taking the time to look into a possible solution :-)