Replace 'as' casts with explicit casts

[SimonCropp]

Replace usages of 'as' with explicit casts across multiple files to ensure correct typing and avoid silent nulls. Updates include DataRow casts in SqlBulkCopy, string casts for Json parsing in SqlDataReader and SqlParameter, object ref cast in SqlDependency, and test parameter casts in AADAuthenticationTests and PoolBlockPeriodTest. This makes failures deterministic (throwing InvalidCastException) and clarifies intent when a null result from 'as' would be unsafe.

[Copilot]

Pull request overview

This PR replaces several as casts with explicit casts across SqlClient source and test code to avoid silent null results and make type assumptions fail deterministically.

Changes:

• Updated test parameter handling to use explicit casts for connection pool and AAD authentication tests.
• Updated SqlBulkCopy DataRow indexing logic to use explicit DataRow casts.
• Updated JSON/vector-related parsing and remoting deserialization call sites to use explicit casts.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/PoolBlockPeriodTest.cs	Uses explicit string casts for xUnit parameter arrays.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/AADAuthenticationTests.cs	Uses explicit string casts when constructing credentials and setting builder keys.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs	Uses explicit string/interface casts for vector JSON coercion.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs	Uses explicit cast for deserialized SqlClientObjRef before unmarshalling.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs	Uses explicit string cast when parsing JsonDocument from buffer value.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs	Uses explicit DataRow cast for determining row number from enumerator current item.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[priyankatiwari08]

I believe with the casts change, there is a need to add InvalidCastException to the catch filter since now with the explicit (string)value cast, a non-string value now throws InvalidCastException, which isn't covered by when (ex is ArgumentNullException || ex is JsonException) and will propagate unhandled.

[paulmedynski]

I think we're good. See the analysis I had Copilot perform.

[paulmedynski]

I suggest at least one other maintainer perform a similar analysis to mine below to see if they agree. Leave this conversation open to avoid inadvertent merging until then.

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[paulmedynski]

Analysis of Cast Changes in Four Files

1. SqlBulkCopy.cs — _rowEnumerator.Current as DataRow → (DataRow)_rowEnumerator.Current

Location: SqlBulkCopy.cs, RowNumber property getter, lines 207 and 210.

Two instances, in the RowArray and DataTable switch branches:

// BEFORE
rowNo = ((DataTable)_dataTableSource).Rows.IndexOf(_rowEnumerator.Current as DataRow);
// AFTER
rowNo = ((DataTable)_dataTableSource).Rows.IndexOf((DataRow)_rowEnumerator.Current);

Reachable from public API: Yes — SqlBulkCopy.WriteToServer(DataRow[]) and WriteToServer(DataTable).

Risk assessment: LOW. The RowArray path is entered only when _rowSourceType == ValueSourceType.RowArray, which is set exclusively in WriteToServer(DataRow[]). There, _rowEnumerator = rows.GetEnumerator() where rows is a DataRow[] — so Current is always a DataRow. Similarly, the DataTable path uses table.Rows.GetEnumerator(), which yields DataRow objects from a DataRowCollection. The cast cannot fail in any reachable code path.

Previous behavior on hypothetical failure: as DataRow would produce null, passed to DataRowCollection.IndexOf(null), which returns -1. The property would return 0 (++rowNo). With the explicit cast, an InvalidCastException would be thrown instead. However, since both paths are type-guaranteed by construction, this is not a practical concern.

Verdict: Safe change. No observable behavioral difference in any reachable code path.

---

2. SqlDataReader.cs — data.Value as string → data.String

Location: SqlDataReader.cs, inside GetFieldValueFromSqlBufferInternal<T>, line 3106.

// BEFORE
JsonDocument document = JsonDocument.Parse(data.Value as string);
// AFTER
JsonDocument document = JsonDocument.Parse(data.String);

Reachable from public API: Yes — SqlDataReader.GetFieldValue<JsonDocument>(i).

Behavioral difference: This is not a simple as → explicit cast. The entire access path changed:

Aspect	data.Value as string (before)	data.String (after)
Null data	Value returns DBNull.Value; as string yields null; JsonDocument.Parse(null) throws ArgumentNullException	String calls ThrowIfNull() → throws SqlNullValueException
Json storage type	Value returns String property → string; as string → string	String returns (string)_object directly
Non-string type	Value returns typed value; as string yields null; Parse(null) → ArgumentNullException	String calls (string)Value → InvalidCastException

Risk assessment: MEDIUM. The typeof(T) == typeof(JsonDocument) branch is only entered when the user explicitly calls GetFieldValue<JsonDocument>(), and the code verifies metaType.SqlDbType == SqlDbTypeExtensions.Json before reaching this line (throwing if not). For a legitimate Json column, data will have StorageType.Json, and both paths return the same string. The practical difference is:

• Null column value: Previously threw ArgumentNullException (from JsonDocument.Parse(null)). Now throws SqlNullValueException (from SqlBuffer.ThrowIfNull()). This is arguably more correct — SqlNullValueException is the standard exception for accessing null SQL values and is what other SqlDataReader.GetFieldValue<T>() paths throw.

