diff --git a/sqlparser/CHANGELOG.md b/sqlparser/CHANGELOG.md index dbf21fee..76fb9d85 100644 --- a/sqlparser/CHANGELOG.md +++ b/sqlparser/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.21.1-dev + +- Lint for `DISTINCT` misuse in aggregate function calls. + ## 0.21.0 - Analysis support for new features in sqlite version 3.38. diff --git a/sqlparser/lib/src/analysis/error.dart b/sqlparser/lib/src/analysis/error.dart index 92cba390..f76a3df3 100644 --- a/sqlparser/lib/src/analysis/error.dart +++ b/sqlparser/lib/src/analysis/error.dart @@ -80,6 +80,7 @@ enum AnalysisErrorType { missingPrimaryKey, noTypeNameInStrictTable, invalidTypeNameInStrictTable, + distinctAggregateWithWrongParameterCount, other, hint, } diff --git a/sqlparser/lib/src/analysis/steps/linting_visitor.dart b/sqlparser/lib/src/analysis/steps/linting_visitor.dart index 0b299711..27b50d63 100644 --- a/sqlparser/lib/src/analysis/steps/linting_visitor.dart +++ b/sqlparser/lib/src/analysis/steps/linting_visitor.dart @@ -204,6 +204,19 @@ class LintingVisitor extends RecursiveVisitor { visitChildren(e, arg); } + @override + void visitExpressionFunctionParameters(ExprFunctionParameters e, void arg) { + if (e.distinct && e.parameters.length != 1) { + context.reportError(AnalysisError( + type: AnalysisErrorType.distinctAggregateWithWrongParameterCount, + message: 'A `DISTINCT` aggregate must have exactly one argument', + relevantNode: e.distinctKeyword ?? e, + )); + } + + visitChildren(e, arg); + } + @override void visitInvocation(SqlInvocation e, void arg) { final lowercaseCall = e.name.toLowerCase(); diff --git a/sqlparser/lib/src/ast/expressions/function.dart b/sqlparser/lib/src/ast/expressions/function.dart index ee9c25a5..bd552080 100644 --- a/sqlparser/lib/src/ast/expressions/function.dart +++ b/sqlparser/lib/src/ast/expressions/function.dart @@ -66,6 +66,7 @@ class StarFunctionParameter extends FunctionParameters { class ExprFunctionParameters extends FunctionParameters { final bool distinct; + Token? distinctKeyword; List parameters; ExprFunctionParameters({this.parameters = const [], this.distinct = false}); diff --git a/sqlparser/lib/src/reader/parser.dart b/sqlparser/lib/src/reader/parser.dart index 1d31e214..dd37d2e0 100644 --- a/sqlparser/lib/src/reader/parser.dart +++ b/sqlparser/lib/src/reader/parser.dart @@ -89,6 +89,13 @@ class Parser { return false; } + Token? _matchOneAndGet(TokenType type) { + if (_matchOne(type)) { + return _previous; + } + return null; + } + /// Returns true if the next token is [type] or if the next two tokens are /// "NOT" followed by [type]. Does not consume any tokens. bool _checkWithNot(TokenType type) { @@ -901,7 +908,7 @@ class Parser { return ExprFunctionParameters(parameters: const [])..synthetic = true; } - final distinct = _matchOne(TokenType.distinct); + final distinct = _matchOneAndGet(TokenType.distinct); final parameters = []; final first = _peek; @@ -909,7 +916,9 @@ class Parser { parameters.add(expression()); } while (_matchOne(TokenType.comma)); - return ExprFunctionParameters(distinct: distinct, parameters: parameters) + return ExprFunctionParameters( + distinct: distinct != null, parameters: parameters) + ..distinctKeyword = distinct ..setSpan(first, _previous); } diff --git a/sqlparser/test/analysis/errors/misc_test.dart b/sqlparser/test/analysis/errors/misc_test.dart new file mode 100644 index 00000000..646f87a1 --- /dev/null +++ b/sqlparser/test/analysis/errors/misc_test.dart @@ -0,0 +1,16 @@ +import 'package:sqlparser/sqlparser.dart'; +import 'package:test/test.dart'; + +import '../data.dart'; +import 'utils.dart'; + +void main() { + final engine = SqlEngine()..registerTable(demoTable); + + test('warns about multiple parameters with DISTINCT', () { + engine + .analyze("SELECT group_concat(DISTINCT content, '-') FROM demo") + .expectError('DISTINCT', + type: AnalysisErrorType.distinctAggregateWithWrongParameterCount); + }); +}