Skip to content

Conversation

@Jei-sKappa
Copy link

Closes #20.


Notes:
I also started working on adding a test case to sqlite_crdt.
Here’s what I’ve done so far:

    test('Merge changeset from other crdt', () async {
      final otherCrdt = await SqliteCrdt.openInMemory();
      await otherCrdt.execute('''
        CREATE TABLE users (
          id INTEGER NOT NULL,
          name TEXT,
          PRIMARY KEY (id)
        )
      ''');
      await insertUser(otherCrdt, 1, 'John Doe');

      final hlc = (await otherCrdt.query('SELECT * FROM users')).first['hlc'];

      final changeset = await otherCrdt.getChangeset();
      expect(changeset['users']!.first['id'], 1);
      expect(changeset['users']!.first['name'], 'John Doe');
      expect(changeset['users']!.first['hlc'], isA<String>());

      await crdt.merge(changeset);
      final result = await crdt.query('SELECT * FROM users');
      expect(result.first['name'], 'John Doe');
      expect(result.first['hlc'], hlc);
    });

However, when running this (with the updated crdt package), I get the following error:

  Unsupported operation: read-only
  package:sqflite_common/src/collection_utils.dart 156:5  QueryRow.[]=
  package:sql_crdt/src/sql_crdt.dart 165:17               SqlCrdt.merge.<fn>
  package:sqlite_crdt/src/sqlite_api.dart 32:12           ExecutorApi.executeBatch
  package:sql_crdt/src/sql_crdt.dart 155:15               SqlCrdt.merge
  ===== asynchronous gap ===========================
  test/sql_crdt_test.dart 187:7                           runSqlCrdtTests.<fn>.<fn>
  
  Bad state: No element
  dart:collection                 ListBase.first
  test/sql_crdt_test.dart 189:21  runSqlCrdtTests.<fn>.<fn>

This happens because the query method in the ExecutorApi class returns a QueryRow object, which is "immutable" and cannot be modified.

I came up with two possible solutions:

1) Update ExecutorApi’s query method to copy the row maps before returning them

class ExecutorApi extends DatabaseApi {
  @override
  Future<List<Map<String, Object?>>> query(String sql, [List<Object?>? args]) =>
      _db.rawQuery(sql, args).then((r) => r.map((e) => Map<String, Object?>.from(e)).toList());
  //  _db.rawQuery(sql, args); // instead of this
}

This ensures the returned data is mutable.
I don’t think this is the cleanest solution, but it works.

2) Update SqlCrdt's merge method to copy the record before updating it:

Future<void> merge(CrdtChangeset changeset) async {
  /* ... */

  await _db.executeBatch((executor) async {
    for (final entry in changeset.entries) {
      final table = entry.key;
      final records = entry.value;
      final keys = tableKeys[table]!.join(', ');

      for (final r in records) { // previously: for (final record in records) {
        final record = Map.from(r); // <-- add this line

        // Extract node id from the record's hlc
        record['node_id'] = (record['hlc'] is String // <-- This is causing the main issue
                ? (record['hlc'] as String).toHlc
                : (record['hlc'] as Hlc))
            .nodeId;
        // Ensure hlc and modified are strings
        record['hlc'] = record['hlc'].toString();
        record['modified'] = hlc.toString();

        /* ... */
      }
    }
  });

  /* ... */
}

Let me know which approach you prefer.
Also, should I file a separate issue for this problem?

@Jei-sKappa
Copy link
Author

Just to clarify - this isn’t a regression from this PR, but rather something we didn’t have a test for yet: merging between two sqlite_crdt instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: can't merge changeset

1 participant