Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use native postgres boolean type instead of integers #2116

Merged
merged 19 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions drift/example/main.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions drift/lib/src/runtime/query_builder/expressions/variables.dart
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class Variable<T extends Object> extends Expression<T> {
/// database engine. For instance, a [DateTime] will me mapped to its unix
/// timestamp.
dynamic mapToSimpleValue(GenerationContext context) {
return context.options.types.mapToSqlVariable(value);
return context.options.types.mapToSqlVariable(value, context.dialect);
}

@override
Expand All @@ -83,7 +83,10 @@ class Variable<T extends Object> extends Expression<T> {
context.buffer
..write(mark)
..write(explicitStart + context.amountOfVariables);
context.introduceVariable(this, mapToSimpleValue(context));
context.introduceVariable(
this,
mapToSimpleValue(context),
);
} else {
context.buffer.write(mark);
context.introduceVariable(this, mapToSimpleValue(context));
Expand Down Expand Up @@ -117,7 +120,10 @@ class Constant<T extends Object> extends Expression<T> {

@override
void writeInto(GenerationContext context) {
context.buffer.write(context.options.types.mapToSqlLiteral(value));
context.buffer.write(context.options.types.mapToSqlLiteral(
value,
context.dialect,
));
}

@override
Expand Down
17 changes: 9 additions & 8 deletions drift/lib/src/runtime/query_builder/schema/column_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class GeneratedColumn<T extends Object> extends Column<T> {
final bool $nullable;

/// Default constraints generated by drift.
final String? _defaultConstraints;
final void Function(GenerationContext)? _defaultConstraints;

/// Custom constraints that have been specified for this column.
///
Expand Down Expand Up @@ -65,8 +65,7 @@ class GeneratedColumn<T extends Object> extends Column<T> {

/// Whether this column has an `AUTOINCREMENT` primary key constraint that was
/// created by drift.
bool get hasAutoIncrement =>
_defaultConstraints?.contains('AUTOINCREMENT') == true;
final bool hasAutoIncrement;

@override
String get name => $name;
Expand All @@ -78,13 +77,14 @@ class GeneratedColumn<T extends Object> extends Column<T> {
this.$nullable, {
this.clientDefault,
required this.type,
String? defaultConstraints,
void Function(GenerationContext)? defaultConstraints,
this.$customConstraints,
this.defaultValue,
this.additionalChecks,
this.requiredDuringInsert = false,
this.generatedAs,
this.check,
this.hasAutoIncrement = false,
}) : _defaultConstraints = defaultConstraints;

/// Applies a type converter to this column.
Expand All @@ -106,6 +106,7 @@ class GeneratedColumn<T extends Object> extends Column<T> {
requiredDuringInsert,
generatedAs,
check,
hasAutoIncrement,
);
}

Expand Down Expand Up @@ -157,9 +158,7 @@ class GeneratedColumn<T extends Object> extends Column<T> {

// these custom constraints refer to builtin constraints from drift
if (!isSerial && _defaultConstraints != null) {
into.buffer
..write(' ')
..write(_defaultConstraints);
_defaultConstraints!(into);
}
} else if ($customConstraints?.isNotEmpty == true) {
into.buffer
Expand Down Expand Up @@ -265,13 +264,14 @@ class GeneratedColumnWithTypeConverter<D, S extends Object>
bool nullable,
S? Function()? clientDefault,
DriftSqlType<S> type,
String? defaultConstraints,
void Function(GenerationContext)? defaultConstraints,
String? customConstraints,
Expression<S>? defaultValue,
VerificationResult Function(S?, VerificationMeta)? additionalChecks,
bool requiredDuringInsert,
GeneratedAs? generatedAs,
Expression<bool> Function()? check,
bool hasAutoIncrement,
) : super(
name,
tableName,
Expand All @@ -285,6 +285,7 @@ class GeneratedColumnWithTypeConverter<D, S extends Object>
requiredDuringInsert: requiredDuringInsert,
generatedAs: generatedAs,
check: check,
hasAutoIncrement: hasAutoIncrement,
);

/// Compares this column against the mapped [dartValue].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ class QueryRow {
/// support non-nullable types.
T read<T>(String key) {
final type = DriftSqlType.forNullableType<T>();
return _db.options.types.read(type, data[key]) as T;
return _db.options.types.read(
type,
data[key],
_db.executor.dialect,
) as T;
}

/// Reads a nullable value from this row.
Expand All @@ -90,7 +94,11 @@ class QueryRow {
/// drift (e.g. booleans, strings, numbers, dates, `Uint8List`s).
T? readNullable<T extends Object>(String key) {
final type = DriftSqlType.forType<T>();
return _db.options.types.read(type, data[key]);
return _db.options.types.read(
type,
data[key],
_db.executor.dialect,
);
}

/// Reads a bool from the column named [key].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,11 @@ class JoinedSelectStatement<FirstT extends HasResultSet, FirstD>
final expr = aliasedColumn.key;
final value = row[aliasedColumn.value];

readColumns[expr] = ctx.options.types.read(expr.driftSqlType, value);
readColumns[expr] = ctx.options.types.read(
expr.driftSqlType,
value,
ctx.dialect,
);
}

return TypedResult(readTables, QueryRow(row, database), readColumns);
Expand Down
34 changes: 26 additions & 8 deletions drift/lib/src/runtime/types/mapping.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ class SqlTypes {

/// Maps a Dart object to a (possibly simpler) object that can be used as a
/// parameter in raw sql queries.
Object? mapToSqlVariable(Object? dartValue) {
Object? mapToSqlVariable(
Object? dartValue, [
SqlDialect dialect = SqlDialect.sqlite,
]) {
if (dartValue == null) return null;

// These need special handling, all other types are a direct mapping
Expand Down Expand Up @@ -71,7 +74,7 @@ class SqlTypes {
}
}

if (dartValue is bool) {
if (dartValue is bool && dialect == SqlDialect.sqlite) {
return dartValue ? 1 : 0;
}

Expand All @@ -80,12 +83,19 @@ class SqlTypes {

/// Maps the [dart] value into a SQL literal that can be embedded in SQL
/// queries.
String mapToSqlLiteral(Object? dart) {
String mapToSqlLiteral(
Object? dart, [
SqlDialect dialect = SqlDialect.sqlite,
simolus3 marked this conversation as resolved.
Show resolved Hide resolved
]) {
if (dart == null) return 'NULL';

// todo: Inline and remove types in the next major drift version
if (dart is bool) {
return dart ? '1' : '0';
if (dialect == SqlDialect.sqlite) {
return dart ? '1' : '0';
} else {
return dart ? 'true' : 'false';
}
} else if (dart is String) {
// From the sqlite docs: (https://www.sqlite.org/lang_expr.html)
// A string constant is formed by enclosing the string in single quotes
Expand All @@ -99,7 +109,7 @@ class SqlTypes {
return dart.toString();
} else if (dart is DateTime) {
if (storeDateTimesAsText) {
final encoded = mapToSqlVariable(dart).toString();
final encoded = mapToSqlVariable(dart, dialect).toString();
return "'$encoded'";
} else {
return (dart.millisecondsSinceEpoch ~/ 1000).toString();
Expand All @@ -115,13 +125,21 @@ class SqlTypes {
}

/// Maps a raw [sqlValue] to Dart given its sql [type].
T? read<T extends Object>(DriftSqlType<T> type, Object? sqlValue) {
T? read<T extends Object>(
DriftSqlType<T> type,
Object? sqlValue, [
SqlDialect dialect = SqlDialect.sqlite,
]) {
if (sqlValue == null) return null;

// ignore: unnecessary_cast
switch (type as DriftSqlType<Object>) {
case DriftSqlType.bool:
return (sqlValue != 0) as T;
if (dialect == SqlDialect.sqlite) {
return (sqlValue != 0) as T;
} else {
return sqlValue as T;
}
case DriftSqlType.string:
return sqlValue.toString() as T;
case DriftSqlType.bigInt:
Expand Down Expand Up @@ -233,7 +251,7 @@ enum DriftSqlType<T extends Object> implements _InternalDriftSqlType<T> {
// ignore: unnecessary_cast
switch (this as DriftSqlType<Object>) {
case DriftSqlType.bool:
return dialect == SqlDialect.sqlite ? 'INTEGER' : 'integer';
return dialect == SqlDialect.sqlite ? 'INTEGER' : 'boolean';
case DriftSqlType.string:
return dialect == SqlDialect.sqlite ? 'TEXT' : 'text';
case DriftSqlType.bigInt:
Expand Down
40 changes: 40 additions & 0 deletions drift/test/database/types/bool_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import 'package:drift/drift.dart';
import 'package:test/test.dart';

void main() {
final typeSystem = DriftDatabaseOptions().types;

group('bool type', () {
test('Can read booleans from sqlite', () {
expect(typeSystem.read(DriftSqlType.bool, 1, SqlDialect.sqlite), true);
expect(typeSystem.read(DriftSqlType.bool, 0, SqlDialect.sqlite), false);
});

test('Can read booleans from postgres', () {
expect(
typeSystem.read(DriftSqlType.bool, true, SqlDialect.postgres), true);
expect(typeSystem.read(DriftSqlType.bool, false, SqlDialect.postgres),
false);
});

test('Can be mapped to sqlite constant', () {
expect(typeSystem.mapToSqlLiteral(true, SqlDialect.sqlite), '1');
expect(typeSystem.mapToSqlLiteral(false, SqlDialect.sqlite), '0');
});

test('Can be mapped to postgres constant', () {
expect(typeSystem.mapToSqlLiteral(true, SqlDialect.postgres), 'true');
expect(typeSystem.mapToSqlLiteral(false, SqlDialect.postgres), 'false');
});

test('Can be mapped to sqlite variable', () {
expect(typeSystem.mapToSqlVariable(true, SqlDialect.sqlite), 1);
expect(typeSystem.mapToSqlVariable(false, SqlDialect.sqlite), 0);
});

test('Can be mapped to postgres variable', () {
expect(typeSystem.mapToSqlVariable(true, SqlDialect.postgres), true);
expect(typeSystem.mapToSqlVariable(false, SqlDialect.postgres), false);
});
});
}
Loading