• Non-Json column: Cannot reach this line — the SqlDbType check above throws first.

Verdict: Behavioral change for null JSON columns only — exception type changes from ArgumentNullException to SqlNullValueException. This is a more correct exception for the scenario but is a change to the exception contract that callers might be catching.

---

3. SqlDependency.cs — refResult as SqlClientObjRef → (SqlClientObjRef)refResult

Location: SqlDependency.cs, inside GetDeserializedObject, line 541. NET Framework only (#if NETFRAMEWORK).

// BEFORE
var result = RemotingServices.Unmarshal((refResult as SqlClientObjRef).GetObjRef());
// AFTER
var result = RemotingServices.Unmarshal(((SqlClientObjRef)refResult).GetObjRef());

Reachable from public API: Indirectly — via SqlDependency.Start() → ObtainProcessDispatcher() → GetDeserializedObject().

Risk assessment: LOW. The caller at line 512-516 already checks SqlClientObjRef.CanCastToSqlDependencyProcessDispatcher() before calling this method. Additionally, DataContractSerializer was constructed with typeof(SqlClientObjRef), so serializer.ReadObject(stream) will either return a SqlClientObjRef or throw a SerializationException. The cast cannot practically fail.

Previous behavior on failure: as SqlClientObjRef → null → NullReferenceException on .GetObjRef(). Now throws InvalidCastException directly. The failure was always a crash; it's just a different exception type.

Verdict: Safe change. The cast is type-guaranteed by the preceding guard check and the DataContractSerializer contract. The only difference is which exception type would be thrown in an impossible failure case.

---

4. SqlParameter.cs — Three cast changes in Vector handling

4a. value as string → (string)value in Vector deserialization (line 2393)

Location: SqlParameter.cs, line 2393.

// BEFORE
value = (new SqlVector<float>(JsonSerializer.Deserialize<float[]>(value as string)) as ISqlVector).VectorPayload;
// AFTER
value = ((ISqlVector)new SqlVector<float>(JsonSerializer.Deserialize<float[]>((string)value))).VectorPayload;

Reachable from public API: Yes — SqlCommand.ExecuteNonQuery() etc., when a parameter has SqlDbType.Vector and a string value.

Risk assessment: LOW. This code is inside the branch currentType == typeof(string) && destinationType.SqlDbType == SqlDbTypeExtensions.Vector — meaning value has already been confirmed to be a string by the typeof(string) check on currentType (which is value.GetType()). The (string)value cast cannot fail because the type was just verified.

The second part of the change — as ISqlVector → (ISqlVector) — is also safe because SqlVector<float> implements ISqlVector.

Verdict: Safe. The type guard immediately above guarantees the cast succeeds.

4b. value as string → (string)value in error handler (line 2397)

Location: SqlParameter.cs, line 2397.

// BEFORE (in catch handler)
throw ADP.InvalidJsonStringForVector(value as string, ex);
// AFTER
throw ADP.InvalidJsonStringForVector((string)value, ex);

Risk assessment: LOW. Same reasoning — this catch block is inside the same currentType == typeof(string) branch, so value is guaranteed to be a string. The cast cannot fail.

Verdict: Safe. Same type guarantee applies.

---

Summary

File	Location	Change	Risk	Observable Behavior Difference
SqlBulkCopy.cs	Lines 207, 210	as DataRow → (DataRow)	LOW	None in reachable code. DataRow[]/DataTable enumerators always yield DataRow.
SqlDataReader.cs	Line 3106	data.Value as string → data.String	MEDIUM	For null JSON columns: exception changes from ArgumentNullException to SqlNullValueException. For non-null JSON: identical.
SqlDependency.cs	Line 541	as SqlClientObjRef → (SqlClientObjRef)	LOW	None in reachable code. Type guaranteed by preceding guard and serializer contract. .NET Framework only.
SqlParameter.cs	Lines 2393, 2397	as string → (string), as ISqlVector → (ISqlVector)	LOW	None. Type guard (currentType == typeof(string)) ensures value is already a string.

The only practically observable public API contract change is in SqlDataReader: GetFieldValue<JsonDocument>(i) on a null JSON column will now throw SqlNullValueException instead of ArgumentNullException. This is arguably more correct (consistent with how other GetFieldValue<T> paths handle null) but is a change to the exception type that user code may be catching.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.53%. Comparing base (569ada7) to head (2c99232).
⚠️ Report is 36 commits behind head on main.

Files with missing lines	Patch %	Lines
...lient/src/Microsoft/Data/SqlClient/SqlParameter.cs	50.00%	1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (569ada7) and HEAD (2c99232). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (569ada7)	HEAD (2c99232)
CI-SqlClient	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4019      +/-   ##
==========================================
- Coverage   72.36%   65.53%   -6.83%     
==========================================
  Files         287      275      -12     
  Lines       43149    65825   +22676     
==========================================
+ Hits        31223    43137   +11914     
- Misses      11926    22688   +10762     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	65.53% <50.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.