Analysis support around strict tables

This commit is contained in:
Simon Binder 2021-08-28 17:40:21 +02:00
parent 074b663b79
commit b9c609a5f2
No known key found for this signature in database
GPG Key ID: 7891917E4147B8C0
10 changed files with 283 additions and 25 deletions

View File

@ -26,7 +26,7 @@ class AnalysisError {
/// The relevant portion of the source code that caused this error. Some AST
/// nodes don't have a span, in that case this error is going to have a null
/// span as well.
FileSpan? get span => source!.span;
FileSpan? get span => source?.span;
@override
String toString() {
@ -75,5 +75,9 @@ enum AnalysisErrorType {
notSupportedInDesiredVersion,
illegalUseOfReturning,
raiseMisuse,
nullableColumnInStrictPrimaryKey,
missingPrimaryKey,
noTypeNameInStrictTable,
invalidTypeNameInStrictTable,
other,
}

View File

@ -126,6 +126,20 @@ class SchemaFromCreateTable {
return const ResolvedType(type: BasicType.real);
}
bool isValidTypeNameForStrictTable(String typeName) {
// See https://www.sqlite.org/draft/stricttables.html
const allowed = {'INT', 'INTEGER', 'REAL', 'TEXT', 'BLOB', 'ANY'};
const alsoAllowedInMoor = {'ENUM', 'BOOL', 'DATE'};
if (allowed.contains(typeName.toUpperCase()) ||
(moorExtensions &&
alsoAllowedInMoor.contains(typeName.toUpperCase()))) {
return true;
}
return false;
}
/// Looks up the correct column affinity for a declared type name with the
/// rules described here:
/// https://www.sqlite.org/datatype3.html#determination_of_column_affinity

View File

@ -26,7 +26,24 @@ class LintingVisitor extends RecursiveVisitor<void, void> {
@override
void visitCreateTableStatement(CreateTableStatement e, void arg) {
final schemaReader =
SchemaFromCreateTable(moorExtensions: options.useMoorExtensions);
var hasPrimaryKeyDeclaration = false;
var isStrict = false;
if (e.isStrict) {
if (options.version < SqliteVersion.v3_37) {
context.reportError(AnalysisError(
type: AnalysisErrorType.notSupportedInDesiredVersion,
message: 'STRICT tables are only supported from sqlite3 version 37',
relevantNode: e.strict ?? e,
));
} else {
// only report warnings related to STRICT tables if strict tables are
// supported.
isStrict = true;
}
}
// Ensure that a table declaration only has one PRIMARY KEY constraint
void handlePrimaryKeyNode(AstNode node) {
@ -41,9 +58,40 @@ class LintingVisitor extends RecursiveVisitor<void, void> {
}
for (final column in e.columns) {
if (isStrict) {
final typeName = column.typeName;
if (typeName == null) {
// Columns in strict tables must have a type name, even if it's
// `ANY`.
context.reportError(AnalysisError(
type: AnalysisErrorType.noTypeNameInStrictTable,
message: 'In `STRICT` tables, columns must have a type name!',
relevantNode: column.nameToken ?? column,
));
} else if (!schemaReader.isValidTypeNameForStrictTable(typeName)) {
context.reportError(AnalysisError(
type: AnalysisErrorType.invalidTypeNameInStrictTable,
message: 'Invalid type name for a `STRICT` table.',
relevantNode: column.typeNames?.toSingleEntity ?? column,
));
}
}
for (final constraint in column.constraints) {
if (constraint is PrimaryKeyColumn) {
handlePrimaryKeyNode(constraint);
// A primary key in a STRICT table must be annoted with "NOT NULL"
if (isStrict && !column.isNonNullable) {
context.reportError(AnalysisError(
type: AnalysisErrorType.nullableColumnInStrictPrimaryKey,
message:
'The column is used as a `PRIMARY KEY` in a `STRICT` table, '
'which means that is must be marked as `NOT NULL`',
relevantNode: constraint,
));
}
}
}
}
@ -51,9 +99,40 @@ class LintingVisitor extends RecursiveVisitor<void, void> {
for (final constraint in e.tableConstraints) {
if (constraint is KeyClause && constraint.isPrimaryKey) {
handlePrimaryKeyNode(constraint);
if (isStrict) {
for (final columnName in constraint.columns) {
final expr = columnName.expression;
if (expr is! Reference) continue;
final column = e.columns.firstWhereOrNull((c) =>
c.columnName.toLowerCase() == expr.columnName.toLowerCase());
if (column != null && !column.isNonNullable) {
context.reportError(
AnalysisError(
type: AnalysisErrorType.nullableColumnInStrictPrimaryKey,
message:
'This column must be marked as `NOT NULL` to be used in '
'a `PRIMARY KEY` clause of a `STRICT` table.',
relevantNode: columnName,
),
);
}
}
}
}
}
if (e.withoutRowId && !hasPrimaryKeyDeclaration) {
context.reportError(
AnalysisError(
type: AnalysisErrorType.missingPrimaryKey,
message: 'Missing PRIMARY KEY declaration for a table without rowid.',
relevantNode: e.tableNameToken ?? e,
),
);
}
visitChildren(e, arg);
}

View File

@ -28,6 +28,8 @@ class ColumnDefinition extends AstNode {
@override
Iterable<AstNode> get childNodes => constraints;
bool get isNonNullable => findConstraint<NotNull>() != null;
/// Finds a constraint of type [T], or null, if none is set.
T? findConstraint<T extends ColumnConstraint>() {
final typedConstraints = constraints.whereType<T>().iterator;

View File

@ -40,6 +40,7 @@ class CreateTableStatement extends TableInducingStatement {
Token? openingBracket;
Token? closingBracket;
Token? strict;
CreateTableStatement({
bool ifNotExists = false,

View File

@ -1886,12 +1886,14 @@ class Parser {
var withoutRowId = false;
var isStrict = false;
Token? strict;
// Parses a `WITHOUT ROWID` or a `STRICT` keyword. Returns if either such
// option has been parsed.
bool tableOptions() {
if (_matchOne(TokenType.strict)) {
isStrict = true;
strict = _previous;
return true;
} else if (_matchOne(TokenType.without)) {
_consume(TokenType.rowid,
@ -1936,7 +1938,8 @@ class Parser {
..setSpan(first, _previous)
..openingBracket = leftParen
..tableNameToken = tableIdentifier
..closingBracket = rightParen;
..closingBracket = rightParen
..strict = strict;
}
/// Parses a `CREATE VIRTUAL TABLE` statement, after the `CREATE VIRTUAL TABLE

View File

@ -53,4 +53,28 @@ extension UnionEntityExtension on Iterable<SyntacticEntity> {
FileSpan? get spanOrNull {
return isEmpty ? null : span;
}
/// Returns a single [SyntacticEntity] representing this range of entities.
SyntacticEntity get toSingleEntity => _UnionSyntacticEntity(toList());
}
class _UnionSyntacticEntity extends SyntacticEntity {
final List<SyntacticEntity> _members;
_UnionSyntacticEntity(this._members);
@override
int get firstPosition => _members.first.firstPosition;
@override
bool get hasSpan => _members.any((entity) => entity.hasSpan);
@override
int get lastPosition => _members.last.lastPosition;
@override
FileSpan? get span => _members.spanOrNull;
@override
bool get synthetic => _members.every((entity) => entity.synthetic);
}

View File

@ -0,0 +1,112 @@
import 'package:sqlparser/sqlparser.dart';
import 'package:test/test.dart';
import 'utils.dart';
void main() {
final oldEngine = SqlEngine(EngineOptions(version: SqliteVersion.v3_35));
final engine = SqlEngine(EngineOptions(version: SqliteVersion.v3_37));
group('using STRICT', () {
test('with an old sqlite3 version', () {
const stmt = 'CREATE TABLE a (c INTEGER) STRICT';
oldEngine.analyze(stmt).expectError('STRICT',
type: AnalysisErrorType.notSupportedInDesiredVersion);
engine.analyze(stmt).expectNoError();
});
test(
'with a nullable primary key column',
() {
const stmt = 'CREATE TABLE a (c INTEGER PRIMARY KEY) STRICT;';
engine.analyze(stmt).expectError('PRIMARY KEY',
type: AnalysisErrorType.nullableColumnInStrictPrimaryKey);
},
);
test(
'with a nullable primary key table constraint',
() {
final errors = engine.analyze(
'''
CREATE TABLE a (
c INTEGER,
c2 INTEGER NOT NULL,
c3 INTEGER,
PRIMARY KEY (c, c2, c3)
) STRICT;''',
).errors;
expect(
errors,
containsAll([
analysisErrorWith(
lexeme: 'c',
type: AnalysisErrorType.nullableColumnInStrictPrimaryKey),
analysisErrorWith(
lexeme: 'c3',
type: AnalysisErrorType.nullableColumnInStrictPrimaryKey),
]),
);
},
);
test(
'without a column type',
() {
engine
.analyze('CREATE TABLE a (c) STRICT;')
.expectError('c', type: AnalysisErrorType.noTypeNameInStrictTable);
},
);
test(
'with an invalid column type',
() {
engine.analyze('CREATE TABLE a (c INTEGER(12)) STRICT;').expectError(
'INTEGER(12)',
type: AnalysisErrorType.invalidTypeNameInStrictTable,
);
},
);
});
test('using WITHOUT ROWID and then not declaring a primary key', () {
engine
.analyze('CREATE TABLE a (c INTEGER) WITHOUT ROWID')
.expectError('a', type: AnalysisErrorType.missingPrimaryKey);
engine.analyze('CREATE TABLE a (c INTEGER);').expectNoError();
engine
.analyze('CREATE TABLE a (c INTEGER PRIMARY KEY) WITHOUT ROWID;')
.expectNoError();
final errors =
engine.analyze('CREATE TABLE a (c INTEGER, PRIMARY KEY (c));');
expect(
errors,
isNot(contains(
analysisErrorWith(type: AnalysisErrorType.missingPrimaryKey))));
});
test('multiple primary key constraints', () {
engine
.analyze(
'CREATE TABLE a (c INTEGER PRIMARY KEY, c2 INTEGER PRIMARY KEY)')
.expectError('PRIMARY KEY',
type: AnalysisErrorType.duplicatePrimaryKeyDeclaration);
final errors = engine
.analyze('CREATE TABLE a (c INTEGER PRIMARY KEY, c2, PRIMARY KEY (c2))')
.errors;
expect(
errors,
contains(analysisErrorWith(
lexeme: 'PRIMARY KEY (c2)',
type: AnalysisErrorType.duplicatePrimaryKeyDeclaration)),
);
});
}

View File

@ -1,6 +1,8 @@
import 'package:sqlparser/sqlparser.dart';
import 'package:test/test.dart';
import 'utils.dart';
void main() {
late SqlEngine engine;
setUp(() {
@ -8,33 +10,39 @@ void main() {
});
test('when using row value in select', () {
engine.analyze('SELECT (1, 2, 3)').expectError('(1, 2, 3)');
engine
.analyze('SELECT (1, 2, 3)')
.expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
});
test('as left hand operator of in', () {
engine.analyze('SELECT (1, 2, 3) IN (4, 5, 6)').expectError('(1, 2, 3)');
engine
.analyze('SELECT (1, 2, 3) IN (4, 5, 6)')
.expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
});
test('in BETWEEN expression', () {
engine.analyze('SELECT 1 BETWEEN (1, 2, 3) AND 3').expectError('(1, 2, 3)');
engine
.analyze('SELECT 1 BETWEEN (1, 2, 3) AND 3')
.expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
});
test('in CASE - value', () {
engine
.analyze('SELECT CASE 1 WHEN 1 THEN (1, 2, 3) ELSE 1 END')
.expectError('(1, 2, 3)');
.expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
});
test('in CASE - when', () {
engine
.analyze('SELECT CASE 1 WHEN (1, 2, 3) THEN 1 ELSE 1 END')
.expectError('(1, 2, 3)');
.expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
});
test('in CASE - base', () {
engine
.analyze('SELECT CASE (1, 2, 3) WHEN 1 THEN 1 ELSE 1 END')
.expectError('(1, 2, 3)');
.expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
});
group('does not generate error for valid usage', () {
@ -61,20 +69,3 @@ void main() {
});
});
}
extension on AnalysisContext {
void expectError(String lexeme) {
expect(
errors,
[
isA<AnalysisError>()
.having((e) => e.type, 'type', AnalysisErrorType.rowValueMisuse)
.having((e) => e.span!.text, 'span.text', lexeme),
],
);
}
void expectNoError() {
expect(errors, isEmpty);
}
}

View File

@ -0,0 +1,28 @@
import 'package:sqlparser/sqlparser.dart';
import 'package:test/test.dart';
extension ExpectErrors on AnalysisContext {
void expectError(String lexeme, {AnalysisErrorType? type}) {
expect(
errors,
[analysisErrorWith(lexeme: lexeme, type: type)],
);
}
void expectNoError() {
expect(errors, isEmpty);
}
}
Matcher analysisErrorWith({String? lexeme, AnalysisErrorType? type}) {
var matcher = isA<AnalysisError>();
if (lexeme != null) {
matcher = matcher.having((e) => e.span?.text, 'span.text', lexeme);
}
if (type != null) {
matcher = matcher.having((e) => e.type, 'type', type);
}
return matcher;
}