tamimattafi/kabin

Multiple and nullable list arguments break code-gen

tamimattafi opened this issue · 5 comments

Problem

  1. Using lists multiple times inside a query breaks code-gen
  2. 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:

  1. types and typesIndexes are doubled
  2. createArguments will never return NULL, which is important for the correctness of queries like :types IS NULL OR type IN :types

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
    }

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
    }

Fixed in 0.1.0-pre-alpha03
Further testing will be made in real environment

The generated queries, which look like this, have invalid SQL syntax:

NULL IS NULL OR type IN NULL
(?, ?) IS NULL OR type IN (?, ?)
  1. The IN operator can't be called with NULL
  2. (?, ?) 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 (?, ?)

Testing in a real environment resulted in a success, therefore, this issue is considered solved.
Changes were published to 0.1.0-pre-alpha04