Skip to content

Add support for parsing DateTime values#3250

Open
ArjunNarendra wants to merge 6 commits intoAzure:mainfrom
ArjunNarendra:user/an/postgres-db-type
Open

Add support for parsing DateTime values#3250
ArjunNarendra wants to merge 6 commits intoAzure:mainfrom
ArjunNarendra:user/an/postgres-db-type

Conversation

@ArjunNarendra
Copy link
Copy Markdown

@ArjunNarendra ArjunNarendra commented Mar 13, 2026

Why make this change?

This PR is to address #3094, which is a bug that happens because the default type conversion for C# to Npgsql is to convert a string to a TEXT field, but this doesn't work when trying to compare a DATE type for PostgreSql databases.

What is this change?

I have updated ExecutionHelper.cs in ExtractValueFromIValueNode to explicitly handle SupportedHotChocolateTypes.DATETIME_TYPE and SupportedHotChocolateTypes.DATETIMEOFFSET_TYPE.

How was this tested?

  • Unit Tests
  • Integration Tests

Sample Request(s)

With query:

query {
  assignments(filter: { assignment_due_date: { gte: "2026-03-23T00:00:00.000Z" } }) {
    items {
      assignment_id
      assignment_name
      assignment_due_date
    }
  }
}

Before:

SELECT * FROM public."Assignments"
WHERE "Assignments"."assignment_due_date" >= '2026-03-23T00:00:00.000Z'::text;

After:

SELECT * FROM public."Assignments"
WHERE "Assignments"."assignment_due_date" >= '2026-03-23T00:00:00.000Z';

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses #3094 by ensuring GraphQL DateTime (and DateTimeOffset) argument values are parsed into typed CLR values before being passed to database providers, avoiding PostgreSQL treating them as text parameters in comparisons.

Changes:

  • Added explicit handling for SupportedHotChocolateTypes.DATETIME_TYPE and SupportedHotChocolateTypes.DATETIMEOFFSET_TYPE in ExecutionHelper.ExtractValueFromIValueNode.
  • Introduced local parsing helpers to convert string inputs into DateTime/DateTimeOffset-typed values using invariant culture + roundtrip parsing.

Comment on lines +398 to 401
SupportedHotChocolateTypes.DATETIME_TYPE => ParseDateTimeValue(value.Value),
SupportedHotChocolateTypes.DATETIMEOFFSET_TYPE => ParseDateTimeOffsetValue(value.Value),
SupportedHotChocolateTypes.UUID_TYPE => Guid.TryParse(value.Value!.ToString(), out Guid guidValue) ? guidValue : value.Value,
_ => value.Value
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This does not make too much sense to me...if we are dealing with SupportedHotChocolateTypes.DATETIME_TYPE shouldn't we return a DateTime (not a DateTimeOffset)?

Comment thread src/Core/Services/ExecutionHelper.cs Outdated
Comment on lines +425 to +432
if (DateTimeOffset.TryParse(s, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out DateTimeOffset parsedDto))
{
return parsedDto.UtcDateTime;
}

if (DateTime.TryParse(s, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out DateTime parsedDt))
{
return parsedDt;
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ok, this makes sense.

Comment thread src/Core/Services/ExecutionHelper.cs Outdated

if (raw is DateTime dt)
{
return new DateTimeOffset(dt);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ok, this makes sense although I am having some difficulty thinking of a concrete example when this would come into play.

Comment thread src/Core/Services/ExecutionHelper.cs Outdated
SupportedHotChocolateTypes.FLOAT_TYPE => value is IntValueNode intValueNode ? intValueNode.ToDouble() : ((FloatValueNode)value).ToDouble(),
SupportedHotChocolateTypes.DECIMAL_TYPE => value is IntValueNode intValueNode ? intValueNode.ToDecimal() : ((FloatValueNode)value).ToDecimal(),
SupportedHotChocolateTypes.DATETIME_TYPE => ParseDateTimeValue(value.Value),
SupportedHotChocolateTypes.DATETIMEOFFSET_TYPE => ParseDateTimeOffsetValue(value.Value),
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ok, I can add a regression test.

@ArjunNarendra ArjunNarendra force-pushed the user/an/postgres-db-type branch from a9a9b1a to ecd6403 Compare April 22, 2026 20:50
@unattendedfaxmachine
Copy link
Copy Markdown

@ArjunNarendra I haven't managed to run the integration test slices against local backends successfully yet. Without that information, from my review we need to make sure these changes don't cause regressions in backends other than PGSQL. My main concern is that the normalization is currently implemented in shared GraphQL datetime extraction rather than a PostgreSQL-specific path, so it changes behavior for every backend that consumes the datetime scalar, including DateTimeOffset cases. I also do not see regression coverage proving SQL Server, MySQL, and Cosmos query generation still behaves correctly after this change, especially for offset-bearing filters. We need to make sure it doesn't break any other functionality. If you've already tested for regression, please disregard this comment. Otherwise, I've forked this repo and branch and started playing with local regression testing with docker here

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.

5 participants