-
-
Notifications
You must be signed in to change notification settings - Fork 789
Added support for nullable cursor keys #8431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@glen-84 Can I submit a documentation PR with a workaround for users of HC versions before your PR or #8230 merges? // Ordering by UpdatedUtc or CreatedUtc will fail with
// Message=The key type `System.Nullable`1[[System.DateTime, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]` is not supported.
[InterfaceType("Auditable")]
public class AuditableGqlModel
{
// Audit properties
public DateTime? UpdatedUtc { get; set; }
public string? CreatedBy { get; set; }
public DateTime? CreatedUtc { get; set; }
public string? UpdatedBy { get; set; }
}
// Workaround: ordering works as expected
[InterfaceType("Auditable")]
public class AuditableGqlModel
{
// Audit properties
[GraphQLNonNullType(nullable: true)]
public DateTime UpdatedUtc { get; set; }
public string? CreatedBy { get; set; }
[GraphQLNonNullType(nullable: true)]
public DateTime CreatedUtc { get; set; }
public string? UpdatedBy { get; set; }
} |
Thanks for the offer, but I don't think that there is a workaround for this issue. I'm assuming that your code is bypassing the error, but the paging will likely still be incorrect if the data includes null values, since the implementation doesn't currently (or at least, correctly) handle null cursor keys. |
Thanks for the info @glen-84. It's great to know that the workaround is not great before we pushed it to prod. We are a bit of a strange case where the fields we want to filter by are not actually allowed to be null within the context of a single microservices DB. But we use fusion to aggregate data across several microservices, with lookups and all that. With eventual consistency and other issues unique to distributed system, lookups could return null for an id and not be able to fill in a property that otherwise would have been specified as not-null. We end up marking almost every field as nullable because of this. But maybe there's a better way. |
Summary of the changes (Less than 80 chars)
To-do:
HotChocolate/Data/test/Data.Tests/Pagination/PagingArgumentsParameterExpressionBuilderTests.cs#X
.HotChocolate/Core/test/Types.Analyzers.Integration.Tests/IntegrationTests.cs#X
.src/HotChocolate/Data/test/Data.Tests/PagingHelperIntegrationTests.cs
.EfQueryableCursorPagingHandler
is breaking – will throw for nullable cursor keys.