Require binary format in binary importer (#5648)

(cherry picked from commit f320515)

src/Npgsql/Internal/PgTypeInfo.cs

@@ -114,24 +114,27 @@ internal PgConverterResolution GetResolution()
         return new(Converter, PgTypeId.GetValueOrDefault());
     }
 
-    bool CachedCanConvert(DataFormat format, out BufferRequirements bufferRequirements)
+    bool CanConvert(PgConverter converter, DataFormat format, out BufferRequirements bufferRequirements)
     {
-        if (format is DataFormat.Binary)
+        if (HasCachedInfo(converter))
         {
-            bufferRequirements = _binaryBufferRequirements;
-            return _canBinaryConvert;
+            switch (format)
+            {
+            case DataFormat.Binary:
+                bufferRequirements = _binaryBufferRequirements;
+                return _canBinaryConvert;
+            case DataFormat.Text:
+                bufferRequirements = _textBufferRequirements;
+                return _canTextConvert;
+            }
         }
 
-        bufferRequirements = _textBufferRequirements;
-        return _canTextConvert;
+        return converter.CanConvert(format, out bufferRequirements);
     }
 
     public BufferRequirements? GetBufferRequirements(PgConverter converter, DataFormat format)
     {
-        var success = HasCachedInfo(converter)
-            ? CachedCanConvert(format, out var bufferRequirements)
-            : converter.CanConvert(format, out bufferRequirements);
-
+        var success = CanConvert(converter, format, out var bufferRequirements);
         return success ? bufferRequirements : null;
     }
 
