Cleanup FieldDescription and throw on resolved tableName in legacy mo…

…de. (#172)

lib/postgres_v3_experimental.dart

@@ -260,15 +260,13 @@ final class PgResultSchema {
 
 final class PgResultColumn {
   final PgDataType type;
-  final String? tableName;
   final int? tableOid;
   final String? columnName;
   final int? columnOid;
   final bool binaryEncoding;
 
   PgResultColumn({
     required this.type,
-    this.tableName,
     this.tableOid,
     this.columnName,
     this.columnOid,
@@ -278,11 +276,7 @@ final class PgResultColumn {
   @override
   String toString() {
     final buffer = StringBuffer('${type.name} ');
-    if (tableName != null && tableName != '') {
-      buffer
-        ..write(tableName)
-        ..write('.');
-    } else if (tableOid != null && tableOid != 0) {
+    if (tableOid != null && tableOid != 0) {
       buffer
         ..write('@$tableOid')
         ..write('.');

lib/src/server_messages.dart

@@ -8,7 +8,6 @@ import 'logical_replication_messages.dart';
 import 'shared_messages.dart';
 import 'time_converters.dart';
 import 'types.dart';
-import 'v2/query.dart';
 
 abstract class ServerMessage extends BaseMessage {}
 
@@ -103,6 +102,53 @@ class BackendKeyMessage extends ServerMessage {
   }
 }
 
+class FieldDescription {
+  final String fieldName;
+  final int tableOid;
+  final int columnOid;
+  final int typeOid;
+  final int typeSize;
+  final int typeModifier;
+  final int formatCode;
+
+  FieldDescription._(
+    this.fieldName,
+    this.tableOid,
+    this.columnOid,
+    this.typeOid,
+    this.typeSize,
+    this.typeModifier,
+    this.formatCode,
+  );
+
+  factory FieldDescription.read(PgByteDataReader reader) {
+    final fieldName = reader.readNullTerminatedString();
+    final tableOid = reader.readUint32();
+    final columnOid = reader.readUint16();
+    final typeOid = reader.readUint32();
+    final dataTypeSize = reader.readUint16();
+    final typeModifier = reader.readInt32();
+    final formatCode = reader.readUint16();
+
+    return FieldDescription._(
+      fieldName,
+      tableOid,
+      columnOid,
+      typeOid,
+      dataTypeSize,
+      typeModifier,
+      formatCode,
+    );
+  }
+
+  late final isBinaryEncoding = formatCode != 0;
+
+  @override
+  String toString() {
+    return '$fieldName $tableOid $columnOid $typeOid $typeSize $typeModifier $formatCode';
+  }
+}
+
 class RowDescriptionMessage extends ServerMessage {
   final fieldDescriptions = <FieldDescription>[];
 

lib/src/v2/connection.dart

@@ -1,12 +1,12 @@
 library postgres.connection;
 
 import 'dart:async';
-import 'dart:collection';
 import 'dart:convert';
 import 'dart:io';
 import 'dart:typed_data';
 
 import 'package:buffer/buffer.dart';
+import 'package:collection/collection.dart';
 import 'package:meta/meta.dart';
 import '../auth/auth.dart';
 
@@ -416,16 +416,14 @@ class _OidCache {
     _tableOIDNameMap.clear();
   }
 
-  Future<List<FieldDescription>> _resolveTableNames(
-      _PostgreSQLExecutionContextMixin c,
-      List<FieldDescription?>? columns) async {
+  Future<List<String?>> _resolveTableNames(_PostgreSQLExecutionContextMixin c,
+      List<FieldDescription>? columns) async {
     if (columns == null) return [];
     //todo (joeconwaystk): If this was a cached query, resolving is table oids is unnecessary.
     // It's not a significant impact here, but an area for optimization. This includes
     // assigning resolvedTableName
     final unresolvedTableOIDs = columns
-        .where((f) => f != null)
-        .map((f) => f!.tableID)
+        .map((f) => f.tableOid)
         .toSet()
         .where((oid) => oid > 0 && !_tableOIDNameMap.containsKey(oid))
         .toList()
@@ -435,9 +433,7 @@ class _OidCache {
       await _resolveTableOIDs(c, unresolvedTableOIDs);
     }
 
-    return columns
-        .map((c) => c!.change(tableName: _tableOIDNameMap[c.tableID]))
-        .toList();
+    return columns.map((c) => _tableOIDNameMap[c.tableOid]).toList();
   }
 
   Future _resolveTableOIDs(
@@ -526,15 +522,19 @@ abstract mixin class _PostgreSQLExecutionContextMixin
 
     final queryResult =
         await _enqueue(query, timeoutInSeconds: timeoutInSeconds);
-    var fieldDescriptions = query.fieldDescriptions;
+    var tableNames = const <String?>[];
     if (resolveOids) {
-      fieldDescriptions = await _connection._oidCache
-          ._resolveTableNames(this, fieldDescriptions);
+      tableNames = await _connection._oidCache
+          ._resolveTableNames(this, query.fieldDescriptions);
     }
-    final metaData = _PostgreSQLResultMetaData(fieldDescriptions!
-        .map((e) => ColumnDescription(
-            typeId: e.typeId, tableName: e.tableName, columnName: e.columnName))
-        .toList());
+    final metaData =
+        _PostgreSQLResultMetaData((query.fieldDescriptions ?? const [])
+            .mapIndexed((i, e) => ColumnDescription(
+                  typeId: e.typeOid,
+                  tableName: tableNames.isEmpty ? '' : tableNames[i] ?? '',
+                  columnName: e.fieldName,
+                ))
+            .toList());
 
     return _PostgreSQLResult(
         queryResult.affectedRowCount,
@@ -604,7 +604,10 @@ abstract mixin class _PostgreSQLExecutionContextMixin
     final fieldDescriptions = query.fieldDescriptions ?? [];
     final metaData = _PostgreSQLResultMetaData(fieldDescriptions
         .map((e) => ColumnDescription(
-            typeId: e.typeId, tableName: e.tableName, columnName: e.columnName))
+              typeId: e.typeOid,
+              tableName: '',
+              columnName: e.fieldName,
+            ))
         .toList());
 
     final value = result.value;

lib/src/v2/query.dart

@@ -2,11 +2,10 @@ import 'dart:async';
 import 'dart:io';
 import 'dart:typed_data';
 
-import 'package:buffer/buffer.dart';
-
 import '../binary_codec.dart';
 import '../client_messages.dart';
 import '../exceptions.dart';
+import '../server_messages.dart';
 import '../types.dart';
 import 'connection.dart';
 import 'execution_context.dart';
@@ -168,7 +167,10 @@ class Query<T> {
     final iterator = fieldDescriptions!.iterator;
     final lazyDecodedData = rawRowData.map((bd) {
       iterator.moveNext();
-      return iterator.current.converter.convert(bd, connection.encoding);
+      final converter = PostgresBinaryDecoder(
+          PgDataType.byTypeOid[iterator.current.typeOid] ??
+              PgDataType.unknownType);
+      return converter.convert(bd, connection.encoding);
     });
 
     rows.add(lazyDecodedData.toList());
@@ -232,69 +234,6 @@ class ParameterValue extends PgTypedParameter {
       : super(type ?? PgDataType.unspecified, value);
 }
 
-class FieldDescription {
-  final PostgresBinaryDecoder converter;
-
-  final String columnName;
-  final int tableID;
-  final int columnID;
-  final int typeId;
-  final int dataTypeSize;
-  final int typeModifier;
-  final int formatCode;
-  final String tableName;
-
-  FieldDescription._(
-    this.converter,
-    this.columnName,
-    this.tableID,
-    this.columnID,
-    this.typeId,
-    this.dataTypeSize,
-    this.typeModifier,
-    this.formatCode,
-    this.tableName,
-  );
-
-  factory FieldDescription.read(ByteDataReader reader) {
-    final buf = StringBuffer();
-    var byte = 0;
-    do {
-      byte = reader.readUint8();
-      if (byte != 0) {
-        buf.writeCharCode(byte);
-      }
-    } while (byte != 0);
-
-    final fieldName = buf.toString();
-
-    final tableID = reader.readUint32();
-    final columnID = reader.readUint16();
-    final typeOid = reader.readUint32();
-    final dataTypeSize = reader.readUint16();
-    final typeModifier = reader.readInt32();
-    final formatCode = reader.readUint16();
-
-    final converter = PostgresBinaryDecoder(
-        PgDataType.byTypeOid[typeOid] ?? PgDataType.unknownType);
-    return FieldDescription._(
-      converter, fieldName, tableID, columnID, typeOid,
-      dataTypeSize, typeModifier, formatCode,
-      '', // tableName
-    );
-  }
-
-  FieldDescription change({String? tableName}) {
-    return FieldDescription._(converter, columnName, tableID, columnID, typeId,
-        dataTypeSize, typeModifier, formatCode, tableName ?? this.tableName);
-  }
-
-  @override
-  String toString() {
-    return '$columnName $tableID $columnID $typeId $dataTypeSize $typeModifier $formatCode';
-  }
-}
-
 typedef SQLReplaceIdentifierFunction = String Function(
     PostgreSQLFormatIdentifier identifier, int index);
 

lib/src/v2/v2_v3_delegate.dart

@@ -252,9 +252,8 @@ class _PostgreSQLResult extends UnmodifiableListView<PostgreSQLResultRow>
 
   @override
   late final columnDescriptions = _result.schema.columns
-      .map((e) => ColumnDescription(
+      .map((e) => _ColumnDescription(
             typeId: e.type.oid ?? 0,
-            tableName: e.tableName ?? '',
             columnName: e.columnName ?? '',
           ))
       .toList();
@@ -267,9 +266,8 @@ class _PostgreSQLResultRow extends UnmodifiableListView
 
   @override
   late final columnDescriptions = _row.schema.columns
-      .map((e) => ColumnDescription(
+      .map((e) => _ColumnDescription(
             typeId: e.type.oid ?? 0,
-            tableName: e.tableName ?? '',
             columnName: e.columnName ?? '',
           ))
       .toList();
@@ -288,18 +286,7 @@ class _PostgreSQLResultRow extends UnmodifiableListView
 
   @override
   Map<String, Map<String, dynamic>> toTableColumnMap() {
-    final tables = <String, Map<String, dynamic>>{};
-
-    for (final (i, col) in _row.schema.columns.indexed) {
-      final tableName = col.tableName;
-      final columnName = col.columnName;
-
-      if (tableName != null && columnName != null) {
-        tables.putIfAbsent(tableName, () => {})[columnName] = _row[i];
-      }
-    }
-
-    return tables;
+    throw UnimplementedError('toTableColumnMap is not supported in v3');
   }
 }
 
@@ -311,3 +298,20 @@ class _PostgreSQLExecutionContext
 
   _PostgreSQLExecutionContext(this._session);
 }
+
+class _ColumnDescription implements ColumnDescription {
+  @override
+  final String columnName;
+
+  @override
+  String get tableName =>
+      throw UnimplementedError('table name is resolved in v3');
+
+  @override
+  final int typeId;
+
+  _ColumnDescription({
+    required this.columnName,
+    required this.typeId,
+  });
+}

lib/src/v3/connection.dart

@@ -630,12 +630,11 @@ class _PgResultStreamSubscription
         final schema = _resultSchema = PgResultSchema([
           for (final field in message.fieldDescriptions)
             PgResultColumn(
-              type: PgDataType.byTypeOid[field.typeId] ?? PgDataType.byteArray,
-              tableName: field.tableName,
-              columnName: field.columnName,
-              columnOid: field.columnID,
-              tableOid: field.tableID,
-              binaryEncoding: field.formatCode != 0,
+              type: PgDataType.byTypeOid[field.typeOid] ?? PgDataType.byteArray,
+              columnName: field.fieldName,
+              columnOid: field.columnOid,
+              tableOid: field.tableOid,
+              binaryEncoding: field.isBinaryEncoding,
             ),
         ]);
         _schema.complete(schema);

test/map_return_test.dart

@@ -129,7 +129,7 @@ void main() {
           '': {'?column?': 1}
         }
       ]);
-    });
+    }, skip: server.skippedOnV3('mappedResultsQuery is a removed feature'));
   });
 }
 

test/query_test.dart

@@ -42,10 +42,12 @@ void main() {
 
       result = await connection.query('select t from t');
       expect(result.columnDescriptions, hasLength(1));
-      expect(
-          result.columnDescriptions.single.tableName,
-          // v3 does not query the table oids
-          server.useV3 ? '' : 't');
+      if (server.useV3) {
+        expect(() => result.columnDescriptions.single.tableName,
+            throwsUnimplementedError);
+      } else {
+        expect(result.columnDescriptions.single.tableName, 't');
+      }
       expect(result.columnDescriptions.single.columnName, 't');
       expect(result, [expectedRow]);
     });
@@ -199,15 +201,13 @@ void main() {
         [false, true, false]
       ];
       expect(result.columnDescriptions, hasLength(24));
-      expect(
-          result.columnDescriptions.first.tableName,
-          // v3 does not query the table oids
-          server.useV3 ? '' : 't');
+      if (server.useV3) {
+        expect(() => result.columnDescriptions.first.tableName,
+            throwsUnimplementedError);
+      } else {
+        expect(result.columnDescriptions.first.tableName, 't');
+      }
       expect(result.columnDescriptions.first.columnName, 'i');
-      expect(
-          result.columnDescriptions.last.tableName,
-          // v3 does not query the table oids
-          server.useV3 ? '' : 't');
       expect(result.columnDescriptions.last.columnName, 'ba');
       expect(result, [expectedRow]);
       result = await connection.query(