Multiple and nullable list arguments break code-gen
tamimattafi opened this issue · 5 comments
tamimattafi commented
Problem
- Using lists multiple times inside a query breaks code-gen
- Nullable lists are not handled when creating query arguments using
createArguments
Example
Here's an example of a query having lists as arguments, which are used many times in different places, and are nullable
@Query("""
SELECT * FROM SimpleEntity
WHERE(:types IS NULL OR type IN :types)
AND (:statuses IS NULL OR status IN :statuses)
AND (:startAt is NULL OR createdAt >= :startAt)
AND (:endAt IS NULL OR createdAt <= :endAt)
AND id IN (
SELECT parentId FROM SampleParticipantEntity
WHERE (:roles IS NULL OR type IN :roles) AND self = 1
)
ORDER BY createdAt DESC LIMIT :limit
""")
suspend fun getSampleCompoundsReactive(
statuses: List<String>?,
types: List<String>?,
roles: List<String>?,
startAt: Int?,
endAt: Int?,
limit: Int
): Flow<List<SampleCompound>>
This will generate the following code:
val types = types.orEmpty()
val typesIndexes = createArguments(types.size)
val types = types.orEmpty()
val typesIndexes = createArguments(types.size)
val statuses = statuses.orEmpty()
val statusesIndexes = createArguments(statuses.size)
val statuses = statuses.orEmpty()
val statusesIndexes = createArguments(statuses.size)
val roles = roles.orEmpty()
val rolesIndexes = createArguments(roles.size)
val roles = roles.orEmpty()
val rolesIndexes = createArguments(roles.size)
Here we see two issues:
types
andtypesIndexes
are doubledcreateArguments
will never returnNULL
, which is important for the correctness of queries like:types IS NULL OR type IN :types
tamimattafi commented
The issue could be reproduced inside the sample using the following query:
@Query("SELECT * FROM BackPackEntity WHERE(:studentIds IS NULL OR studentId IN :studentIds)")
suspend fun getStudentBackPacks(
studentIds: List<String>?
): Flow<List<BackPackEntity>>
The generated code looks like this:
override fun <R> execute(mapper: SqlCursor.() -> QueryResult<R>): QueryResult<R> {
val studentIds = studentIds.orEmpty()
val studentIdsIndexes = createArguments(studentIds.size)
val studentIds = studentIds.orEmpty()
val studentIdsIndexes = createArguments(studentIds.size)
val kabinQuery =
"""SELECT * FROM BackPackEntity WHERE($studentIdsIndexes IS NULL OR studentId IN $studentIdsIndexes)"""
val kabinParametersCount = studentIds.size + studentIds.size + 0
val result = driver.executeQuery(
-1664847722,
kabinQuery,
mapper,
kabinParametersCount
)
{
studentIds.forEachIndexed { index, studentIdsChild ->
bindString(index, studentIdsChild)
}
studentIds.forEachIndexed { index, studentIdsChild ->
bindString(index + studentIds.orEmpty().size, studentIdsChild)
}
}
return result
}
tamimattafi commented
Generated code for such case should and would look like this:
override fun <R> execute(mapper: SqlCursor.() -> QueryResult<R>): QueryResult<R> {
val studentIdsSize = studentIds.orEmpty().size
val studentIdsIndexes = createNullableArguments(studentIds?.size)
val kabinQuery =
"""SELECT * FROM BackPackEntity WHERE($studentIdsIndexes IS NULL OR studentId IN $studentIdsIndexes)"""
val kabinParametersCount = studentIdsSize + studentIdsSize + 0
val result = driver.executeQuery(
261282966,
kabinQuery,
mapper,
kabinParametersCount
)
{
studentIds?.forEachIndexed { index, studentIdsChild ->
bindString(index, studentIdsChild)
}
studentIds?.forEachIndexed { index, studentIdsChild ->
bindString(index + studentIdsSize, studentIdsChild)
}
}
return result
}
tamimattafi commented
Fixed in 0.1.0-pre-alpha03
Further testing will be made in real environment
tamimattafi commented
The generated queries, which look like this, have invalid SQL syntax:
NULL IS NULL OR type IN NULL
(?, ?) IS NULL OR type IN (?, ?)
- The
IN
operator can't be called withNULL
- (?, ?) can't be checked as
NULL
and will break parameters count and indexes
The generated queries should look like this:
NULL IS NULL OR type IN ()
:anyNotNullValue IS NULL OR type IN (?, ?)
tamimattafi commented
Testing in a real environment resulted in a success, therefore, this issue is considered solved.
Changes were published to 0.1.0-pre-alpha04