Skip to content

Query of the history of a temporary table returns additional records #27035

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
mendozagit opened this issue Dec 18, 2021 · 10 comments
Closed

Query of the history of a temporary table returns additional records #27035

mendozagit opened this issue Dec 18, 2021 · 10 comments
Assignees

Comments

@mendozagit
Copy link

File a bug

Imagine that you have a simple invoices and InvoicesHistory entities, also imagine that you have already inserted some records (only inserted, not updated ).

So if you query from SQL Server Management Studio (SSMS), the [InvoicesHistory] table has no records. (Of course, because there were no updates)

The surprise is that if you query from EFC , as many records as the temporary table has are retrieved.
i.e

Query from SSMS
Invoices has 5 insertions,
InvoicesHistory has 0 insertions (because Invoices were no updates)

The surprise is that if you query the InvoicesHistory table from EFC, it returns 5 records. (for this example)

 var invoicesHistory = context.Invoices.TemporalAll().ToList();  // invoicesHistory.Count =5
 var count = context.Invoices.TemporalAll().ToList().Count;   // count = 5 
     /*
     * If the records were not updated, the historical table should not retrieve  records,
     * you would expect the same behavior as SQL Server. 
     */

stack traces

   at System.Environment.get_StackTrace()\r\n   at MyForms.Form1.button1_Click(Object sender, EventArgs e) in C:\\Users\\PHILIPS.JESUSMENDOZA\\source\\repos\\MyForms\\Form1.cs:line 71\r\n   at System.Windows.Forms.Control.OnClick(EventArgs e)\r\n   at System.Windows.Forms.Button.OnClick(EventArgs e)\r\n   at System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent)\r\n   at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks)\r\n   at System.Windows.Forms.Control.WndProc(Message& m)\r\n   at System.Windows.Forms.ButtonBase.WndProc(Message& m)\r\n   at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m)\r\n   at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, WM msg, IntPtr wparam, IntPtr lparam)\r\n   at Interop.User32.DispatchMessageW(MSG& msg)\r\n   at Interop.User32.DispatchMessageW(MSG& msg)\r\n   at System.Windows.Forms.Application.ComponentManager.Interop.Mso.IMsoComponentManager.FPushMessageLoop(UIntPtr dwComponentID, msoloop uReason, Void* pvLoopData)\r\n   at System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner(msoloop reason, ApplicationContext context)\r\n   at System.Windows.Forms.Application.ThreadContext.RunMessageLoop(msoloop reason, ApplicationContext context)\r\n   at System.Windows.Forms.Application.Run(Form mainForm)\r\n   at MyForms.Program.Main() in C:\\Users\\PHILIPS.JESUSMENDOZA\\source\\repos\\MyForms\\Program.cs:line 14

Include provider and version information

EF Core version: EFC 6.0.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 6.0
Operating system:
Edición Windows 10 Pro
Versión 21H2
Se instaló el ‎08/‎12/‎2021
Compilación del SO 19044.1387
Experiencia Windows Feature Experience Pack 120.2212.3920.0

IDE: Microsoft Visual Studio Professional 2022 (64-bit)
Version 17.0.3

image

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 18, 2021

You are querying Invoices...

@mendozagit
Copy link
Author

Hi @ErikEJ ,

Based on official documentation,
TemporalAll(): Returns all rows in the historical data. This is typically many rows from the history table and/or the current table for a given primary key.

If this is not the correct way to consult the historical table, could you please guide me as to what should be the right way.

Thank you very much! :)

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 18, 2021

Sorry, I thought you had a InvoicesHistory entity

@mendozagit
Copy link
Author

I understand, don't worry.

When you have a little time, you can take a look please. I am convinced that many of us would appreciate it! :)

@maumar maumar self-assigned this Dec 19, 2021
@maumar
Copy link
Contributor

maumar commented Dec 19, 2021

TemporalAll maps to FOR SYSTEM_TIME ALL operation on sql server, which according to tsql documentation "Returns the union of rows that belong to the current and the history table.". So we should adjust documentation to reflect that. But should consider providing a (convenient) way to query the history table directly given the discrepancy between ALL and history table shown in this scenario. For now you can use FromSql - make sure to use NoTracking

@ajcvickers
Copy link
Contributor

Note that the What's new documentation was recently updated to reflect this.

@mendozagit
Copy link
Author

@ajcvickers, do you refer this ?

Now say that current or history data...
but how can I know which data are of history table, and which data is te current table, if when I query history table both data is retrieved? (Data of current and history)

image

@ajcvickers
Copy link
Contributor

@mendozagit As it says in the linked doc, "Notice that the ValidTo column (by default called PeriodEnd) contains the datetime2 max value. This is always the case for the current rows in the table."

@mendozagit
Copy link
Author

Staff, thanks for the clarifications.

Personally, I think the temporary tables are a wonderful feature of EFC 6, however, in my opinion it is confusing how to query.

I think it will be a great tool, but it can be improved further.

For example,
1-Could use normal properties (not shadow properties).
2-Could the properties PeriodEnd and PeriodStart be nullable (for semantics)
3-There could be a simple and transparent way to query historical data,

Example for item 3
The TemporalAll() method should simply translate for a select * from InvoiceHistory, as it happens in SSMS.
(the filters should be placed using the where clause, as always)

Something like that.. , so in this
Remember that we are under the assumption that the first 5 invoices were inserted (insert only, no update).

where idToAudit be 1 or 2 or 3 or 4 or 5. query, should not recover records, because there was no update or deletion.

var idToAudit=5;

var query = context.Invoices.TemporalAll()
                       .where(i => i.InvoiceId = idToAudit)
                       .ToList();

Example.

If I insert 5 records in the invoice table (only insert, no update)

SSMS: Select * from InvoiceHistory returns 0 records.
EFC 6: Context.Invoice.TemporalAll() returns 5 records.

This in my opinion is confusing.

In my opinion, based on readings, forums, videos this would help developers a lot.
I certainly appreciate your time and guidance!

@ajcvickers
Copy link
Contributor

@mendozagit

1-Could use normal properties (not shadow properties).

This is tracked by #26463.

2-Could the properties PeriodEnd and PeriodStart be nullable (for semantics)
3-There could be a simple and transparent way to query historical data,

The EF Core implementation aligns with the way this is implement in SQL Server. See https://docs.microsoft.com/en-us/sql/relational-databases/tables/temporal-tables?view=sql-server-ver15. The way the columns and queries are handled matches how temporal tables are implemented by SQL Server. I don't think it makes sense for EF to do something different here; that would both be confusing, and would create mapping issues when attempting to go from EF to SQL Server.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

4 participants