[API Proposal]: DataTable.LoadDataRow with array length greater than the number of columns

[0UserName]

Background and motivation

LoadDataRow into DataTable, or more precisely, into NewRecordFromArray the check is performed to ensure that the length of the array containing the row values ​​does not exceed the number of columns:

if (colCount < value.Length)
{
    throw ExceptionBuilder.ValueArrayLength();
}

Because of this, we cannot use built-in solutions like ArrayPool.Shared to pass row values, since the pool does not guarantee the exact length of the returned array.

In this case, we are forced to either perform explicit allocations without using a pool, or implement a custom pool that returns arrays with exact lengths. However, in scenarios where the number of columns and consequently, the required array length varies dynamically, the number of such pools becomes unbounded.

API Proposal

public DataRow LoadDataRow(ReadOnlySpan<object> values, bool fAcceptChanges)

This will allow to use arrays of any origin. The methods in the chain must also accept ReadOnlySpan<T>.

API Usage

DataTable dt = new
DataTable
();

dt.Columns.Add(nameof(TVP.Property1), typeof(int));
dt.Columns.Add(nameof(TVP.Property2), typeof(int));
dt.Columns.Add(nameof(TVP.Property3), typeof(int));

ReadOnlySpan<object> values = ArrayPool<object>.Shared.Rent(10).AsSpan().Slice(0, 3);

dt.LoadDataRow(values, false);

Alternative Designs

Or simply skip validating the length of the input array (as described above) and instead base logic on the number of columns.

Risks

No response

[Wraith2]

Since you're using Table Valued Parameters you must be using SqlClient. You can pass table valued parameters without using a data table at all and with far fewer allocations required if you use some custom logic to produce your own IEnumerable<SqlDataRecord> which is what DataTable is providing.

I have an example of how to do this in my SqlBench project that use for reproduction and perf testing of SqlClient bugs, here. The pattern is to create an adapter for T->SqlDataRecord and then setup an IEnumerable to iterate in input source and use the adapter on each item to convert it to the form that SqlServer expects. This may only require a single row-length array allocation because it can be reused. Setup is a fairly simple pattern using an extension like this:

var command = new SqlCommand("InsertJobResult", syncConnection)
{
    CommandType = CommandType.StoredProcedure
};
var parameter = new SqlParameter("@result", JobResultAdapter.Generator(500, 1, 100, DateTime.Now))
    .SetupTableValuedParameter(new JobResultAdapter());
command.Parameters.Add(parameter);

[0UserName]

Since you're using Table Valued Parameters you must be using SqlClient.

We can emulate TVP, in which case depending on Microsoft.Data.SqlClient becomes redundant. DataTable and DbDataReader are built-in types that don’t require additional dependencies and allows implement driver-agnostic code.

--

I’d like to slightly improve the current API to minimize allocations, at least in client code. At the same time, I’m aware that complex types like DataTable can still incur additional allocations internally (just as their handling does inside SqlClient).

[Wraith2]

Very interesting.

In that case I'd suggest generalizing your AbstractConverter<DbDataReader> to AbstractConverter<IDataReader> and then producing your own PgDataRecord class to use instead of SqlDataRecord, `SqlDataRecord is just a convenient implementation of IDataRecord. Then you could mostly use my approach and avoid DataTable entirely where performance is important.

The example I linked was created to minimize the in memory footprint of using TVP and does so by allowing re-use of adapters, buffers and permitting streaming of data from a data reader directly into the tvp writer without needing to buffer it all first.

To feedback on your original request. I see two problems

1. The ownership of the arrays used by DataTable is entirely managed by the DataTable and usually DataAdapter. There are no callbacks or lifecycle events which you can easily use to return arrays. You also cannot be certain that the arrays are not being retained by internal classes.
2. The DataTable and DataAdapter classes are .net 1.0 era classes and their model of operation is no longer recommended. They work, they're maintained, but not actively being modernised. You're unlikely to succeed in getting changes to them approved.

[0UserName]

Not sure that I understand suggestion.

In that case I'd suggest generalizing your AbstractConverter to AbstractConverter and then producing your own PgDataRecord class to use instead of SqlDataRecord

I think I’ve tried something similar before, but it wasn’t very portable. SqlClient identifies a passed parameter as a TVP only if it meets one of the following conditions:

else if (
    TdsEnums.SQLTABLE == destinationType.TDSType &&
    (
        value is DataTable ||
        value is DbDataReader ||
        value is IEnumerable<SqlDataRecord>
    )
)

If create a custom type that implements IDataRecord, the flow will fall in this branch:

else
{
    value = Convert.ChangeType(value, destinationType.ClassType, null);
}

and an exception will be thrown. In other words, seems like SqlClient expects SqlDataRecord. Therefore, it’s not clear what advantage the proposed approach actually provides. Either way, I’ll need to use the concrete implementation (and the package).

[Wraith2]

My suggestion was about your postgres emulation verion, not SqlClient. Looking at the code snippet you provided for the type check you're right that using IDataReader won't work so stick with DBDataReader. These are discussions we should probably be having over on the tvp emulation project not here though.