updateHook row is always empty for insert & update

[Zoxive]

Describe the bug In previous versions of op-sqlite updateHook would contain the row of the insert or update.

The notion documentation talks about it here: https://ospfranco.notion.site/API-1a39b6bb3eb74eb893d640c8c3459362#6983c7e0a73e4f219b560e3434ca9faf

// Bear in mind: rowId is not your table primary key but the internal rowId sqlite uses
// to keep track of the table rows
db.updateHook(({ rowId, table, operation, row = {} }) => {
  console.warn(`Hook has been called, rowId: ${rowId}, ${table}, ${operation}`);
  // Will contain the entire row that changed
  // only on UPDATE and INSERT operations
  console.warn(JSON.stringify(row, null, 2));
});

But it seems to always be an empty object now in 9.1.2+ (im not sure what version caused the regression.. i actually just updated from 2.0.7 -> 9.1.2)

I'm actually ok with the behavior.. i can query the row myself via the table + rowId. The only heartache is that because the apis are all async now, i have do a little refactoring on my end (again not that bad).

I'm opening this to either.

1. Confirm the change is intended, and get the docs updated or
2. point out the regression

Versions:

• OS and version: android 14
• op-sqlite version: 9.1.2
• RN version: 0.74.5

Reproducible example

let db: DB;

const DB_CONFIG = {
  name: "op-sqlite",
};

export function getPromiseResolve<T>() {
    // biome-ignore lint/style/noNonNullAssertion: <explanation>
    let promiseResolve: ((input: T) => void) = null!;
    const promise = new Promise<T>((resolve) => {
        promiseResolve = resolve;
    });

    return { promise, promiseResolve };
}

describe("op-sqlite-raw", () => {
  beforeEach(async () => {
    try {
      if (db) {
        db.close();
        db.delete();
      }

      db = open(DB_CONFIG);

      await db.execute("DROP TABLE IF EXISTS User;");
      await db.execute(
        "CREATE TABLE User (id INT PRIMARY KEY, name TEXT NOT NULL, age INT, networth REAL) STRICT;"
      );
    } catch (e) {
      console.warn("error on before each", e);
    }
  });

  afterAll(() => {
    db.close();
    db.delete();
  });

  it("update hook, INSERT", async () => {
    const { promise, promiseResolve } = getPromiseResolve<{
      operation: string;
      table: string;
      row: unknown;
    }>();

    db.updateHook(({ operation, row, rowId, table }) => {
      // console.warn(
      //   `Hook has been called, rowId: ${rowId}, ${table}, ${operation}`,
      //   JSON.stringify(row, null, 2)
      // );
      promiseResolve?.({ operation, table, row });
    });

    await db.execute(
      'INSERT INTO "User" (id, name, age, networth) VALUES(?, ?, ?, ?)',
      [1, "bob", 76, 523.02]
    );

    const hookResult = await promise;

    expect(hookResult).toEqual({
      operation: "INSERT",
      table: "User",
      row: {}, // expected?
    });
  });

  it("update hook, UPDATE", async () => {
    const { promise, promiseResolve } = getPromiseResolve<{
      operation: string;
      table: string;
      row: unknown;
    }>();

    db.updateHook(({ operation, row, rowId, table }) => {
      if (operation === "UPDATE") {
        // console.warn(
        //   `Hook has been called, rowId: ${rowId}, ${table}, ${operation}`,
        //   JSON.stringify(row, null, 2)
        // );
        promiseResolve?.({ operation, table, row });
      }
    });

    await db.execute(
      'INSERT INTO "User" (id, name, age, networth) VALUES(?, ?, ?, ?)',
      [1, "bob", 76, 523.02]
    );

    await db.execute(
      'UPDATE "User" SET age = ?, networth = ? WHERE id = ?',
      [77, 1111.11, 1]
    );

    const hookResult = await promise;

    expect(hookResult).toEqual({
      operation: "UPDATE",
      table: "User",
      row: {}, // expected?
    });
  });
});

[Zoxive]

Ok. Something fishy is going on. Just refactored my code to query the row myself.. and the rowId that is given is not right.

Test updated to check rowId:

it("update hook, INSERT", async () => {
  const { promise, promiseResolve } = getPromiseResolve<{ operation: string; table: string; row: unknown; rowId: number; }>();

  db.updateHook(({ operation, row, rowId, table }) => {
    // console.warn(
    //   `Hook has been called, rowId: ${rowId}, ${table}, ${operation}`,
    //  JSON.stringify(row, null, 2)
    // );
    promiseResolve?.({ operation, table, rowId, row });
  });

  await db.execute(
    'INSERT INTO "User" (id, name, age, networth) VALUES(?, ?, ?, ?)',
    [1, 'bob', 76, 523.02],
  );

  const hookResult = await promise;

  const query = await db.execute('SELECT rowId FROM "User" WHERE id = ?', [1]);
  // biome-ignore lint/style/noNonNullAssertion: <explanation>
  const rowId = query.rows![0];

  expect(hookResult).toEqual({ operation: "INSERT", table: "User", rowId, row: {} });
});

Locally, im getting rowId: 8 vs (expected) rowId: 1.

Edit: Quite the Heisenbug.

Sometimes the rowId is right, but it really likes to return 8 as the first rowId in the hook. (If i spam refresh my test app.. sometimes the test passes.. but its rare)

Ive also seen it return a rowId like 108, when my max in that table is 23.

[Zoxive]

More context after playing around today.. I decided to revert to before the reactiveQuery changes, which was 6.0.1

The row {} object is now populated for "UPDATE" | "INSERT" operations. Yay.

But the same rowid weirdness exists. The given rowid from the updateHook does not match the rowid from the row that my test changed. Im guessing theres some conversion problem from c++ -> native -> JS land. (But at the time c++ receives it and queries the db to create the row obj its fine there.. in 6.0.1 at least..) I dont even need the rowid now that the row obj is there, but it I kinda expected them to match.

[ospfranco]

You are going to have to wait until I have time to take a look at this, I'm currently changing cities and next flying to a conference in Asia, so it might take some time. Or you can take a look at the C++ code yourself.

[ospfranco]

I just took a look and the issue might be the same as #128. Basically executing any operation inside of the update hook callback is a undefined behavior/buggy behavior. I've removed the row from the result and it should work properly now I hope. Give 9.2.0 a try. This means to get the row you will have to query for the row manually outside of the hook. I've also updated the docs to reflect the row is no longer returned.

[ospfranco]

Going to close this, if you are still facing issues feel free to comment and I will take a look