Update method code generation is broken for variation of OnConflictStrategy [176138543]

Fixed

Bug

Status Update

No update yet.

Description

se...@gmail.com

created issue #1

Dec 23, 2020 12:24PM

Component used: Room

Version used: 2.3.0-alpha04

Devices/Android versions reproduced on: Emulator API 26 with google services

Case 1: Consider Base DAO class

abstract class BaseDao<Entity : ScopedEntity>(val tableName: String) {
    @Update(onConflict = OnConflictStrategy.ABORT)
    abstract suspend fun update(entity: Entity): Int

    @Update(onConflict = OnConflictStrategy.REPLACE)
    abstract suspend fun updateOrReplace(entity: Entity): Int
  
    @Update(onConflict = OnConflictStrategy.IGNORE)
    abstract suspend fun updateOrIgnore(entity: Entity): Int

    // Insert and delete methods go below...
}

And class

@Dao
abstract class CompanyDao : BaseDao<CompanyDao>(CompanyDao.TABLE_NAME)

It is expected that the generated implementation code will have 3 Update methods with corresponding conflict strategies.

What is actually generated:

public final class CompanyDao_Impl extends CompanyDao {
  private final RoomDatabase __db;

  private final EntityInsertionAdapter<CompanyEntity> __insertionAdapterOfCompanyEntity;

  private final EntityInsertionAdapter<CompanyEntity> __insertionAdapterOfCompanyEntity_1;

  private final EntityInsertionAdapter<CompanyEntity> __insertionAdapterOfCompanyEntity_2;

  private final EntityDeletionOrUpdateAdapter<CompanyEntity> __deletionAdapterOfCompanyEntity;

  private final EntityDeletionOrUpdateAdapter<CompanyEntity> __updateAdapterOfCompanyEntity;

  private final SharedSQLiteStatement __preparedStmtOfDeleteCompanyByLocalId;

  //...Other code
 this.__updateAdapterOfCompanyEntity = new EntityDeletionOrUpdateAdapter<CompanyEntity>(__db) {
      @Override
      public String createQuery() {
        return "UPDATE OR ABORT `Company` SET `name` = ?,`local_id` = ?,`server_id` = ? WHERE `local_id` = ?";
      }

      @Override
      public void bind(SupportSQLiteStatement stmt, CompanyEntity value) {
        if (value.getName() == null) {
          stmt.bindNull(1);
        } else {
          stmt.bindString(1, value.getName());
        }
        stmt.bindLong(2, value.getLocalId());
        if (value.getServerId() == null) {
          stmt.bindNull(3);
        } else {
          stmt.bindLong(3, value.getServerId());
        }
        stmt.bindLong(4, value.getLocalId());
      }
    };

 @Override
  public Object update(final CompanyEntity entity, final Continuation<? super Integer> arg1) {
    return CoroutinesRoom.execute(__db, true, new Callable<Integer>() {
      @Override
      public Integer call() throws Exception {
        int _total = 0;
        __db.beginTransaction();
        try {
          _total +=__updateAdapterOfCompanyEntity.handle(entity);
          __db.setTransactionSuccessful();
          return _total;
        } finally {
          __db.endTransaction();
        }
      }
    }, arg1);
  }

  @Override
  public Object updateOrReplace(final CompanyEntity entity,
      final Continuation<? super Integer> arg1) {
    return CoroutinesRoom.execute(__db, true, new Callable<Integer>() {
      @Override
      public Integer call() throws Exception {
        int _total = 0;
        __db.beginTransaction();
        try {
          _total +=__updateAdapterOfCompanyEntity.handle(entity);
          __db.setTransactionSuccessful();
          return _total;
        } finally {
          __db.endTransaction();
        }
      }
    }, arg1);
  }

  @Override
  public Object updateOrIgnore(final CompanyEntity entity,
      final Continuation<? super Integer> arg1) {
    return CoroutinesRoom.execute(__db, true, new Callable<Integer>() {
      @Override
      public Integer call() throws Exception {
        int _total = 0;
        __db.beginTransaction();
        try {
          _total +=__updateAdapterOfCompanyEntity.handle(entity);
          __db.setTransactionSuccessful();
          return _total;
        } finally {
          __db.endTransaction();
        }
      }
    }, arg1);
  }

Actual - all update methods are using the same adapter with ABORT.

Case 2: Change very first update strategy to IGNORE in BaseDao

