Skip to content

Commit

Permalink
Remove string writing with FormatCode text
Browse files Browse the repository at this point in the history
Closes #1841
  • Loading branch information
roji committed Mar 12, 2018
1 parent c7d5a27 commit 90ec9cb
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 117 deletions.
38 changes: 8 additions & 30 deletions src/Npgsql/FrontendMessages/BindMessage.cs
Expand Up @@ -76,63 +76,41 @@ internal override async Task Write(NpgsqlWriteBuffer buf, bool async)
4 + // Message length
1 + // Portal is always empty (only a null terminator)
Statement.Length + 1 +
2; // Number of parameter format codes that follow
2 + // Number of parameter format codes that follow
2 + // "List" of parameter format codes (always 1, we only support binary)
2; // Number of parameters

if (buf.WriteSpaceLeft < headerLength)
{
Debug.Assert(buf.Size >= headerLength, "Buffer too small for Bind header");
await buf.Flush(async);
}

var formatCodesSum = 0;
var paramsLength = 0;
foreach (var p in InputParameters)
{
formatCodesSum += (int)p.FormatCode;
p.LengthCache?.Rewind();
paramsLength += p.ValidateAndGetLength();
}

var formatCodeListLength = formatCodesSum == 0 ? 0 : formatCodesSum == InputParameters.Count ? 1 : InputParameters.Count;

var messageLength = headerLength +
2 * formatCodeListLength + // List of format codes
2 + // Number of parameters
4 * InputParameters.Count + // Parameter lengths
paramsLength + // Parameter values
2 + // Number of result format codes
2 * (UnknownResultTypeList?.Length ?? 1); // Result format codes

// Write the header
buf.WriteByte(Code);
buf.WriteInt32(messageLength - 1);
Debug.Assert(Portal == string.Empty);
buf.WriteByte(0); // Portal is always empty

buf.WriteNullTerminatedString(Statement);
buf.WriteInt16(formatCodeListLength);

// 0 length implicitly means all-text, 1 means all-binary, >1 means mix-and-match
if (formatCodeListLength == 1)
{
if (buf.WriteSpaceLeft < 2)
await buf.Flush(async);
buf.WriteInt16((short)FormatCode.Binary);
}
else if (formatCodeListLength > 1)
{
foreach (var p in InputParameters)
{
if (buf.WriteSpaceLeft < 2)
await buf.Flush(async);
buf.WriteInt16((short)p.FormatCode);
}
}

if (buf.WriteSpaceLeft < 2)
await buf.Flush(async);

buf.WriteNullTerminatedString(Statement);
buf.WriteInt16(1); // Number of format codes
buf.WriteInt16((short)FormatCode.Binary); // Binary format code for all parameters, always
buf.WriteInt16(InputParameters.Count);

// Write the rest of the message
foreach (var param in InputParameters)
{
param.LengthCache?.Rewind();
Expand Down
5 changes: 1 addition & 4 deletions src/Npgsql/NpgsqlParameter.cs
Expand Up @@ -71,8 +71,6 @@ public class NpgsqlParameter : DbParameter, ICloneable
[CanBeNull]
internal NpgsqlTypeHandler Handler { get; set; }

internal FormatCode FormatCode { get; private set; }

#endregion

#region Constructors
Expand Down Expand Up @@ -536,7 +534,6 @@ internal void Bind(ConnectorTypeMapper typeMapper)
{
ResolveHandler(typeMapper);
Debug.Assert(Handler != null);
FormatCode = Handler.PreferTextWrite ? FormatCode.Text : FormatCode.Binary;
}

internal virtual int ValidateAndGetLength()
Expand Down Expand Up @@ -607,7 +604,7 @@ public NpgsqlParameter Clone()
}

object ICloneable.Clone() => Clone();

#endregion
}
}
2 changes: 0 additions & 2 deletions src/Npgsql/TypeHandlers/JsonbHandler.cs
Expand Up @@ -49,8 +49,6 @@ public class JsonbHandler : TextHandler
/// </summary>
const byte JsonbProtocolVersion = 1;

internal override bool PreferTextWrite => false;

protected internal JsonbHandler(NpgsqlConnection connection) : base(connection) { }

#region Write
Expand Down
4 changes: 0 additions & 4 deletions src/Npgsql/TypeHandlers/TextHandler.cs
Expand Up @@ -64,10 +64,6 @@ public class TextHandler : NpgsqlTypeHandler<string>, INpgsqlTypeHandler<char[]>
internal const uint CharTypeOID = 18;
internal const uint UnknownTypeOID = 705;

// Text types are handled a bit more efficiently when sent as text than as binary
// see https://github.com/npgsql/npgsql/issues/1210#issuecomment-235641670
internal override bool PreferTextWrite => true;

readonly Encoding _encoding;