@@ -141,7 +144,7 @@ internal bool TryBind(Field field, DataFormat format, out PgConverterInfo info)
         switch (this)
         {
         case { IsResolverInfo: false }:
-            if (!CachedCanConvert(format, out var bufferRequirements))
+            if (!CanConvert(Converter, format, out var bufferRequirements))
             {
                 info = default;
                 return false;
@@ -150,9 +153,7 @@ internal bool TryBind(Field field, DataFormat format, out PgConverterInfo info)
             return true;
         case PgResolverTypeInfo resolverInfo:
             var resolution = resolverInfo.GetResolution(field);
-            if (HasCachedInfo(resolution.Converter)
-                    ? !CachedCanConvert(format, out bufferRequirements)
-                    : !resolution.Converter.CanConvert(format, out bufferRequirements))
+            if (!CanConvert(resolution.Converter, format, out bufferRequirements))
             {
                 info = default;
                 return false;
@@ -217,27 +218,25 @@ internal PgConverterInfo Bind(Field field, DataFormat format)
         return new(this, converter, bufferRequirements.Write);
     }
 
-    // If we don't have a converter stored we must ask the retrieved one.
     DataFormat ResolveFormat(PgConverter converter, out BufferRequirements bufferRequirements, DataFormat? formatPreference = null)
     {
+        // First try to check for preferred support.
         switch (formatPreference)
         {
-        // The common case, no preference means we default to binary if supported.
-        case null or DataFormat.Binary when HasCachedInfo(converter) ? CachedCanConvert(DataFormat.Binary, out bufferRequirements) : converter.CanConvert(DataFormat.Binary, out bufferRequirements):
+        case DataFormat.Binary when CanConvert(converter, DataFormat.Binary, out bufferRequirements):
             return DataFormat.Binary;
-        // In this case we either prefer text or we have no preference and our converter doesn't support binary.
-        case null or DataFormat.Text:
-            var canTextConvert = HasCachedInfo(converter) ? CachedCanConvert(DataFormat.Text, out bufferRequirements) : converter.CanConvert(DataFormat.Text, out bufferRequirements);
-            if (!canTextConvert)
-            {
-                if (formatPreference is null)
-                    throw new InvalidOperationException("Converter doesn't support any data format.");
-                // Rerun without preference.
-                return ResolveFormat(converter, out bufferRequirements);
-            }
+        case DataFormat.Text when CanConvert(converter, DataFormat.Text, out bufferRequirements):
             return DataFormat.Text;
         default:
-            throw new ArgumentOutOfRangeException();
+            // The common case, no preference given (or no match) means we default to binary if supported.
+            if (CanConvert(converter, DataFormat.Binary, out bufferRequirements))
+                return DataFormat.Binary;
+            if (CanConvert(converter, DataFormat.Text, out bufferRequirements))
+                return DataFormat.Text;
+
+            ThrowHelper.ThrowInvalidOperationException("Converter doesn't support any data format.");
+            bufferRequirements = default;
+            return default;
         }
     }
 }

src/Npgsql/NpgsqlBinaryImporter.cs

@@ -296,7 +296,7 @@ async Task Core(bool async, T value, NpgsqlDbType? npgsqlDbType, string? dataTyp
             if (newParam)
                 _params[_column] = param;
 
-            param.Bind(out _, out _);
+            param.Bind(out _, out _, requiredFormat: DataFormat.Binary);
 
             try
             {

src/Npgsql/NpgsqlParameter.cs

@@ -617,7 +617,7 @@ private protected virtual PgConverterResolution ResolveConverter(PgTypeInfo type
     }
 
     /// Bind the current value to the type info, truncate (if applicable), take its size, and do any final validation before writing.
-    internal void Bind(out DataFormat format, out Size size)
+    internal void Bind(out DataFormat format, out Size size, DataFormat? requiredFormat = null)
     {
         if (TypeInfo is null)
             ThrowHelper.ThrowInvalidOperationException($"Missing type info, {nameof(ResolveTypeInfo)} needs to be called before {nameof(Bind)}.");
@@ -626,19 +626,18 @@ internal void Bind(out DataFormat format, out Size size)
             ThrowHelper.ThrowNotSupportedException($"Cannot write values for parameters of type '{TypeInfo.Type}' and postgres type '{TypeInfo.Options.DatabaseInfo.GetDataTypeName(PgTypeId).DisplayName}'.");
 
         // We might call this twice, once during validation and once during WriteBind, only compute things once.
-        if (WriteSize is not null)
+        if (WriteSize is null)
         {
-            format = Format;
-            size = WriteSize.Value;
-            return;
-        }
+            if (_size > 0)
+                HandleSizeTruncation();
 
-        if (_size > 0)
-            HandleSizeTruncation();
+            BindCore(requiredFormat);
+        }
 
-        BindCore();
         format = Format;
         size = WriteSize!.Value;
+        if (requiredFormat is not null && format != requiredFormat)
+            ThrowHelper.ThrowNotSupportedException($"Parameter '{ParameterName}' must be written in {requiredFormat} format, but does not support this format.");
 
         // Handle Size truncate behavior for a predetermined set of types and pg types.
         // Doesn't matter if we 'box' Value, all supported types are reference types.
@@ -678,7 +677,7 @@ void HandleSizeTruncation()
         }
     }
 
-    private protected virtual void BindCore(bool allowNullReference = false)
+    private protected virtual void BindCore(DataFormat? formatPreference, bool allowNullReference = false)
     {
         // Pull from Value so we also support object typed generic params.
         var value = Value;
@@ -688,7 +687,7 @@ private protected virtual void BindCore(bool allowNullReference = false)
         if (_useSubStream && value is not null)
             value = _subStream = new SubReadStream((Stream)value, _size);
 
-        if (TypeInfo!.BindObject(Converter!, value, out var size, out _writeState, out var dataFormat) is { } info)
+        if (TypeInfo!.BindObject(Converter!, value, out var size, out _writeState, out var dataFormat, formatPreference) is { } info)
         {
             WriteSize = size;
             _bufferRequirement = info.BufferRequirement;
@@ -698,6 +697,7 @@ private protected virtual void BindCore(bool allowNullReference = false)
             WriteSize = -1;
             _bufferRequirement = default;
         }
+
         Format = dataFormat;
     }
 

src/Npgsql/NpgsqlParameter`.cs

@@ -91,17 +91,17 @@ private protected override PgConverterResolution ResolveConverter(PgTypeInfo typ
     }
 
     // We ignore allowNullReference, it's just there to control the base implementation.
-    private protected override void BindCore(bool allowNullReference = false)
+    private protected override void BindCore(DataFormat? formatPreference, bool allowNullReference = false)
     {
         if (_asObject)
         {
             // If we're object typed we should not support null.
-            base.BindCore(typeof(T) != typeof(object));
+            base.BindCore(formatPreference, typeof(T) != typeof(object));
             return;
         }
 
         var value = TypedValue;
-        if (TypeInfo!.Bind(Converter!.UnsafeDowncast<T>(), value, out var size, out _writeState, out var dataFormat) is { } info)
+        if (TypeInfo!.Bind(Converter!.UnsafeDowncast<T>(), value, out var size, out _writeState, out var dataFormat, formatPreference) is { } info)
         {
             WriteSize = size;
             _bufferRequirement = info.BufferRequirement;
@@ -111,6 +111,7 @@ private protected override void BindCore(bool allowNullReference = false)
             WriteSize = -1;
             _bufferRequirement = default;
         }
+
         Format = dataFormat;
     }