supabase-community / postgrest-csharp

A C# Client library for Postgrest
https://supabase-community.github.io/postgrest-csharp/api/Postgrest.html
MIT License
114 stars 22 forks source link

Using LINQ expression to handle DateTime comparisons produces inconsistent results. #76

Closed acupofjose closed 7 months ago

acupofjose commented 7 months ago

Referenced Lines: https://github.com/supabase-community/postgrest-csharp/blob/a0b36bdf5d6413ba7bbb0ed1aa6211668dce6b9c/Postgrest/Linq/WhereExpressionVisitor.cs#L184-L215

Issue Report: Re: https://github.com/supabase-community/supabase-csharp/discussions/121

Reproducing this issue [from @aquatyk here]

DateTime day1 = DateTime.Now.AddDays(-1);
string day = day1.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture);

// Produces incorrect result.
var result = await _client.From<Item>().Where(x => x.Created_at >= day1).Get();

// Produces correct result.
var result = await _client.From<Item>().Filter("created_at", Operator.GreaterThanOrEqual, day).Get();
acupofjose commented 7 months ago

@wiverson do you see any obvious reason why it would be problematic to remove .ToUniversalTime() on L205 there?

acupofjose commented 7 months ago

This is a deviation from how the original query worked, so it's been removed. It is incorrect to force a ToUniversalTime conversion in the clause here.

Now available in 3.2.10

wiverson commented 7 months ago

Sorry I'm late, FWIW I think most ORMs would prefer deferring to the underlying datetime w/tz instead of the conversion. I wish all dbs and timestamps were UTC but alas.

FWIW I'd add some test cases to cover more time/date stuff for the different db date/time configurations or at least add as a issue for later?

acupofjose commented 7 months ago

Agreed! I'll add it as a chore. Thanks @wiverson!