-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Description
Background and Motivation
In the desktop framework the System.Data.SqlClient namespace was originally part of the System.Data assembly allowing SqlClient to access internal methods of some types in the System.Data.SqlTypes namespace. In netcoreSqlClient was split out into a separate assembly but rather than add new public apis to the SqlTypes struct overlay hacks were used to maintain the existing access to internals.
SqlClient has now been split out of the framework entirely into a separate project Microsoft.Data.SqlClient and that project still uses the overlay hack to access internals, all collected into the [SqlTypeWorkarounds](SqlClient/SqlTypeWorkarounds.cs at main · dotnet/SqlClient (github.com)) class. It is unlikely that the layout or definition of the SqlTypes members will change but it is undesirable to have a first party library use such access when it is possible and relatively simple to extend the public api.
Proposed API
namespace System.Data.SqlTypes
{
public struct SqlDateTime
{
+ public static DateTime FromInternalRepresentation(int daypart, int timepart)
}
public struct SqlMoney
{
+ public static SqlMoney FromInternalRepresentation(long value);
+ public long GetInternalRepresentation();
}
public struct SqlDecimal
{
+ public GetInternalDataRepresentation(Span<uint> value)
}
public struct SqlBinary
{
+ public static SqlBinary WrapBytes(byte[] bytes)
}
public struct SqlGuid
{
+ public static SqlGuid WrapBytes(byte[] bytes)
}
}
Usage Examples
Creating an SqlMoney from it's internal TDS representation will change from:
...
case TdsEnums.SQLUNIQUEID:
value.SqlGuid = SqlTypeWorkarounds.SqlGuidCtor(unencryptedBytes, true); // doesn't copy the byte array
break;
...
public static class SqlTypeWorkarounds
{
internal static SqlGuid SqlGuidCtor(byte[] value, bool ignored)
{
var c = default(SqlGuidCaster);
c.Fake._value = value;
return c.Real;
}
[StructLayout(LayoutKind.Sequential)]
private struct SqlGuidLookalike
{
internal byte[] _value;
}
[StructLayout(LayoutKind.Explicit)]
private struct SqlGuidCaster
{
[FieldOffset(0)]
internal SqlGuid Real;
[FieldOffset(0)]
internal SqlGuidLookalike Fake;
}
}to:
...
case TdsEnums.SQLUNIQUEID:
value.SqlGuid = SqlGuid.WrapBytes(unencryptedBytes);
break;
...Alternative Designs/Reasoning
SqlDateTime.FromInternalRepresentation
- SqlDateTime has an internal static method
internal static DateTime ToDateTime(int daypart, int timepart)which could have the access modifier changed to public. I chose not to suggest this because it doesn't convert an SqlDateTime instance to a DateTime, it converts parameters which are the internal representation used in TDS into a DateTime, the input and output are not what a user would expect without reading the documentation. - It could be argued that the return value should be an SqlDateTime however this is not how it is used and if an SqlDateTime were used it would usually need to be unwrapped because a user requested a known-not-null DateTime through
GetFieldValue<DateTime>orGetDateTime
SqlMoney.ToSqlInternalRepresentation
SqlMoney already contains an internal implementation of this method so i propose only to change the existing access modifier. The internal representation is an unscaled value read from a TDS packet.
SqlMoney.FromInternalRepresentation
SqlMoney contains an internal ctor internal SqlMoney(long value, int ignored) which was used to perform an unscaled construction of SqlMoney from the internal representation. The ignored parameter is present because there is already a public ctor internal SqlMoney(long value) which performs a scaled creation. Since I've already proposed a method to get the unscaled value and contains the words InternalRepresentation it made sense to me to have a symmetric static method to do the creation from that representation.
SqlDecimal.GetInternalDataRepresentation
This method name was chosen for close to be similarity to the SqlMoney and SqlDateTime methods which provide access to internal data representations. This method does not give access to all of the internal data for the SqlDecimal, it only provides access to 4 Data uints which contain the value, it does not provide access to the additional 4 bytes of data (Status, Length, Precision and Scale) and because of that I have added the word Data in the middle of the name. Access to the other byte fields is done through existing public accessor properties.
This method could have been called GetBits for some symmetry with decimal.GetBits, i decided against this because it isn't an exactly analogous operation.
SqlBinary.WrapBytes and SqlGuid.WrapBytes
These could be called TakeBytes but that felt clumsy or too low level a name. Their intention is to return an instance of their containing type which uses the reference to the array parameter passed in, currently byte[] ctor for each type would allocate a new appropriately sized array and copy the contents of the parameter into it.
General
The existing behaviour of accessing internal members will be present in pre NET6 framework and SqlClient binaries for as long as both exist. It would cause compatibility problems discoverable only at runtime if we tried to change that. Adding these members will allow the same access to be done using a legitimate channel on NET6 and future SqlClient versions that compile against it.
Adding these new members simply makes the access to data used by SqlClient official and thus documented. They should have no performance impact.
Risks
The new members will need to be documented.
/cc area owners @ajcvickers, @roji and @David-Engel