Skip to content

Commit bd427f5

Browse files
committed
Warning for identifiers looking like strings
Contextualize the error message for double-quoted identifiers that can't be resolved, because those should potentially have been single-quoted string literals. Closes #3631
1 parent 6406f62 commit bd427f5

5 files changed

Lines changed: 42 additions & 2 deletions

File tree

‎sqlparser/CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 0.41.2
2+
3+
- Improve error message on unknown columns when it looks like the identifier
4+
should have been a string literal.
5+
16
## 0.41.1
27

38
- Support new features introduced in SQLite version 3.50.0.

‎sqlparser/lib/src/analysis/steps/reference_resolver.dart‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,11 @@ class ReferenceResolver
121121

122122
void _reportUnknownColumnError(Reference e, {Iterable<Column>? columns}) {
123123
final msg = StringBuffer('Unknown column.');
124-
if (columns != null) {
124+
125+
if (e.isSingleDoubleQuotedToken) {
126+
msg.write(' Note: Double-quotes define an identifier in SQL. '
127+
'If you meant to write a string literal, use single quotes instead.');
128+
} else if (columns != null) {
125129
final columnNames =
126130
columns.map((c) => c.humanReadableDescription()).join(', ');
127131
msg.write(' These columns are available: $columnNames');

‎sqlparser/lib/src/ast/expressions/reference.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,27 @@ class Reference extends Expression with ReferenceOwner {
2828
'When setting a schemaName, entityName must not be null either.',
2929
);
3030

31+
/// Returns whether this [Reference] is syntactically derived from a single
32+
/// identifier token wrapped in double quotes.
33+
///
34+
/// Because the strict "double quotes are identifiers and never string
35+
/// literals" rule enabled by drift and sqlparser can be surprising, we use
36+
/// this information to improve error messages and point this out
37+
/// specifically.
38+
bool get isSingleDoubleQuotedToken {
39+
if (schemaName != null || entityName != null) {
40+
return false;
41+
}
42+
43+
if (first == last) {
44+
if (first case final IdentifierToken singleToken) {
45+
return singleToken.escaped;
46+
}
47+
}
48+
49+
return false;
50+
}
51+
3152
@override
3253
R accept<A, R>(AstVisitor<A, R> visitor, A arg) {
3354
return visitor.visitReference(this, arg);

‎sqlparser/pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name: sqlparser
22
description: Parses sqlite statements and performs static analysis on them
3-
version: 0.41.1
3+
version: 0.41.2-dev
44
homepage: https://github.com/simolus3/drift/tree/develop/sqlparser
55
repository: https://github.com/simolus3/drift
66
#homepage: https://drift.simonbinder.eu/

‎sqlparser/test/analysis/reference_resolver_test.dart‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,4 +440,14 @@ CREATE TABLE routes (
440440
expect(fromReferenced!.source.containingSet,
441441
result.rootScope.knownTables['points']);
442442
});
443+
444+
test('warns about identifiers that should have been strings', () {
445+
final result = SqlEngine().analyze('''SELECT 'hello ' || "world";''');
446+
447+
result.expectError(
448+
'"world"',
449+
type: AnalysisErrorType.referencedUnknownColumn,
450+
message: contains('Note: Double-quotes define an identifier in SQL.'),
451+
);
452+
});
443453
}

0 commit comments

Comments
 (0)