JordanMarr / SqlHydra

SqlHydra is a suite of NuGet packages for working with databases in F# including code generation tools and query expressions.
MIT License
212 stars 20 forks source link

SQL Server Temporal Tables Select Issue #77

Closed nicholas-peterson closed 1 month ago

nicholas-peterson commented 6 months ago

I get an exception selecting from SQL Server tables with Temporal Versioning turned on when using the 'HIDDEN' column feature for the temporal columns.

Here's an example, in SQL:

CREATE TABLE [dbo].[test_table] (
RecordId INT NOT NULL IDENTITY(1,1) PRIMARY KEY,
RecordStateAt DATETIME2(7) GENERATED ALWAYS AS ROW START HIDDEN NOT NULL,
RecordStateUntil DATETIME2(7) GENERATED ALWAYS AS ROW END HIDDEN NOT NULL,
PERIOD FOR SYSTEM_TIME(RecordStateAt, RecordStateUntil)
) WITH (SYSTEM_VERSIONING =  ON(HISTORY_TABLE = [dbo].[test_table_history]));
GO

INSERT INTO dbo.test_table (RecordStateAt) VALUES (default);

SELECT * FROM dbo.test_table;

The result of that final SELECT *... query returns just this column: RecordId: 1

If I explicitly name the columns in the select list of SqlHydra, that works, but otherwise it throws an exception about the fieldcount not being long enough.

The work around is to not declare the columns as hidden in t-sql.

I think if SQL Hydra avoided using SELECT * format of querying and instead wrote out all the columns, it would work around this issue. Might make for a decent config option?

JordanMarr commented 6 months ago

Interesting. I'm hesitant to change the default behavior from * to an explicit list of columns as that could cause a run-time breaking change for some unknown edge case.

But I can imagine having a way to explicitly ask for all table columns to be selected.

Maybe something like:

    selectAsync HydraReader.Read (Create openContext) {
        for c in SalesLT.Customer do
        leftJoin ca in SalesLT.CustomerAddress on (c.CustomerID = ca.Value.CustomerID)
        leftJoin a  in SalesLT.Address on (ca.Value.AddressID = a.Value.AddressID)
        select (allColumns c, a)
    }

Or even a custom operator like !* or !** that could be applied to a table:

    selectAsync HydraReader.Read (Create openContext) {
        for c in SalesLT.Customer do
        leftJoin ca in SalesLT.CustomerAddress on (c.CustomerID = ca.Value.CustomerID)
        leftJoin a  in SalesLT.Address on (ca.Value.AddressID = a.Value.AddressID)
        select (!* c, a)
    }
nicholas-peterson commented 6 months ago

Part of me thinks it’s a really weird choice to hide the columns at all (I turned it off on all my tables). It feels like it breaks a pretty strong convention of querying sql tables. But felt l should mention it once I saw it.

I wonder how other sql server oriented DB libraries handle it (it’s been in since 2016/2017).

JordanMarr commented 6 months ago

I wasn’t even aware of the feature of hidden columns in SQL Server. Should hidden columns be excluded from the generated types?

nicholas-peterson commented 6 months ago

It might make sense to remove them from the generated types. My reasoning is that t-sql also has special keywords for querying temporal tables that the query builder doesn’t support (I don’t think?), so it’s conceptually easier for hydra to opt out of temporal tables in general.

JordanMarr commented 6 months ago

You can also filter our tables and/or columns in your toml file:

https://github.com/JordanMarr/SqlHydra/wiki/TOML-Configuration#filtering

For example, you can filter out the table entirely:

[filters]
include = [ "*" ]
exclude = [ "dbo/test_table" ]

Or leave the table but filter out the hidden columns:

[filters]
include = [ "*" ]
exclude = [ "dbo/test_table.RecordStateAt",  "dbo/test_table.RecordStateUntil" ]
nicholas-peterson commented 1 month ago

Filtering the columns is a great idea honestly. Part of my original intent with hiding the columns in T-SQL was I didn't want the application to rely on them in any way (I wanted it to be purely a side-effect managed by the database engine). Marking them hidden and then adding the column filter feels like a great way to achieve that effect. My theory is, if I ever wanted to display the historical data in the application, I could probably just create a view backed by a query using these features and then allow SQLHydra to see that view.

JordanMarr commented 1 month ago

I think v2.5 should fix your issue. https://github.com/JordanMarr/SqlHydra/releases

Also, you shouldn’t have to filter the columns as I previously suggested (unless you want to). v2.5 should fix the issue because the generated HydraReader.Read has been fixed so that it ignores columns that aren’t in your queried type.

JordanMarr commented 1 month ago

Closing as I believe this issue is completely resolved. Feel free to reopen if not.