From 30e43141daae1fd2a4e957a5f7dd065305dbe1ab Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Tue, 5 Dec 2023 00:27:22 +0100 Subject: [PATCH] Pass TypeRegistry into Sql tokenization. --- CHANGELOG.md | 2 + lib/postgres.dart | 9 ++-- lib/src/v3/connection.dart | 10 +++- lib/src/v3/query_description.dart | 85 +++++++++++++++++++++++++----- lib/src/v3/variable_tokenizer.dart | 10 +++- test/pool_test.dart | 2 +- 6 files changed, 93 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 475f3a31..b8bcb368 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## 3.0.3 - Using const for ConnectionSettings, SessionSettings and PoolSettings classes. ([#267](https://github.com/isoos/postgresql-dart/pull/267) by [Gerrel](https://github.com/Gerrel)) +- Parsing of `Sql.indexed` and `Sql.named` happens when the `Connection` starts + to interpret it. Errors with unknown type names are thrown in this later step. ## 3.0.2 diff --git a/lib/postgres.dart b/lib/postgres.dart index 11ff2adc..001776c2 100644 --- a/lib/postgres.dart +++ b/lib/postgres.dart @@ -43,8 +43,7 @@ class Sql { /// The [types] parameter can optionally be used to pass the types of /// parameters in the query. If they're not set, only [TypedValue] /// instances can be used when binding values later. - factory Sql(String sql, {List? types}) = - InternalQueryDescription.direct; + factory Sql(String sql, {List? types}) = SqlImpl.direct; /// Looks for positional parameters in [sql] and desugars them. /// @@ -62,8 +61,7 @@ class Sql { /// /// Just like with [Sql.named], it is possible to declare an explicit type for /// variables: `Sql.indexed('SELECT @1:int8')`. - factory Sql.indexed(String sql, {String substitution}) = - InternalQueryDescription.indexed; + factory Sql.indexed(String sql, {String substitution}) = SqlImpl.indexed; /// Looks for named parameters in [sql] and desugars them. /// @@ -106,8 +104,7 @@ class Sql { /// Also, the scanner might interpret queries incorrectly in the case of /// malformed [sql] (like an unterminated string literal or comment). In that /// case, the transformation might not recognize all variables. - factory Sql.named(String sql, {String substitution}) = - InternalQueryDescription.named; + factory Sql.named(String sql, {String substitution}) = SqlImpl.named; } abstract class Session { diff --git a/lib/src/v3/connection.dart b/lib/src/v3/connection.dart index fe971e92..4ef2b5a2 100644 --- a/lib/src/v3/connection.dart +++ b/lib/src/v3/connection.dart @@ -113,7 +113,10 @@ abstract class _PgSessionBase implements Session { throw PgException( 'Attempting to execute query, but connection is not open.'); } - final description = InternalQueryDescription.wrap(query); + final description = InternalQueryDescription.wrap( + query, + typeRegistry: _connection._settings.typeRegistry, + ); final variables = description.bindParameters( parameters, ignoreSuperfluous: _settings.ignoreSuperfluousParameters, @@ -171,7 +174,10 @@ abstract class _PgSessionBase implements Session { }) async { final conn = _connection; final name = 's/${conn._statementCounter++}'; - final description = InternalQueryDescription.wrap(query); + final description = InternalQueryDescription.wrap( + query, + typeRegistry: _connection._settings.typeRegistry, + ); await _sendAndWaitForQuery(ParseMessage( description.transformedSql, diff --git a/lib/src/v3/query_description.dart b/lib/src/v3/query_description.dart index ab5f480b..7e7b0ce4 100644 --- a/lib/src/v3/query_description.dart +++ b/lib/src/v3/query_description.dart @@ -1,7 +1,26 @@ import '../../postgres.dart'; import 'variable_tokenizer.dart'; -class InternalQueryDescription implements Sql { +class SqlImpl implements Sql { + final String sql; + final TokenizerMode mode; + final String substitution; + final List? types; + + SqlImpl.direct(this.sql, {this.types}) + : mode = TokenizerMode.none, + substitution = ''; + + SqlImpl.indexed(this.sql, {this.substitution = '@'}) + : mode = TokenizerMode.indexed, + types = null; + + SqlImpl.named(this.sql, {this.substitution = '@'}) + : mode = TokenizerMode.named, + types = null; +} + +class InternalQueryDescription { /// The SQL to send to postgres. /// /// This is the [originalSql] statement after local processing ran to @@ -36,39 +55,77 @@ class InternalQueryDescription implements Sql { namedVariables, ); - factory InternalQueryDescription.indexed(String sql, - {String substitution = '@'}) { - return _viaTokenizer(sql, substitution, TokenizerMode.indexed); + factory InternalQueryDescription.indexed( + String sql, { + String substitution = '@', + TypeRegistry? typeRegistry, + }) { + return _viaTokenizer(typeRegistry ?? TypeRegistry(), sql, substitution, + TokenizerMode.indexed); } - factory InternalQueryDescription.named(String sql, - {String substitution = '@'}) { - return _viaTokenizer(sql, substitution, TokenizerMode.named); + factory InternalQueryDescription.named( + String sql, { + String substitution = '@', + TypeRegistry? typeRegistry, + }) { + return _viaTokenizer( + typeRegistry ?? TypeRegistry(), sql, substitution, TokenizerMode.named); } static InternalQueryDescription _viaTokenizer( - String sql, String substitution, TokenizerMode mode) { + TypeRegistry typeRegistry, + String sql, + String substitution, + TokenizerMode mode, + ) { final charCodes = substitution.codeUnits; if (charCodes.length != 1) { throw ArgumentError.value(substitution, 'substitution', 'Must be a string with a single code unit'); } - final tokenizer = - VariableTokenizer(variableCodeUnit: charCodes[0], sql: sql, mode: mode) - ..tokenize(); + final tokenizer = VariableTokenizer( + typeRegistry: typeRegistry, + variableCodeUnit: charCodes[0], + sql: sql, + mode: mode, + )..tokenize(); return tokenizer.result; } - factory InternalQueryDescription.wrap(Object query) { + factory InternalQueryDescription.wrap( + Object query, { + required TypeRegistry typeRegistry, + }) { if (query is String) { return InternalQueryDescription.direct(query); } else if (query is InternalQueryDescription) { return query; + } else if (query is SqlImpl) { + switch (query.mode) { + case TokenizerMode.none: + return InternalQueryDescription.direct( + query.sql, + types: query.types, + ); + case TokenizerMode.indexed: + return InternalQueryDescription.indexed( + query.sql, + substitution: query.substitution, + typeRegistry: typeRegistry, + ); + case TokenizerMode.named: + return InternalQueryDescription.named( + query.sql, + substitution: query.substitution, + typeRegistry: typeRegistry, + ); + } } else { - throw ArgumentError.value(query, 'query', - 'Must either be a String or an InternalQueryDescription'); + throw ArgumentError.value( + query, 'query', 'Must either be a String or an SqlImpl'); } } diff --git a/lib/src/v3/variable_tokenizer.dart b/lib/src/v3/variable_tokenizer.dart index 3d7cf539..48f792e3 100644 --- a/lib/src/v3/variable_tokenizer.dart +++ b/lib/src/v3/variable_tokenizer.dart @@ -11,6 +11,7 @@ import '../types.dart'; import 'query_description.dart'; enum TokenizerMode { + none, named, indexed, } @@ -30,6 +31,9 @@ enum TokenizerMode { /// Just like postgres, we ignore variables inside string literals, identifiers /// or comments. class VariableTokenizer { + /// The TypeRegistry of the connection. + final TypeRegistry typeRegistry; + /// A map from variable names (without the [_variableCodeUnit]) to their /// resolved index of the underlying SQL parameter. final Map _namedVariables = {}; @@ -59,6 +63,7 @@ class VariableTokenizer { bool get _isAtEnd => _index == _codeUnits.length; VariableTokenizer({ + required this.typeRegistry, int variableCodeUnit = $at, required String sql, required this.mode, @@ -372,6 +377,8 @@ class VariableTokenizer { int actualVariableIndex; switch (mode) { + case TokenizerMode.none: + throw ArgumentError('Did not expect $mode.'); case TokenizerMode.named: actualVariableIndex = _namedVariables.putIfAbsent( nameBuffer.toString(), () => _namedVariables.length + 1); @@ -386,8 +393,7 @@ class VariableTokenizer { if (consumedColonForType) { final typeName = typeBuffer.toString(); - // TODO: get this from the connection settings - final type = TypeRegistry().resolveSubstitution(typeName); + final type = typeRegistry.resolveSubstitution(typeName); if (type == null) { error('Unknown type: $typeName'); } diff --git a/test/pool_test.dart b/test/pool_test.dart index be7fc708..cea69e73 100644 --- a/test/pool_test.dart +++ b/test/pool_test.dart @@ -135,7 +135,7 @@ void main() { ), ); - // this doesn't throw but it causes the connection to close + // this doesn't throw but it causes the connection to close await db.execute('-- test'); await db.execute('SELECT 1'); });