Skip to content
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

Added AsOf and Between extension methods on a DbSet. #10

Merged
merged 3 commits into from
Feb 14, 2020

Conversation

lpperras
Copy link

Added some simple extension methods on a DbSet to easily query a entities at a specific time or between a range. The returned IQueryable can still be used by suppoted EF LINQ operators and the filter will apply fine.

@MoazAlkharfan
Copy link

Your extensions will only return data from the TemporalTable instead of the history table.

@lpperras
Copy link
Author

lpperras commented Feb 1, 2020

Did you test the extension? Data from both tables is returned with this syntax. Maybe I don't understand what you mean, but I'd you test both extensions, days from the main table and history table is returned if it matches the date.

@MoazAlkharfan
Copy link

@lpperras Sorry i've mistaken, take a look at my test results
dotnet/efcore#4693 (comment)

@findulov findulov merged commit 97d85bb into findulov:master Feb 14, 2020
@findulov
Copy link
Owner

Hi, I've checked your pull request and it appears to be working so I merged it.
It took me while to get it merged because I've been trying to find a way to prevent SELECT * FROM ... if in your query you have something like:
db.MyTable.Select(x => new { Id = x.Id, Name = x.Name }).ToList();
and have the corresponding SQL code containing only the columns that are in the projection.

I still can't find a solution for that problem but meanwhile your solution will work for maybe most of the cases so thank you for cooperating on the project!

@lpperras lpperras deleted the filters branch February 15, 2020 01:40
@lpperras
Copy link
Author

I would have to try, but I think that since the FromRawSql method returns an IQueryable, doing this:
db.MyTable.AsOf(DateTime.Now).Select(x => new { Id = x.Id, Name = x.Name }).ToList()
would be queried like this:
SELECT t.[Id], t.[Name] FROM (SELECT * FROM MyTable) AS t

Only selected columns would be returned from the database.

@lpperras
Copy link
Author

Just noticed that you changed the type from DateTimeOffset to DateTime. Since Temporal tables are datetime2 columns, I had timezone issues when using DateTime. It would not work properly if my timezone wasn't GMT. Maybe my tests were not valid, did you try with different timezones?

@findulov
Copy link
Owner

I would have to try, but I think that since the FromRawSql method returns an IQueryable, doing this:
db.MyTable.AsOf(DateTime.Now).Select(x => new { Id = x.Id, Name = x.Name }).ToList()
would be queried like this:
SELECT t.[Id], t.[Name] FROM (SELECT * FROM MyTable) AS t

Only selected columns would be returned from the database.

Yes, I've tested that it and it only fetches the specified columns when I apply a where clause, otherwise it returns all columns.

@findulov
Copy link
Owner

findulov commented Feb 15, 2020

Just noticed that you changed the type from DateTimeOffset to DateTime. Since Temporal tables are datetime2 columns, I had timezone issues when using DateTime. It would not work properly if my timezone wasn't GMT. Maybe my tests were not valid, did you try with different timezones?

I guess we can overload the methods to support both DateTime and DateTimeOffset, I wasn't sure why you need DateTimeOffset initially. When I work with time zones I store the dates in UTC and have the corresponding client applications convert the UTC dates to their local date & time and I'm able to achieve complete independence of the time zone my database is being hosted in.

I'll modify the library to support DateTimeOffset as well to keep it more functional and to resolve such time zone issue but I wanted to also share my experience with time zones and why I tend to use UTC dates in such scenarios.

@ferarnoled
Copy link

Just noticed that you changed the type from DateTimeOffset to DateTime. Since Temporal tables are datetime2 columns, I had timezone issues when using DateTime. It would not work properly if my timezone wasn't GMT. Maybe my tests were not valid, did you try with different timezones?

I guess we can overload the methods to support both DateTime and DateTimeOffset, I wasn't sure why you need DateTimeOffset initially. When I work with time zones I store the dates in UTC and have the corresponding client applications convert the UTC dates to their local date & time and I'm able to achieve complete independence of the time zone my database is being hosted in.

Agree with your approach

I'll modify the library to support DateTimeOffset as well to keep it more functional and to resolve such time zone issue but I wanted to also share my experience with time zones and why I tend to use UTC dates in such scenarios.

I don't think you should support DateTimeOffset because the underlying data type is always datetime2 for the temporal table. Correct me if I'm wrong

@lpperras
Copy link
Author

You are right that the underlying type is always datetime2, which is the type that is generated for any property of type DateTimeOffset. By default, I thinbk EF still generates a datetime for properties of type DateTime. I just tested again, and if you DateTime is of kind DateTimeKind.Local, you will not get the expected result unless you are in GMT timezone. I think that if DateTimeOffset is not added, the code should at least validate the type of DateTime that is passed, and if it's local, convert it to universal.

Ex: date = date.Kind == DateTimeKind.Local ? date.ToUniversalTime() : date;

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.

None yet

4 participants