Better recovery for selects without columns

Author: simolus3

sqlparser/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.44.2
+
+- Better error recovery for select statements without result columns.
+
 ## 0.44.1
 
 - Expose optional table name token for `StarResultColumn`.

sqlparser/lib/src/reader/parser.dart

@@ -1631,16 +1631,27 @@ extension Parser on ParserState {
 
   List<ResultColumn> _resultColumns() {
     final columns = <ResultColumn>[];
+    var isFirst = true;
+
     do {
-      columns.add(_resultColumn());
+      // While there must be at least one column, making the first optional
+      // allows for better error recovery in queries like `SELECT   FROM ...`.
+      final column = _resultColumn(optional: isFirst);
+      if (column == null) {
+        errors.add(ParsingError(_peek, 'Expected a result column here.'));
+        break;
+      }
+
+      columns.add(column);
+      isFirst = false;
     } while (_matchOne(TokenType.comma));
 
     return columns;
   }
 
   /// Parses a [ResultColumn] or throws if none is found.
   /// https://www.sqlite.org/syntax/result-column.html
-  ResultColumn _resultColumn() {
+  ResultColumn? _resultColumn({bool optional = false}) {
     if (_matchOne(TokenType.star)) {
       return StarResultColumn(null)..setSpan(_previous, _previous);
     }
@@ -1673,11 +1684,14 @@ extension Parser on ParserState {
     Expression expr;
 
     {
-      final parser = _ExpressionParser(this, allowResultColumn: true);
+      final parser =
+          _ExpressionParser(this, allowResultColumn: true, optional: optional);
       try {
         expr = parser._or();
       } on _ParsedResultColumn catch (e) {
         return e._resultColumn;
+      } on _NoExpressionFound {
+        return null;
       }
     }
 
@@ -2716,7 +2730,15 @@ final class _ExpressionParser extends ParserState {
   /// instead of returning regularly if this is enabled.
   final bool allowResultColumn;
 
-  _ExpressionParser(super.state, {this.allowResultColumn = false})
+  /// Whether the expression being parsed is optional, meaning that it's
+  /// acceptable for no expression to be parsed too.
+  ///
+  /// The parser throws a [_NoExpressionFound] exception in this case instead of
+  /// emitting a syntax error.
+  final bool optional;
+
+  _ExpressionParser(super.state,
+      {this.allowResultColumn = false, this.optional = false})
       : super.fromParent();
 
   /// Parses an expression of the form `a <T> b`, where `<T>` is in [types] and
@@ -3066,6 +3088,10 @@ final class _ExpressionParser extends ParserState {
       return _referenceOrFunctionCall();
     }
 
+    if (optional) {
+      throw const _NoExpressionFound();
+    }
+
     if (_peek is KeywordToken) {
       // Improve error messages for possible function calls, https://github.com/simolus3/drift/discussions/2277
       final (_, next) = tokens.keywordLookahead2();
@@ -3240,6 +3266,10 @@ final class _ParsedResultColumn implements Exception {
   _ParsedResultColumn(this._resultColumn);
 }
 
+final class _NoExpressionFound implements Exception {
+  const _NoExpressionFound();
+}
+
 extension on List<Token> {
   String get lexeme => first.span.expand(last.span).text;
 }

sqlparser/pubspec.yaml

@@ -1,6 +1,6 @@
 name: sqlparser
 description: Parses sqlite statements and performs static analysis on them
-version: 0.44.1
+version: 0.44.2
 homepage: https://github.com/simolus3/drift/tree/develop/sqlparser
 repository: https://github.com/simolus3/drift
 #homepage: https://drift.simonbinder.eu/

sqlparser/test/parser/errors_test.dart

@@ -1,4 +1,5 @@
 import 'package:sqlparser/sqlparser.dart';
+import 'package:sqlparser/src/utils/ast_equality.dart';
 import 'package:test/test.dart';
 
 import 'utils.dart';
@@ -21,7 +22,7 @@ void main() {
 
   group('when using keywords', () {
     test('for function calls', () {
-      expectError('SELECT replace(a, b, c);', [
+      expectError('SELECT 1, replace(a, b, c);', [
         isParsingError(
           message: contains('Did you mean to call a function?'),
           span: 'replace',
@@ -30,7 +31,7 @@ void main() {
     });
 
     test('as identifiers', () {
-      expectError('SELECT group FROM foo;', [
+      expectError('SELECT * FROM foo WHERE group;', [
         isParsingError(
           message: contains('Did you mean to use it as a column?'),
           span: 'group',
@@ -53,6 +54,22 @@ void main() {
       ]);
     });
   });
+
+  test('missing result columns', () {
+    final parsed =
+        SqlEngine().parse(ParserEntrypoint.statement, 'SELECT   FROM users;');
+
+    enforceEqual(
+      parsed.rootNode,
+      SelectStatement(
+        columns: [],
+        from: TableReference('users'),
+      ),
+    );
+    expect(parsed.errors, [
+      isParsingError(message: 'Expected a result column here.', span: 'FROM')
+    ]);
+  });
 }
 
 void expectError(String sql, errorsMatcher) {