#region State
Expand Down
28 changes: 7 additions & 21 deletions src/Npgsql/TypeHandlers/UnknownTypeHandler.cs
Expand Up @@ -78,32 +78,18 @@ public override ValueTask<string> Read(NpgsqlReadBuffer buf, int byteLen, bool a
public override int ValidateAndGetLength<T2>(T2 value, ref NpgsqlLengthCache lengthCache, NpgsqlParameter parameter)
=> ValidateObjectAndGetLength(value, ref lengthCache, parameter);

//protected override Task Write<T2>(T2 value, NpgsqlWriteBuffer buf, NpgsqlLengthCache lengthCache, NpgsqlParameter parameter, bool async)
// => WriteObjectWithLength(value, buf, lengthCache, parameter, async);

protected internal override int ValidateObjectAndGetLength(object value, ref NpgsqlLengthCache lengthCache, NpgsqlParameter parameter)
{
if (value is string asString)
return base.ValidateAndGetLength(asString, ref lengthCache, parameter);

var converted = Convert.ToString(value);
if (parameter == null)
throw CreateConversionButNoParamException(value.GetType());
parameter.ConvertedValue = converted;
return base.ValidateAndGetLength(converted, ref lengthCache, parameter);
throw new NotSupportedException(
"Parameters with NpgsqlDbType.Unknown are no longer supported. " +
"If you're writing enums as strings, see the section on unmapped enums in the documentation.");
}

protected internal override Task WriteObjectWithLength(object value, NpgsqlWriteBuffer buf, NpgsqlLengthCache lengthCache, NpgsqlParameter parameter, bool async)
public override Task Write(string value, NpgsqlWriteBuffer buf, NpgsqlLengthCache lengthCache, NpgsqlParameter parameter, bool async)
{
if (value == null || value is DBNull)
return base.WriteObjectWithLength(value, buf, lengthCache, parameter, async);

buf.WriteInt32(ValidateObjectAndGetLength(value, ref lengthCache, parameter));
return base.Write(
value is string asString
? asString
: (string)parameter.ConvertedValue,
buf, lengthCache, parameter, async);
throw new NotSupportedException(
"Parameters with NpgsqlDbType.Unknown are no longer supported. " +
"If you're writing enums as strings, see the section on unmapped enums in the documentation.");
}

#endregion Write
Expand Down
2 changes: 0 additions & 2 deletions src/Npgsql/TypeHandling/NpgsqlTypeHandler.cs
Expand Up @@ -163,8 +163,6 @@ internal async ValueTask<T> ReadWithLength<T>(NpgsqlReadBuffer buf, bool async,
internal abstract Type GetFieldType(FieldDescription fieldDescription = null);
internal abstract Type GetProviderSpecificFieldType(FieldDescription fieldDescription = null);

internal virtual bool PreferTextWrite => false;

/// <summary>
/// Creates a type handler for arrays of this handler's type.
/// </summary>
Expand Down
18 changes: 0 additions & 18 deletions test/Npgsql.Tests/CommandTests.cs
Expand Up @@ -838,24 +838,6 @@ public void InputAndOutputParameters()
}
}

[Test]
public void SendUnknown([Values(PrepareOrNot.NotPrepared, PrepareOrNot.Prepared)] PrepareOrNot prepare)
{
using (var conn = OpenConnection())
using (var cmd = new NpgsqlCommand("SELECT @p::TIMESTAMP", conn))
{
cmd.CommandText = "SELECT @p::TIMESTAMP";
cmd.Parameters.Add(new NpgsqlParameter("p", NpgsqlDbType.Unknown) { Value = "2008-1-1" });
if (prepare == PrepareOrNot.Prepared)
cmd.Prepare();
using (var reader = cmd.ExecuteReader())
{
reader.Read();
Assert.That(reader.GetValue(0), Is.EqualTo(new DateTime(2008, 1, 1)));
}
}
}

[Test, IssueLink("https://github.com/npgsql/npgsql/issues/503")]
public void InvalidUTF8()
{
Expand Down
36 changes: 0 additions & 36 deletions test/Npgsql.Tests/Types/EnumTests.cs
Expand Up @@ -386,42 +386,6 @@ public void ReadUnmappedEnumsAsString()
}
}

[Test, Description("Test that a c# string can be written to a backend enum when DbType is unknown")]
public void WriteStringToBackendEnum()
{
using (var conn = OpenConnection())
{
conn.ExecuteNonQuery("CREATE TYPE pg_temp.fruit AS ENUM ('Banana', 'Apple', 'Orange')");
conn.ExecuteNonQuery("create table pg_temp.test_fruit ( id serial, value1 pg_temp.fruit, value2 pg_temp.fruit );");
conn.ReloadTypes();
const string expected = "Banana";
using (var cmd = new NpgsqlCommand("insert into pg_temp.test_fruit(id, value1, value2) values(default, @p1, @p2);", conn))
{
cmd.Parameters.AddWithValue("p2", NpgsqlDbType.Unknown, expected);
var p2 = new NpgsqlParameter("p1", NpgsqlDbType.Unknown) {Value = expected};
cmd.Parameters.Add(p2);
cmd.ExecuteNonQuery();
}
}
}

[Test, Description("Tests that a a C# enum an be written to an enum backend when passed as dbUnknown")]
public void WriteEnumAsDbUnknwown()
{
using (var conn = OpenConnection())
{
conn.ExecuteNonQuery("CREATE TYPE pg_temp.mood8 AS ENUM ('Sad', 'Ok', 'Happy')");
conn.ExecuteNonQuery("CREATE TABLE pg_temp.test_mood_writes (value1 pg_temp.mood8)");
conn.ReloadTypes();
var expected = Mood.Happy;
using (var cmd = new NpgsqlCommand("insert into pg_temp.test_mood_writes(value1) values(@p1);", conn))
{
cmd.Parameters.AddWithValue("p1", NpgsqlDbType.Unknown, expected);
cmd.ExecuteNonQuery();
}
}
}

[Test, IssueLink("https://github.com/npgsql/npgsql/issues/859")]
public void NameTranslationDefaultSnakeCase()
{
Expand Down

0 comments on commit 90ec9cb

Please sign in to comment.