Skip to content

Feature | Write TimeOnly instances as time values - #4604

Open
edwardneal wants to merge 2 commits into
dotnet:mainfrom
edwardneal:feat/timeonly-variant
Open

Feature | Write TimeOnly instances as time values#4604
edwardneal wants to merge 2 commits into
dotnet:mainfrom
edwardneal:feat/timeonly-variant

Conversation

@edwardneal

Copy link
Copy Markdown
Contributor

Description

This PR follows up the earlier #4439. At the conclusion of that PR, DateOnly instances were always written as a date type. This one now raises TimeOnly to parity with DateOnly in three cases:

  1. SqlBulkCopy, based upon a TimeOnly instance within a DataRow or a DataTable.
  2. A simple SqlParameter of SqlDbType Variant containing a TimeOnly instance.
  3. When run in a Debug configuration, a user-defined table type parameter, specified via a SqlParameter with its value being an array of SqlDataRecord objects, all with a column of SqlDbType Variant and a row containing a TimeOnly instance.

I also noticed while in the area that although DateTimeVariantTests was performing a number of validity checks against invalid time values, checks against a valid time value were missing. I've added these.

Notes

  • This only opens the DataRow and DataTable paths in SqlBulkCopy; the SqlDataReader path was already open. I opened this path to the other two situations by making an explicit type check in TdsParser.WriteSqlVariantDataRowValue.
  • The simpler case (a Variant-typed SqlParameter) is opened by the change to TdsParser.WriteSqlVariantValue.
  • When run in Debug mode, an assertion was tripping based upon the contents of s_canAccessSetterDirectly. I added a mapping confirming that a variant type can contain a TimeOnly instance.

Issues

None. Repeats the style of fix in #4439 for TimeOnly instances.

Testing

New test cases have been added which verify functionality, existing tests continue to pass.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

1 participant