simolus3 / drift

Drift is an easy to use, reactive, typesafe persistence library for Dart & Flutter.
https://drift.simonbinder.eu/
MIT License
2.54k stars 360 forks source link

Support storing arbitrary dart types as JSONB BLOBs with configuration support within .drift files #2841

Open dvoloshyn opened 7 months ago

dvoloshyn commented 7 months ago

Problem I have the following column definition in .drift file:

dependencies   TEXT    NOT NULL MAPPED BY `const JsonArrayConverter<int>()`

It allows to store dart List as json TEXT in the sqlite database. Sqlite 3.45.0 added support for serializing internal json parse tree (JSONB) as BLOB to avoid parsing it. I want to be able to store the dart list as JSONB BLOB.

Suggested solution Introduce a new JSONB type that could be used within .drift files so that example above is translated to

dependencies   JSONB    NOT NULL MAPPED BY `const JsonArrayConverter<int>()`

JSONB type should ensure that appropriate json functions (json()/jsonb()) are added to the SQL queries to translate between text and JSONB BLOB. MAPPED BY converted should still receive TEXT payload from database because JSONB BLOB format is not intended to be used from the application side.

We could also take it a step further and implement support similarly to enums:

dependencies   JSONB(List<int>)    NOT NULL

In this case JSONB signifies the JSONB BLOB on-disk format and List<int> specifies the dart field type. With such a solution we also avoid the need to write a custom TypeConverter.

Other suggestions are welcome.

simolus3 commented 7 months ago

I agree that we should have JSONB support. Using a type for this is kind of tricky because it requires things type converters are not yet able to do - altering the generated query.

For instance, if a user writes INSERT INTO tbl (dependencies) VALUES (?) in a drift file, we need to turn that query into INSERT INTO tbl (dependencies) VALUES (jsonb(?)). That's not super hard to do with the things we already have in place in the generator, but we also need to do it at runtime: into(tbl).insert(TblCompanion.insert(dependencies: [1, 2, 3])) also needs to construct a jsonb invocation somewhere. Similarly, when selecting from a JSONB column, we'd have to wrap the expression in json() to get a readable format.

(of course, we could cheat and open an in-memory database that just does json(?) and jsonb(?) synchronously in the converter. but that doesn't work with sqflite and requires even more setup on the web)

dvoloshyn commented 7 months ago

Agree, this does not look like a trivial problem. It likely requires architectural changes. As such, it might make sense to start a discussion and collect all the known and expected problems around types to see a broader picture. From your comment I already see two (generalized):

simolus3 commented 7 months ago

For those using NativeDatabase (meaning that the sqlite3 library is available via dart:ffi), a converter like this can support JSONB today:

import 'dart:convert';

import 'package:drift/drift.dart';
import 'package:sqlite3/sqlite3.dart' as sqlite3;

class JsonbConverter<T> implements TypeConverter<T, Uint8List> {
  final T Function(Object?) _fromJson;

  JsonbConverter(this._fromJson);

  final sqlite3.Database _database = sqlite3.sqlite3.openInMemory();
  late final sqlite3.PreparedStatement _toText =
      _database.prepare('select json(?)');
  late final sqlite3.PreparedStatement _toBinary =
      _database.prepare('select jsonb(?)');

  @override
  T fromSql(Uint8List fromDb) {
    final result = _toText.select([fromDb]).single;
    final asText = result.values[0] as String;

    return _fromJson(json.decode(asText));
  }

  @override
  Uint8List toSql(T value) {
    final asText = json.encode(value);
    final result = _toBinary.select([asText]).single;
    return result.values[0] as Uint8List;
  }
}

It can be used in a column definition like this:

  BlobColumn myJson => blob().map(JsonbConverter((json) => MyClass.fromJson(json)))();
AlexandreAndrade00 commented 4 months ago

Hi @simolus3, is this feature in the roadmap or is hanged?

simolus3 commented 4 months ago

You can already get pretty far with the JsonbConverter I've suggested here if you don't need web support.

I've tried approaching this a few times, but I always backed out due to the complexity - this feature requires us to rewrite queries based on the result type, so if you do selectOnly(table)..addColumns([table.jsonbColumn]), we'd have to recognize that and actually run SELECT json(jsonbColumn) FROM table. And for default tables, the SELECT * also wouldn't work anymore - we'd have to list the columns explicitly. It might not sound that hard, but there is nothing in the runtime currently ready for that, and so it requires a lot of refactoring to get right. It's not closed, and I'll probably give this another go once I have a bit more time, but it's not on my near-term list.

AlexandreAndrade00 commented 4 months ago

Thank you so much for the fast update! :grinning:

jakobhec commented 1 month ago

Hi @simolus3, using static for _database, _toText and _toBinary allows us to make the constructor const. This help us to avoid calling sqlite3.sqlite3.openInMemory() multiple times. Do you think that's a good idea or could there be any downsides?

class JsonbConverter<T> implements TypeConverter<T, Uint8List> {
  final T Function(Object?) _fromJson;

  const JsonbConverter(this._fromJson);

  static final sqlite3.Database _database = sqlite3.sqlite3.openInMemory();
  static final sqlite3.PreparedStatement _toText = _database.prepare('select json(?)');
  static final sqlite3.PreparedStatement _toBinary = _database.prepare('select jsonb(?)');

  // ...
}

For others trying this, I've found that if I define tables using .drift files (documentation), there are two things to keep in mind:

  1. Use the blob instead of the jsonb datatype. Otherwise my converter is supposed to accept double instead of Uint8List.
  2. Create a new converter class.

So instead of simply using

CREATE TABLE my_table (
  my_model blob MAPPED BY `const JsonbConverter<MyModel>(myFromJsonFn)`
)

I needed to create a new class

class MyConverter extends JsonbConverter<MyModel> {
  const MyConverter() : super(myFromJsonFn);
}

And then use

CREATE TABLE my_table (
  my_model blob MAPPED BY `const MyConverter()`
)
simolus3 commented 1 month ago

Since the database connection for the jsonb conversion is kept active all the time, I don't think there's a downside of just storing it in active field. There's no close method on type converters either way, so what's the point.

That jsonb is not supported as a type is expected, we're following established rules for column affinity here. What's going wrong when not using a separate class though? I think drift should support that.

jakobhec commented 1 month ago

I just wanted to make sure that we don't instantiate JsonbConverter multiple times. And as far as I understand it's always a good idea to make things const.

If I don't use a separate class myModel will be dynamic. Here is the generated class:

class MyTableData extends DataClass implements Insertable<MyTableData> {
  final dynamic myModel;
  const MyTableData({this.myModel});
  // ...
}