  ...
  @Update(onConflict = OnConflictStrategy.IGNORE)
  abstract suspend fun update(entity: Entity): Int
  ...

And now adapter becomes:

this.__updateAdapterOfCompanyEntity = new EntityDeletionOrUpdateAdapter<CompanyEntity>(__db) {
      @Override
      public String createQuery() {
        return "UPDATE OR IGNORE `Company` SET `name` = ?,`local_id` = ?,`server_id` = ? WHERE `local_id` = ?";
      }

      @Override
      public void bind(SupportSQLiteStatement stmt, CompanyEntity value) {
        if (value.getName() == null) {
          stmt.bindNull(1);
        } else {
          stmt.bindString(1, value.getName());
        }
        stmt.bindLong(2, value.getLocalId());
        if (value.getServerId() == null) {
          stmt.bindNull(3);
        } else {
          stmt.bindLong(3, value.getServerId());
        }
        stmt.bindLong(4, value.getLocalId());
      }
    };

Case 3: Use case 2 and add a new method to ComanyDao:

    @Update(onConflict = OnConflictStrategy.REPLACE)
    abstract fun testUpdate(entity: CompanyEntity)

then update implementation becomes

    this.__updateAdapterOfCompanyEntity = new EntityDeletionOrUpdateAdapter<CompanyEntity>(__db) {
      @Override
      public String createQuery() {
        return "UPDATE OR REPLACE `Company` SET `name` = ?,`local_id` = ?,`server_id` = ? WHERE `local_id` = ?";
      }

      @Override
      public void bind(SupportSQLiteStatement stmt, CompanyEntity value) {
        if (value.getName() == null) {
          stmt.bindNull(1);
        } else {
          stmt.bindString(1, value.getName());
        }
        stmt.bindLong(2, value.getLocalId());
        if (value.getServerId() == null) {
          stmt.bindNull(3);
        } else {
          stmt.bindLong(3, value.getServerId());
        }
        stmt.bindLong(4, value.getLocalId());
      }
    };

@Override
  public void testUpdate(final CompanyEntity entity) {
    __db.assertNotSuspendingTransaction();
    __db.beginTransaction();
    try {
      __updateAdapterOfCompanyEntity.handle(entity);
      __db.setTransactionSuccessful();
    } finally {
      __db.endTransaction();
    }
  }

  @Override
  public Object update(final CompanyEntity entity, final Continuation<? super Integer> arg1) {
    return CoroutinesRoom.execute(__db, true, new Callable<Integer>() {
      @Override
      public Integer call() throws Exception {
        int _total = 0;
        __db.beginTransaction();
        try {
          _total +=__updateAdapterOfCompanyEntity.handle(entity);
          __db.setTransactionSuccessful();
          return _total;
        } finally {
          __db.endTransaction();
        }
      }
    }, arg1);
  }

  @Override
  public Object updateOrReplace(final CompanyEntity entity,
      final Continuation<? super Integer> arg1) {
    return CoroutinesRoom.execute(__db, true, new Callable<Integer>() {
      @Override
      public Integer call() throws Exception {
        int _total = 0;
        __db.beginTransaction();
        try {
          _total +=__updateAdapterOfCompanyEntity.handle(entity);
          __db.setTransactionSuccessful();
          return _total;
        } finally {
          __db.endTransaction();
        }
      }
    }, arg1);
  }

  @Override
  public Object updateOrIgnore(final CompanyEntity entity,
      final Continuation<? super Integer> arg1) {
    return CoroutinesRoom.execute(__db, true, new Callable<Integer>() {
      @Override
      public Integer call() throws Exception {
        int _total = 0;
        __db.beginTransaction();
        try {
          _total +=__updateAdapterOfCompanyEntity.handle(entity);
          __db.setTransactionSuccessful();
          return _total;
        } finally {
          __db.endTransaction();
        }
      }
    }, arg1);
  }

Comments

da...@google.com <da...@google.com> #2Dec 23, 2020 05:45PM

A couple of questions:

1. Have you saw crash in real device or only in simulators?
2. Do you use dynamic feature for language ID?

se...@gmail.com <se...@gmail.com> #3Dec 24, 2020 01:59PM

Tested on Android 12 Emulator with custom executor, but cannot repro this issue.

da...@google.com <da...@google.com> Dec 28, 2020 05:33PM

Reassigned to el...@google.com.

ap...@google.com <ap...@google.com> #4Jan 12, 2021 03:06PM

Second crash in the description is from a real device. Experienced it myself on two different Xiaomi phones, plus lots of crashes from users in the Google Play console.
Dynamic features are not used in the application.

As a wild guess, I have downgraded build tools from 31.0.0 to 30.0.3, compileSdk from 31 to 30, and moved all work with Language ID to the service in a separate process (just to be sure that crash can kill secondary process instead of main). This combination is in beta for 2 days by now and I don't see any SIGSEGV crashes.

el...@google.com <el...@google.com> Jan 12, 2021 03:06PM

Marked as fixed.

Issue 176138543