From 8b6abd7140ffe9d376d1344eb15f522046cefee0 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Mon, 4 May 2020 22:00:41 +0200 Subject: [PATCH] Move referenced table finder to sqlparser package (#537) --- .../analyzer/moor/create_table_reader.dart | 4 +- .../lib/src/analyzer/moor/entity_handler.dart | 2 +- .../analyzer/sql_queries/query_handler.dart | 2 +- .../analyzer/sql_queries/type_mapping.dart | 13 +++- moor_generator/pubspec.yaml | 4 +- sqlparser/CHANGELOG.md | 5 ++ .../analysis/schema/from_create_table.dart | 2 +- .../lib/src/analysis/steps/prepare_ast.dart | 4 ++ .../lib/utils/find_referenced_tables.dart | 56 +++++++++++++++-- sqlparser/pubspec.yaml | 2 +- .../test/analysis/schema/column_test.dart | 2 +- .../schema/from_create_table_test.dart | 7 ++- .../test/analysis/schema/table_test.dart | 2 +- sqlparser/test/engine/module/fts5_test.dart | 16 ++--- .../test/utils/find_referenced_tables.dart | 60 +++++++++++++++++++ 15 files changed, 153 insertions(+), 28 deletions(-) rename moor_generator/lib/src/analyzer/sql_queries/affected_tables_visitor.dart => sqlparser/lib/utils/find_referenced_tables.dart (50%) create mode 100644 sqlparser/test/utils/find_referenced_tables.dart diff --git a/moor_generator/lib/src/analyzer/moor/create_table_reader.dart b/moor_generator/lib/src/analyzer/moor/create_table_reader.dart index c18ce3f5..b8efdea2 100644 --- a/moor_generator/lib/src/analyzer/moor/create_table_reader.dart +++ b/moor_generator/lib/src/analyzer/moor/create_table_reader.dart @@ -16,12 +16,14 @@ class CreateTableReader { final TableInducingStatement stmt; final Step step; + static const _schemaReader = SchemaFromCreateTable(moorExtensions: true); + CreateTableReader(this.stmt, this.step); Future extractTable(TypeMapper mapper) async { Table table; try { - table = SchemaFromCreateTable(moorExtensions: true).read(stmt); + table = _schemaReader.read(stmt); } catch (e) { step.reportError(ErrorInMoorFile( span: stmt.tableNameToken.span, diff --git a/moor_generator/lib/src/analyzer/moor/entity_handler.dart b/moor_generator/lib/src/analyzer/moor/entity_handler.dart index 729bda6e..d9d025fa 100644 --- a/moor_generator/lib/src/analyzer/moor/entity_handler.dart +++ b/moor_generator/lib/src/analyzer/moor/entity_handler.dart @@ -2,10 +2,10 @@ import 'package:moor_generator/moor_generator.dart'; import 'package:moor_generator/src/analyzer/errors.dart'; import 'package:moor_generator/src/analyzer/runner/results.dart'; import 'package:moor_generator/src/analyzer/runner/steps.dart'; -import 'package:moor_generator/src/analyzer/sql_queries/affected_tables_visitor.dart'; import 'package:moor_generator/src/analyzer/sql_queries/lints/linter.dart'; import 'package:moor_generator/src/analyzer/sql_queries/query_analyzer.dart'; import 'package:sqlparser/sqlparser.dart'; +import 'package:sqlparser/utils/find_referenced_tables.dart'; /// Handles `REFERENCES` clauses in tables by resolving their columns and /// reporting errors if they don't exist. Further, sets the diff --git a/moor_generator/lib/src/analyzer/sql_queries/query_handler.dart b/moor_generator/lib/src/analyzer/sql_queries/query_handler.dart index 2f3f4b02..8872bb38 100644 --- a/moor_generator/lib/src/analyzer/sql_queries/query_handler.dart +++ b/moor_generator/lib/src/analyzer/sql_queries/query_handler.dart @@ -3,8 +3,8 @@ import 'package:moor_generator/src/model/used_type_converter.dart'; import 'package:moor_generator/src/analyzer/sql_queries/type_mapping.dart'; import 'package:moor_generator/src/utils/type_converter_hint.dart'; import 'package:sqlparser/sqlparser.dart' hide ResultColumn; +import 'package:sqlparser/utils/find_referenced_tables.dart'; -import 'affected_tables_visitor.dart'; import 'lints/linter.dart'; /// Maps an [AnalysisContext] from the sqlparser to a [SqlQuery] from this diff --git a/moor_generator/lib/src/analyzer/sql_queries/type_mapping.dart b/moor_generator/lib/src/analyzer/sql_queries/type_mapping.dart index 047610fb..f91747f6 100644 --- a/moor_generator/lib/src/analyzer/sql_queries/type_mapping.dart +++ b/moor_generator/lib/src/analyzer/sql_queries/type_mapping.dart @@ -1,8 +1,9 @@ +import 'package:moor/moor.dart' as m; import 'package:moor_generator/moor_generator.dart'; -import 'package:moor_generator/src/analyzer/sql_queries/affected_tables_visitor.dart'; import 'package:moor_generator/src/model/sql_query.dart'; import 'package:moor_generator/src/utils/type_converter_hint.dart'; import 'package:sqlparser/sqlparser.dart'; +import 'package:sqlparser/utils/find_referenced_tables.dart' as s; /// Converts tables and types between the moor_generator and the sqlparser /// library. @@ -224,7 +225,13 @@ class TypeMapper { return _engineTablesToSpecified[table]; } - WrittenMoorTable writtenToMoor(WrittenTable table) { - return WrittenMoorTable(tableToMoor(table.table), table.kind); + WrittenMoorTable writtenToMoor(s.TableWrite table) { + final moorKind = const { + s.UpdateKind.insert: m.UpdateKind.insert, + s.UpdateKind.update: m.UpdateKind.update, + s.UpdateKind.delete: m.UpdateKind.delete, + }[table.kind]; + + return WrittenMoorTable(tableToMoor(table.table), moorKind); } } diff --git a/moor_generator/pubspec.yaml b/moor_generator/pubspec.yaml index c0bdc340..2df3616a 100644 --- a/moor_generator/pubspec.yaml +++ b/moor_generator/pubspec.yaml @@ -1,6 +1,6 @@ name: moor_generator description: Dev-dependency to generate table and dataclasses together with the moor package. -version: 3.0.0 +version: 3.1.0-dev repository: https://github.com/simolus3/moor homepage: https://moor.simonbinder.eu/ issue_tracker: https://github.com/simolus3/moor/issues @@ -23,7 +23,7 @@ dependencies: # Moor-specific analysis moor: ^3.0.0 - sqlparser: ^0.8.0 + sqlparser: ^0.9.0 # Dart analysis analyzer: '^0.39.0' diff --git a/sqlparser/CHANGELOG.md b/sqlparser/CHANGELOG.md index b576a9ef..44d0e04d 100644 --- a/sqlparser/CHANGELOG.md +++ b/sqlparser/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.9.0 + +- New `package:sqlparser/utils/find_referenced_tables.dart` library. Use it to easily find all referenced tables +in a query. + ## 0.8.1 - Support collate expressions in the new type inference ([#533](https://github.com/simolus3/moor/issues/533)) diff --git a/sqlparser/lib/src/analysis/schema/from_create_table.dart b/sqlparser/lib/src/analysis/schema/from_create_table.dart index 74b8b5b9..d3649a3c 100644 --- a/sqlparser/lib/src/analysis/schema/from_create_table.dart +++ b/sqlparser/lib/src/analysis/schema/from_create_table.dart @@ -6,7 +6,7 @@ class SchemaFromCreateTable { /// and `DATETIME` columns. final bool moorExtensions; - SchemaFromCreateTable({this.moorExtensions = false}); + const SchemaFromCreateTable({this.moorExtensions = false}); Table read(TableInducingStatement stmt) { if (stmt is CreateTableStatement) { diff --git a/sqlparser/lib/src/analysis/steps/prepare_ast.dart b/sqlparser/lib/src/analysis/steps/prepare_ast.dart index 66e3e9f5..ec8fc83c 100644 --- a/sqlparser/lib/src/analysis/steps/prepare_ast.dart +++ b/sqlparser/lib/src/analysis/steps/prepare_ast.dart @@ -105,6 +105,10 @@ class AstPreparingVisitor extends RecursiveVisitor { // acts like a table for expressions in the same scope, so let's // register it. if (table.as != null) { + // todo should we register a TableAlias instead? Some parts of this + // package and moor_generator might depend on this being a table + // directly (e.g. nested result sets in moor). + // Same for nested selects, joins and table-valued functions below. scope.register(table.as, table); } }, diff --git a/moor_generator/lib/src/analyzer/sql_queries/affected_tables_visitor.dart b/sqlparser/lib/utils/find_referenced_tables.dart similarity index 50% rename from moor_generator/lib/src/analyzer/sql_queries/affected_tables_visitor.dart rename to sqlparser/lib/utils/find_referenced_tables.dart index 74652850..1550105c 100644 --- a/moor_generator/lib/src/analyzer/sql_queries/affected_tables_visitor.dart +++ b/sqlparser/lib/utils/find_referenced_tables.dart @@ -1,4 +1,5 @@ -import 'package:moor/moor.dart' show UpdateKind; +library utils.find_referenced_tables; + import 'package:sqlparser/sqlparser.dart'; /// An AST-visitor that walks sql statements and finds all tables referenced in @@ -40,11 +41,25 @@ class ReferencedTablesVisitor extends RecursiveVisitor { } } -class WrittenTable { +enum UpdateKind { insert, update, delete } + +/// A write to a table as found while analyzing a statement. +class TableWrite { + /// The table that a statement might write to when run. final Table table; + + /// What kind of update was found (e.g. insert, update or delete). final UpdateKind kind; - WrittenTable(this.table, this.kind); + TableWrite(this.table, this.kind); + + @override + int get hashCode => 37 * table.hashCode + kind.hashCode; + + @override + bool operator ==(dynamic other) { + return other is TableWrite && other.table == table && other.kind == kind; + } } /// Finds all tables that could be affected when executing a query. In @@ -56,12 +71,12 @@ class UpdatedTablesVisitor extends ReferencedTablesVisitor { /// Note that this is a subset of [foundTables], since an updating tables /// could reference tables it's not updating (e.g. with `INSERT INTO foo /// SELECT * FROM bar`). - final Set writtenTables = {}; + final Set writtenTables = {}; void _addIfResolved(ResolvesToResultSet r, UpdateKind kind) { final resolved = _toTableOrNull(r); if (resolved != null) { - writtenTables.add(WrittenTable(resolved, kind)); + writtenTables.add(TableWrite(resolved, kind)); } } @@ -83,3 +98,34 @@ class UpdatedTablesVisitor extends ReferencedTablesVisitor { visitChildren(e, arg); } } + +/// Finds all writes to a table that occur anywhere inside the [root] node or a +/// descendant. +/// +/// The [root] node must have all its references resolved. This means that using +/// a node obtained via [SqlEngine.parse] directly won't report meaningful +/// results. Instead, use [SqlEngine.analyze] or [SqlEngine.analyzeParsed]. +/// +/// If you want to find all referenced tables, use [findReferencedTables]. If +/// you want to find writes (including their [UpdateKind]) and referenced +/// tables, constrct a [UpdatedTablesVisitor] manually. +/// Then, let it [RecursiveVisitor.visit] the [root] node. You can now use +/// [UpdatedTablesVisitor.writtenTables] and +/// [ReferencedTablesVisitor.foundTables]. This will only walk the ast once, +/// whereas calling this and [findReferencedTables] will require two walks. +/// +Set findWrittenTables(AstNode root) { + return (UpdatedTablesVisitor()..visit(root, null)).writtenTables; +} + +/// Finds all tables referenced in [root] or a descendant. +/// +/// The [root] node must have all its references resolved. This means that using +/// a node obtained via [SqlEngine.parse] directly won't report meaningful +/// results. Instead, use [SqlEngine.analyze] or [SqlEngine.analyzeParsed]. +/// +/// If you want to use both [findWrittenTables] and this on the same ast node, +/// follow the advice on [findWrittenTables] to only walk the ast once. +Set findReferencedTables(AstNode root) { + return (ReferencedTablesVisitor()..visit(root, null)).foundTables; +} diff --git a/sqlparser/pubspec.yaml b/sqlparser/pubspec.yaml index d7d55e31..37585f00 100644 --- a/sqlparser/pubspec.yaml +++ b/sqlparser/pubspec.yaml @@ -1,6 +1,6 @@ name: sqlparser description: Parses sqlite statements and performs static analysis on them -version: 0.8.1 +version: 0.9.0-dev homepage: https://github.com/simolus3/moor/tree/develop/sqlparser #homepage: https://moor.simonbinder.eu/ issue_tracker: https://github.com/simolus3/moor/issues diff --git a/sqlparser/test/analysis/schema/column_test.dart b/sqlparser/test/analysis/schema/column_test.dart index 146f9421..4d2f9f82 100644 --- a/sqlparser/test/analysis/schema/column_test.dart +++ b/sqlparser/test/analysis/schema/column_test.dart @@ -4,7 +4,7 @@ import 'package:test/test.dart'; void main() { test('isAliasForRowId', () { final engine = SqlEngine(); - final schemaParser = SchemaFromCreateTable(); + const schemaParser = SchemaFromCreateTable(); final isAlias = { 'CREATE TABLE x (id INTEGER PRIMARY KEY)': true, diff --git a/sqlparser/test/analysis/schema/from_create_table_test.dart b/sqlparser/test/analysis/schema/from_create_table_test.dart index 25d9a6f0..096cdcdf 100644 --- a/sqlparser/test/analysis/schema/from_create_table_test.dart +++ b/sqlparser/test/analysis/schema/from_create_table_test.dart @@ -36,7 +36,7 @@ const _affinityTests = { void main() { test('affinity from typename', () { - final resolver = SchemaFromCreateTable(); + const resolver = SchemaFromCreateTable(); _affinityTests.forEach((key, value) { expect(resolver.columnAffinity(key), equals(value), @@ -48,7 +48,8 @@ void main() { final engine = SqlEngine(); final stmt = engine.parse(createTableStmt).rootNode; - final table = SchemaFromCreateTable().read(stmt as CreateTableStatement); + final table = + const SchemaFromCreateTable().read(stmt as CreateTableStatement); expect(table.resolvedColumns.map((c) => c.name), ['id', 'email', 'score', 'display_name']); @@ -70,7 +71,7 @@ void main() { ) ''').rootNode; - final table = SchemaFromCreateTable(moorExtensions: true) + final table = const SchemaFromCreateTable(moorExtensions: true) .read(stmt as CreateTableStatement); expect(table.resolvedColumns.map((c) => c.type), const [ ResolvedType(type: BasicType.int, hint: IsBoolean(), nullable: true), diff --git a/sqlparser/test/analysis/schema/table_test.dart b/sqlparser/test/analysis/schema/table_test.dart index cf7d4109..a21047f2 100644 --- a/sqlparser/test/analysis/schema/table_test.dart +++ b/sqlparser/test/analysis/schema/table_test.dart @@ -4,7 +4,7 @@ import 'package:test/test.dart'; void main() { group('finds columns', () { final engine = SqlEngine(); - final schemaParser = SchemaFromCreateTable(); + const schemaParser = SchemaFromCreateTable(); Column findWith(String createTbl, String columnName) { final stmt = engine.parse(createTbl).rootNode as CreateTableStatement; diff --git a/sqlparser/test/engine/module/fts5_test.dart b/sqlparser/test/engine/module/fts5_test.dart index 725ed4ad..7f3320b3 100644 --- a/sqlparser/test/engine/module/fts5_test.dart +++ b/sqlparser/test/engine/module/fts5_test.dart @@ -11,8 +11,8 @@ void main() { final result = engine.analyze('CREATE VIRTUAL TABLE foo USING ' "fts5(bar , tokenize = 'porter ascii')"); - final table = - SchemaFromCreateTable().read(result.root as TableInducingStatement); + final table = const SchemaFromCreateTable() + .read(result.root as TableInducingStatement); expect(table.name, 'foo'); final columns = table.resultColumns; @@ -24,8 +24,8 @@ void main() { final result = engine .analyze('CREATE VIRTUAL TABLE foo USING fts5(bar, baz UNINDEXED)'); - final table = - SchemaFromCreateTable().read(result.root as TableInducingStatement); + final table = const SchemaFromCreateTable() + .read(result.root as TableInducingStatement); expect(table.name, 'foo'); expect(table.resultColumns.map((c) => c.name), ['bar', 'baz']); @@ -39,7 +39,7 @@ void main() { // add an fts5 table for the following queries final fts5Result = engine.analyze('CREATE VIRTUAL TABLE foo USING ' 'fts5(bar, baz);'); - engine.registerTable(SchemaFromCreateTable() + engine.registerTable(const SchemaFromCreateTable() .read(fts5Result.root as TableInducingStatement)); }); @@ -83,7 +83,7 @@ void main() { // add an fts5 table for the following queries final fts5Result = engine.analyze('CREATE VIRTUAL TABLE foo USING ' 'fts5(bar, baz);'); - engine.registerTable(SchemaFromCreateTable() + engine.registerTable(const SchemaFromCreateTable() .read(fts5Result.root as TableInducingStatement)); }); @@ -129,11 +129,11 @@ void main() { // add an fts5 table for the following queries final fts5Result = engine.analyze('CREATE VIRTUAL TABLE foo USING ' 'fts5(bar, baz);'); - engine.registerTable(SchemaFromCreateTable() + engine.registerTable(const SchemaFromCreateTable() .read(fts5Result.root as TableInducingStatement)); final normalResult = engine.analyze('CREATE TABLE other (bar TEXT);'); - engine.registerTable(SchemaFromCreateTable() + engine.registerTable(const SchemaFromCreateTable() .read(normalResult.root as TableInducingStatement)); }); diff --git a/sqlparser/test/utils/find_referenced_tables.dart b/sqlparser/test/utils/find_referenced_tables.dart new file mode 100644 index 00000000..a954ec6e --- /dev/null +++ b/sqlparser/test/utils/find_referenced_tables.dart @@ -0,0 +1,60 @@ +import 'package:sqlparser/sqlparser.dart'; +import 'package:sqlparser/utils/find_referenced_tables.dart'; +import 'package:test/test.dart'; + +void main() { + SqlEngine engine; + const schemaReader = SchemaFromCreateTable(); + Table users, logins; + + setUpAll(() { + engine = SqlEngine(); + + Table addTableFromStmt(String create) { + final parsed = engine.parse(create); + final table = schemaReader.read(parsed.rootNode as CreateTableStatement); + + engine.registerTable(table); + return table; + } + + users = addTableFromStmt(''' + CREATE TABLE users ( + id INTEGER NOT NULL PRIMARY KEY, + name TEXT NOT NULL, + ); + '''); + + logins = addTableFromStmt(''' + CREATE TABLE logins ( + user INTEGER NOT NULL REFERENCES users (id), + timestamp INT + ); + '''); + }); + + test('recognizes read tables', () { + final ctx = engine.analyze('SELECT * FROM logins INNER JOIN users u ' + 'ON u.id = logins.user;'); + expect(findReferencedTables(ctx.root), {users, logins}); + }); + + test('resolves aliased tables', () { + final ctx = engine.analyze(''' + CREATE TRIGGER foo AFTER INSERT ON users BEGIN + INSERT INTO logins (user, timestamp) VALUES (new.id, 0); + END; + '''); + final body = (ctx.root as CreateTriggerStatement).action; + + // Users referenced via "new" in body. + expect(findReferencedTables(body), contains(users)); + }); + + test('recognizes written tables', () { + final ctx = engine.analyze('INSERT INTO logins ' + 'SELECT id, CURRENT_TIME FROM users;'); + expect( + findWrittenTables(ctx.root), {TableWrite(logins, UpdateKind.insert)}); + }); +}