Skip to content

Add support for datetimeoffset MsSql data type#1400

Merged
Aniruddh25 merged 9 commits intomainfrom
fixDateColumnOffset
Apr 3, 2023
Merged

Add support for datetimeoffset MsSql data type#1400
Aniruddh25 merged 9 commits intomainfrom
fixDateColumnOffset

Conversation

@Aniruddh25
Copy link
Collaborator

@Aniruddh25 Aniruddh25 commented Mar 31, 2023

Why make this change?

What is this change?

  • Add an additional case for DateTimeOffset while converting from SystemType to EdmPrimitiveTypeKind.
  • DateTimeOffset is simply DateTime in HotChocolate.Types used in SchemaConverter.
  • Add a new DateTimeOffset case to Param creation.

How was this tested?

  • Added new column with datetimeoffset in MsSql to test this. Also added columns for datetime2, date, smalldatetime to ensure those are already supported
  • Integration Tests
  • Unit Tests

Copy link
Contributor

@ayush3797 ayush3797 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@abhishekkumams abhishekkumams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far, is there any way to add 1 test for e2e REST request with this new datatype?

@Aniruddh25
Copy link
Collaborator Author

Looks good so far, is there any way to add 1 test for e2e REST request with this new datatype?

There is an existing issue to add new tests for REST - #662 for additional supported types

@Aniruddh25 Aniruddh25 merged commit dcffa82 into main Apr 3, 2023
@Aniruddh25 Aniruddh25 deleted the fixDateColumnOffset branch April 3, 2023 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DAB - Column type DateTimeOffset not yet supported

4 participants