Cleanup | Merge SqlDataRecord and remove context connection remnants #3454
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| SqlMetaData md = GetSqlMetaData(ordinal); | ||
|
|
||
| #if NETFRAMEWORK | ||
| if (md.SqlDbType == SqlDbType.Udt) |
There was a problem hiding this comment.
Since this check only applies to netframework and will return, let's avoid the braces for netcore
#if NETFRAMEWORK
if (md.SqlDbType == SqlDbType.Udt)
{
return md.Type;
}
#endif
return MetaType.GetMetaTypeFromSqlDbType(md.SqlDbType, false).ClassType;There was a problem hiding this comment.
Also tiny request, can we get an argument label for the false here? :)
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3454 +/- ##
==========================================
- Coverage 64.55% 58.82% -5.73%
==========================================
Files 281 271 -10
Lines 62519 61984 -535
==========================================
- Hits 40358 36464 -3894
- Misses 22161 25520 +3359
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| // Allow values array longer than FieldCount, just ignore the extra cells. | ||
| int copyLength = (values.Length > FieldCount) ? FieldCount : values.Length; |
There was a problem hiding this comment.
We can do some early exit optimization here if (copyLength == 0) return 0;
| } | ||
|
|
||
| // Now move the data (it'll only throw if someone plays with the values array between | ||
| // the validation loop and here, or if an invalid UDT was sent). |
There was a problem hiding this comment.
We can make a shallow copy of the portion of array to be used
object[] stableValues = new object[copyLength];
Array.Copy(values, stableValues, copyLength);
then use stableValues[i] in both loops, guaranteeing stability of inputs between validation and writing phase.
There was a problem hiding this comment.
Nothing actually tampers with the values array so I don't think we need to make a defensive copy. I'll adjust the comment accordingly.
Exit early if we have no fields to set. Clarify comment.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Description
This picks up where #3438 left off. With
SmiEventSink_Defaultremoved, almost all of the code in SqlDataRecord.netfx.cs and SqlDataRecord.netcore.cs is identical and can be removed. We do so.The
SmiRecordBufferclass is also removed, since this isn't doing anything meaningful. I'm not pleased with the overall type hierarchy here, but I want to think through the consequences of changing them in the wider context of ValueUtilsSmi.With that work done, we just remove the now-unused resource strings and exception methods from SqlUtil, AdapterUtil and the string resources. There are other unused strings and exception methods here, for anyone interested in picking up a bulky commit.
Closes #2838.
Issues
Follows #3438.
Resolves #2838.
Contributes to #1261.
Testing
Local unit tests for SqlDataRecord pass.