diff --git a/moor/CHANGELOG.md b/moor/CHANGELOG.md index d1ad8b9e..c5dbe5e8 100644 --- a/moor/CHANGELOG.md +++ b/moor/CHANGELOG.md @@ -8,6 +8,7 @@ - Allow transactions inside a `beforeOpen` callback - New `batch` method on generated databases to execute multiple queries in a single batch - Experimental support to run moor on a background isolate +- Reduce use of parentheses in SQL code generated at runtime ## 2.0.1 diff --git a/moor/lib/src/dsl/columns.dart b/moor/lib/src/dsl/columns.dart index f45b12fb..7b7c4b8e 100644 --- a/moor/lib/src/dsl/columns.dart +++ b/moor/lib/src/dsl/columns.dart @@ -2,7 +2,10 @@ part of 'dsl.dart'; /// Base class for columns in sql. The [T] type refers to the type a value of /// this column will have in Dart, [S] is the mapping class from moor. -abstract class Column> extends Expression {} +abstract class Column> extends Expression { + @override + final Precedence precedence = Precedence.primary; +} /// A column that stores int values. abstract class IntColumn extends Column {} diff --git a/moor/lib/src/runtime/query_builder/expressions/algebra.dart b/moor/lib/src/runtime/query_builder/expressions/algebra.dart index cbfdfcf3..6b1bd162 100644 --- a/moor/lib/src/runtime/query_builder/expressions/algebra.dart +++ b/moor/lib/src/runtime/query_builder/expressions/algebra.dart @@ -7,7 +7,8 @@ extension MonoidExpr> on Expression { assert(other is! Expression, 'Used Monoid extension but should have resolved to String extension'); - return _BaseInfixOperator(this, '+', other); + return _BaseInfixOperator(this, '+', other, + precedence: Precedence.plusMinus); } } @@ -16,7 +17,8 @@ extension StringMonoidExpr on Expression { /// Performs a string concatenation in sql by appending [other] to `this`. Expression operator +( Expression other) { - return _BaseInfixOperator(this, '||', other); + return _BaseInfixOperator(this, '||', other, + precedence: Precedence.stringConcatenation); } } @@ -25,7 +27,8 @@ extension ArithmeticExpr> on Expression { /// Performs a subtraction (`this` - [other]) in sql. Expression operator -(Expression other) { - return _BaseInfixOperator(this, '-', other); + return _BaseInfixOperator(this, '-', other, + precedence: Precedence.plusMinus); } /// Returns the negation of this value. @@ -35,11 +38,13 @@ extension ArithmeticExpr> /// Performs a multiplication (`this` * [other]) in sql. Expression operator *(Expression other) { - return _BaseInfixOperator(this, '*', other); + return _BaseInfixOperator(this, '*', other, + precedence: Precedence.mulDivide); } /// Performs a division (`this` / [other]) in sql. Expression operator /(Expression other) { - return _BaseInfixOperator(this, '/', other); + return _BaseInfixOperator(this, '/', other, + precedence: Precedence.mulDivide); } } diff --git a/moor/lib/src/runtime/query_builder/expressions/bools.dart b/moor/lib/src/runtime/query_builder/expressions/bools.dart index 1c48c195..0b7a6a35 100644 --- a/moor/lib/src/runtime/query_builder/expressions/bools.dart +++ b/moor/lib/src/runtime/query_builder/expressions/bools.dart @@ -5,16 +5,18 @@ part of '../query_builder.dart'; /// This is now deprecated. Instead of `and(a, b)`, use `a & b`. @Deprecated('Use the operator on BooleanExpressionOperators instead') Expression and( - Expression a, Expression b) => - _AndExpression(a, b); + Expression a, Expression b) { + return a & b; +} /// Returns an expression that is true iff [a], [b] or both are true. /// /// This is now deprecated. Instead of `or(a, b)`, use `a | b`; @Deprecated('Use the operator on BooleanExpressionOperators instead') Expression or( - Expression a, Expression b) => - _OrExpression(a, b); + Expression a, Expression b) { + return a | b; +} /// Returns an expression that is true iff [a] is not true. /// @@ -31,43 +33,26 @@ extension BooleanExpressionOperators on Expression { /// Returns an expression that is true iff both `this` and [other] are true. Expression operator &(Expression other) { - return _AndExpression(this, other); + return _BaseInfixOperator(this, 'AND', other, precedence: Precedence.and); } /// Returns an expression that is true if `this` or [other] are true. Expression operator |(Expression other) { - return _OrExpression(this, other); + return _BaseInfixOperator(this, 'OR', other, precedence: Precedence.or); } } -class _AndExpression extends _InfixOperator { - @override - Expression left, right; - - @override - final String operator = 'AND'; - - _AndExpression(this.left, this.right); -} - -class _OrExpression extends _InfixOperator { - @override - Expression left, right; - - @override - final String operator = 'OR'; - - _OrExpression(this.left, this.right); -} - class _NotExpression extends Expression { - Expression inner; + final Expression inner; _NotExpression(this.inner); + @override + Precedence get precedence => Precedence.unary; + @override void writeInto(GenerationContext context) { context.buffer.write('NOT '); - inner.writeInto(context); + writeInner(context, inner); } } diff --git a/moor/lib/src/runtime/query_builder/expressions/comparable.dart b/moor/lib/src/runtime/query_builder/expressions/comparable.dart index b7d091b5..6b09168d 100644 --- a/moor/lib/src/runtime/query_builder/expressions/comparable.dart +++ b/moor/lib/src/runtime/query_builder/expressions/comparable.dart @@ -77,6 +77,10 @@ extension ComparableExpr> class _BetweenExpression extends Expression { final Expression target; + // https://www.sqlite.org/lang_expr.html#between + @override + final Precedence precedence = Precedence.comparisonEq; + /// Whether to negate this between expression final bool not; @@ -91,13 +95,13 @@ class _BetweenExpression extends Expression { @override void writeInto(GenerationContext context) { - target.writeInto(context); + writeInner(context, target); if (not) context.buffer.write(' NOT'); context.buffer.write(' BETWEEN '); - lower.writeInto(context); + writeInner(context, lower); context.buffer.write(' AND '); - higher.writeInto(context); + writeInner(context, higher); } } diff --git a/moor/lib/src/runtime/query_builder/expressions/datetimes.dart b/moor/lib/src/runtime/query_builder/expressions/datetimes.dart index 5e085018..830aa943 100644 --- a/moor/lib/src/runtime/query_builder/expressions/datetimes.dart +++ b/moor/lib/src/runtime/query_builder/expressions/datetimes.dart @@ -44,6 +44,9 @@ const Expression currentDateAndTime = class _CustomDateTimeExpression extends CustomExpression { + @override + final Precedence precedence = Precedence.primary; + const _CustomDateTimeExpression(String content) : super(content); } diff --git a/moor/lib/src/runtime/query_builder/expressions/expression.dart b/moor/lib/src/runtime/query_builder/expressions/expression.dart index ab16a4ba..a45e57c8 100644 --- a/moor/lib/src/runtime/query_builder/expressions/expression.dart +++ b/moor/lib/src/runtime/query_builder/expressions/expression.dart @@ -7,6 +7,10 @@ abstract class Expression> implements Component { /// Constant constructor so that subclasses can be constant. const Expression(); + /// The precedence of this expression. This can be used to automatically put + /// parentheses around expressions as needed. + Precedence get precedence => Precedence.unknown; + /// Whether this expression is a literal. Some use-sites need to put /// parentheses around non-literals. bool get isLiteral => false; @@ -21,6 +25,107 @@ abstract class Expression> implements Component { /// an SQL-injection. Expression equals(D compare) => _Comparison.equal(this, Variable(compare)); + + /// Writes this expression into the [GenerationContext], assuming that there's + /// an outer expression with [precedence]. If the [Expression.precedence] of + /// `this` expression is lower, it will be wrapped in + /// + /// See also: + /// - [Component.writeInto], which doesn't take any precedence relation into + /// account. + void writeAroundPrecedence(GenerationContext context, Precedence precedence) { + if (this.precedence < precedence) { + context.buffer.write('('); + writeInto(context); + context.buffer.write(')'); + } else { + writeInto(context); + } + } + + /// If this [Expression] wraps an [inner] expression, this utility method can + /// be used inside [writeInto] to write that inner expression while wrapping + /// it in parentheses if necessary. + @protected + void writeInner(GenerationContext ctx, Expression inner) { + assert(precedence != Precedence.unknown, + "Expressions with unknown precedence shouldn't have inner expressions"); + inner.writeAroundPrecedence(ctx, precedence); + } +} + +/// Used to order the precedence of sql expressions so that we can avoid +/// unnecessary parens when generating sql statements. +class Precedence implements Comparable { + /// Higher means higher precedence. + final int _value; + + const Precedence._(this._value); + + @override + int compareTo(Precedence other) { + return _value.compareTo(other._value); + } + + @override + int get hashCode => _value; + + @override + bool operator ==(other) { + // runtimeType comparison isn't necessary, the private constructor prevents + // subclasses + return other is Precedence && other._value == _value; + } + + /// Returns true if this [Precedence] is lower than [other]. + bool operator <(Precedence other) => compareTo(other) < 0; + + /// Returns true if this [Precedence] is lower or equal to [other]. + bool operator <=(Precedence other) => compareTo(other) <= 0; + + /// Returns true if this [Precedence] is higher than [other]. + bool operator >(Precedence other) => compareTo(other) > 0; + + /// Returns true if this [Precedence] is higher or equal to [other]. + bool operator >=(Precedence other) => compareTo(other) >= 0; + + /// Precedence is unknown, assume lowest. This can be used for a + /// [CustomExpression] to always put parens around it. + static const Precedence unknown = Precedence._(-1); + + /// Precedence for the `OR` operator in sql + static const Precedence or = Precedence._(10); + + /// Precedence for the `AND` operator in sql + static const Precedence and = Precedence._(11); + + /// Precedence for most of the comparisons operators in sql, including + /// equality, is (not) checks, in, like, glob, match, regexp. + static const Precedence comparisonEq = Precedence._(12); + + /// Precedence for the <, <=, >, >= operators in sql + static const Precedence comparison = Precedence._(13); + + /// Precedence for bitwise operators in sql + static const Precedence bitwise = Precedence._(14); + + /// Precedence for the (binary) plus and minus operators in sql + static const Precedence plusMinus = Precedence._(15); + + /// Precedence for the *, / and % operators in sql + static const Precedence mulDivide = Precedence._(16); + + /// Precedence for the || operator in sql + static const Precedence stringConcatenation = Precedence._(17); + + /// Precedence for unary operators in sql + static const Precedence unary = Precedence._(20); + + /// Precedence for postfix operators (like collate) in sql + static const Precedence postfix = Precedence._(21); + + /// Highest precedence in sql, used for variables and literals. + static const Precedence primary = Precedence._(100); } /// An expression that looks like "$a operator $b", where $a and $b itself @@ -36,29 +141,13 @@ abstract class _InfixOperator> /// The sql operator to write String get operator; - /// Whether we should put parentheses around the [left] and [right] - /// expressions. - bool get placeBrackets => true; - @override void writeInto(GenerationContext context) { - _placeBracketIfNeeded(context, true); - - left.writeInto(context); - - _placeBracketIfNeeded(context, false); + writeInner(context, left); context.writeWhitespace(); context.buffer.write(operator); context.writeWhitespace(); - _placeBracketIfNeeded(context, true); - - right.writeInto(context); - - _placeBracketIfNeeded(context, false); - } - - void _placeBracketIfNeeded(GenerationContext context, bool open) { - if (placeBrackets) context.buffer.write(open ? '(' : ')'); + writeInner(context, right); } } @@ -72,7 +161,11 @@ class _BaseInfixOperator> extends _InfixOperator { @override final Expression right; - _BaseInfixOperator(this.left, this.operator, this.right); + @override + final Precedence precedence; + + _BaseInfixOperator(this.left, this.operator, this.right, + {this.precedence = Precedence.unknown}); } /// Defines the possible comparison operators that can appear in a [_Comparison]. @@ -112,10 +205,16 @@ class _Comparison extends _InfixOperator { final _ComparisonOperator op; @override - final bool placeBrackets = false; + String get operator => _operatorNames[op]; @override - String get operator => _operatorNames[op]; + Precedence get precedence { + if (op == _ComparisonOperator.equal) { + return Precedence.comparisonEq; + } else { + return Precedence.comparison; + } + } /// Constructs a comparison from the [left] and [right] expressions to compare /// and the [ComparisonOperator] [op]. @@ -130,6 +229,9 @@ class _UnaryMinus> extends Expression { _UnaryMinus(this.inner); + @override + Precedence get precedence => Precedence.unary; + @override void writeInto(GenerationContext context) { context.buffer.write('-'); diff --git a/moor/lib/src/runtime/query_builder/expressions/in.dart b/moor/lib/src/runtime/query_builder/expressions/in.dart index 9835fc67..f23202bc 100644 --- a/moor/lib/src/runtime/query_builder/expressions/in.dart +++ b/moor/lib/src/runtime/query_builder/expressions/in.dart @@ -20,11 +20,14 @@ class _InExpression, T> final Iterable _values; final bool _not; + @override + Precedence get precedence => Precedence.comparisonEq; + _InExpression(this._expression, this._values, this._not); @override void writeInto(GenerationContext context) { - _expression.writeInto(context); + writeInner(context, _expression); if (_not) { context.buffer.write(' NOT'); diff --git a/moor/lib/src/runtime/query_builder/expressions/null_check.dart b/moor/lib/src/runtime/query_builder/expressions/null_check.dart index d9ec7821..272be444 100644 --- a/moor/lib/src/runtime/query_builder/expressions/null_check.dart +++ b/moor/lib/src/runtime/query_builder/expressions/null_check.dart @@ -12,11 +12,14 @@ class _NullCheck extends Expression { final Expression _inner; final bool _isNull; + @override + final Precedence precedence = Precedence.comparisonEq; + _NullCheck(this._inner, this._isNull); @override void writeInto(GenerationContext context) { - _inner.writeInto(context); + writeInner(context, _inner); context.buffer.write(' IS '); if (!_isNull) { diff --git a/moor/lib/src/runtime/query_builder/expressions/text.dart b/moor/lib/src/runtime/query_builder/expressions/text.dart index fd9f9eac..a1267eea 100644 --- a/moor/lib/src/runtime/query_builder/expressions/text.dart +++ b/moor/lib/src/runtime/query_builder/expressions/text.dart @@ -25,14 +25,17 @@ class _LikeOperator extends Expression { /// The regex-like expression to test the [target] against. final Expression regex; + @override + final Precedence precedence = Precedence.comparisonEq; + /// Perform a like operator with the target and the regex. _LikeOperator(this.target, this.regex); @override void writeInto(GenerationContext context) { - target.writeInto(context); + writeInner(context, target); context.buffer.write(' LIKE '); - regex.writeInto(context); + writeInner(context, regex); } } @@ -65,13 +68,16 @@ class _CollateOperator extends Expression { /// The [Collate] to use. final Collate collate; + @override + final Precedence precedence = Precedence.postfix; + /// Constructs a collate expression on the [inner] expression and the /// [Collate]. _CollateOperator(this.inner, this.collate); @override void writeInto(GenerationContext context) { - inner.writeInto(context); + writeInner(context, inner); context.buffer..write(' COLLATE ')..write(_operatorNames[collate]); } diff --git a/moor/lib/src/runtime/query_builder/expressions/variables.dart b/moor/lib/src/runtime/query_builder/expressions/variables.dart index 2a3c4374..f0b06e14 100644 --- a/moor/lib/src/runtime/query_builder/expressions/variables.dart +++ b/moor/lib/src/runtime/query_builder/expressions/variables.dart @@ -6,6 +6,9 @@ class Variable> extends Expression { /// The Dart value that will be sent to the database final T value; + @override + final Precedence precedence = Precedence.primary; + /// Constructs a new variable from the [value]. const Variable(this.value); @@ -65,6 +68,9 @@ class Constant> extends Expression { /// Constructs a new constant (sql literal) holding the [value]. const Constant(this.value); + @override + final Precedence precedence = Precedence.primary; + /// The value that will be converted to an sql literal. final T value; diff --git a/moor/lib/src/runtime/query_builder/statements/query.dart b/moor/lib/src/runtime/query_builder/statements/query.dart index 8722584c..a053f747 100644 --- a/moor/lib/src/runtime/query_builder/statements/query.dart +++ b/moor/lib/src/runtime/query_builder/statements/query.dart @@ -204,19 +204,23 @@ mixin SingleTableQueryMixin 'UpdateStatement.write respectively. In that case, you need to use a ' 'custom where statement.'); - final primaryKeys = table.$primaryKey.map((c) => c.$name); + final primaryKeyColumns = Map.fromEntries(table.$primaryKey.map((column) { + return MapEntry(column.$name, column); + })); final updatedFields = table.entityToSql(d.createCompanion(false)); - // Extract values of the primary key as they are needed for the where clause + // Construct a map of [GeneratedColumn] to [Variable] where each column is + // a primary key and the associated value was extracted from d. final primaryKeyValues = Map.fromEntries(updatedFields.entries - .where((entry) => primaryKeys.contains(entry.key))); + .where((entry) => primaryKeyColumns.containsKey(entry.key))) + .map((columnName, value) { + return MapEntry(primaryKeyColumns[columnName], value); + }); Expression predicate; for (var entry in primaryKeyValues.entries) { - // custom expression that references the column - final columnExpression = CustomExpression(entry.key); final comparison = - _Comparison(columnExpression, _ComparisonOperator.equal, entry.value); + _Comparison(entry.key, _ComparisonOperator.equal, entry.value); if (predicate == null) { predicate = comparison; diff --git a/moor/test/delete_test.dart b/moor/test/delete_test.dart index baf44dfa..8ee2e248 100644 --- a/moor/test/delete_test.dart +++ b/moor/test/delete_test.dart @@ -32,14 +32,14 @@ void main() { .go(); verify(executor.runDelete( - 'DELETE FROM users WHERE (NOT is_awesome) OR (id < ?);', [100])); + 'DELETE FROM users WHERE NOT is_awesome OR id < ?;', [100])); }); test('to delete an entity via a dataclasss', () async { await db.delete(db.sharedTodos).delete(SharedTodo(todo: 3, user: 2)); verify(executor.runDelete( - 'DELETE FROM shared_todos WHERE (todo = ?) AND (user = ?);', [3, 2])); + 'DELETE FROM shared_todos WHERE todo = ? AND user = ?;', [3, 2])); }); }); diff --git a/moor/test/expressions/algebra_test.dart b/moor/test/expressions/algebra_test.dart index 2e0d8480..d64b4d04 100644 --- a/moor/test/expressions/algebra_test.dart +++ b/moor/test/expressions/algebra_test.dart @@ -10,11 +10,12 @@ void main() { final s2 = GeneratedTextColumn('s2', 'tbl', true); test('arithmetic test', () { - _expectSql(i1 + i2 * i1, '(i1) + ((i2) * (i1))'); + _expectSql(i1 + i2 * i1, 'i1 + i2 * i1'); + _expectSql((i1 + i2) * i1, '(i1 + i2) * i1'); }); test('string concatenation', () { - _expectSql(s1 + s2, '(s1) || (s2)'); + _expectSql(s1 + s2, 's1 || s2'); }); } diff --git a/moor/test/select_test.dart b/moor/test/select_test.dart index 136d2563..6a639d69 100644 --- a/moor/test/select_test.dart +++ b/moor/test/select_test.dart @@ -70,7 +70,7 @@ void main() { .get(); verify(executor.runSelect( - 'SELECT * FROM users WHERE (NOT name = ?) AND (id > ?);', + 'SELECT * FROM users WHERE NOT (name = ?) AND id > ?;', ['Dash', 12])); });