Efcore: Columnstore Index causes unnecessarily-specific Scaffold-DB output.

[ixtreon]

After uprading all our components to version 3.1-preview3 we noticed scaffold-db started picking up a previously ignored table which utilizes an SQL Server columnstore index[1]. The table contains a column of type nvarchar(max) which, when scaffolded, results in a model that won't load[2] in Sqlite. The generated code includes a .HasColumnType("varbinary(max)") line as part of the column's configuration, causing the Sqlite provider to throw an exception for it doesn't recognize that type.

This seems to also happen on columns of type varchar and varbinary. It only happens if the length is max and only if there is a columnstore index in place.

[1] - it is clustered so the table doesn't have a conventional PK, meaning prior to 3.1 it was considered a query type and wasn't picked up (my guess).

[2] - We create in-memory Sqlite instances for tests by using the DatabaseFacade.GenerateCreateScript() method.

Steps to reproduce

Create a table with an array-based column of length max with a CSI:

CREATE TABLE [TestTable] (
    [ID] [int] IDENTITY(1,1),
    [Body] [nvarchar](max) NOT NULL
)
GO

CREATE CLUSTERED COLUMNSTORE INDEX cci_Test ON [TestTable]
GO

Running scaffold-db against this DB creates a PrimaryContext.cs with the following config:
```C#
modelBuilder.Entity(entity =>
{
entity.HasNoKey();

entity.ToTable("TestTable");

entity.HasIndex(e => new { e.ID, e.Body })
    .HasName("cci_Test");

entity.Property(e => e.Body)
    .IsRequired()
    .HasColumnType("nvarchar(max)");

entity.Property(e => e.ID).ValueGeneratedOnAdd();

});

This breaks with Sqlite which uses the `TEXT` type. Changing the `Body` column to `nvarchar(255)` modifies the relevant part of the config to
```C#
entity.Property(e => e.Body)
     .IsRequired()
     .HasMaxLength(255);

Which works fine with Sqlite. Similarly, removing the cci_Test index results in the following (valid) config:
C# entity.Property(e => e.Body).IsRequired();
I would've expected to see this same result from the initial example - which would seemingly make it work correctly with both SQL Server and Sqlite.

Further technical details

EF Core version: 3.1-preview3
Database provider: Microsoft.EntityFrameworkCore.SqlServer, Microsoft.EntityFrameworkCore.Sqlite
Target framework: net461
Operating system: Win10
IDE: Visual Studio 2019 16.3

[ErikEJ]

Scaffolding is provider specific, so you cannot expect a scaffolded SQL Server model to work with SQLite, if it does it is pure luck!

[ixtreon]

While I see what you mean, this directly breaks the possibility of testing code that uses reverse-engineering to keep the DB model up-to-date, when using Sqlite as the testing back-end, as suggested here. As the first article implies, this would not be an issue if or when #831 is implemented.

All of this is secondary to the issue at hand which naively seems to be that the scaffold tool has trouble recognizing column types consistently. For a known type, I would expect the tool to use only one of the following formats:

• explicitly specify its type and size using HasColumnType("<type>(<length>)")
• use type defaults and known methods such as HasMaxLength(<length>)

The examples I gave suggest this is not the case. Additionally, I would not expect the presence of an index or the changing of a column size to affect the type of any column as seen by the tool.

[smitpatel]

The issue here seem to be nvarchar(max) is being scaffolded in fluent API even though it should be default. ColumnStore index is red-herring.

@ixtreon - As @ErikEJ said, scaffolding DbContext for 1 provider and using it for another may work in some scenario but it is certainly not a supported scenario.

[smitpatel]

CREATE TABLE [TestTable] (
[ID] [int] IDENTITY(1,1),
[Body] [nvarchar](max) NOT NULL
)
GO
CREATE CLUSTERED COLUMNSTORE INDEX cci_Test ON [TestTable]
GO

The script you provided fails with error
The statement failed. Column 'Body' has a data type that cannot participate in a columnstore index. Omit column 'Body'.

If I use something other than nvarchar(max) like nvarchar(100) then scaffolded code contains
C# entity.Property(e => e.Body) .IsRequired() .HasMaxLength(100);

Please provide a runnable project which demonstrate the issue you are seeing.

[ajcvickers]

@ixtreon With regard to testing we have an issue to update the documentation. The trade-off here is between ease of setting up a test database against how closely that database matches what the production database will do. For example, if the target is SQL Azure, then:

• Testing against the in-memory database is sufficient for simple updates and queries, but behavior specific to relational databases may be different or missing.
• Testing against a SQLite in-memory database will bring in more of the relational-specific behaviors and hence be closer to how SQL Azure behaves.
• Testing with SQL Server locally will be very close to how SQL Azure works, but there can still be differences.
• Testing against SQL Azure will ensure the database behaves the same as in production.

[ixtreon]

@smitpatel Using SQL Server Dev 64-bit, v14.0.2027.2 the following SQL script outputs "Commands completed successfully" in SSMS; the table is created and a CCI is present:

CREATE TABLE [TestTable] (
    [ID] [int] IDENTITY(1,1),
    [Body] [nvarchar](max) NOT NULL
)
GO

CREATE CLUSTERED COLUMNSTORE INDEX cci_Test ON [TestTable]
GO

This is slightly different from the SQL you've run which contains [Body] nvarchar NOT NULL (the max is omitted). Both versions seem to work on my SQL Server instance and create a table with a corresponding CCI. The second one creates a column of type nvarchar(1) though which shouldn't trigger the issue from what I've observed.

@ajcvickers I am aware the two providers are different. This was the reason to choose Sqlite over in-memory for testing. I am afraid I'm not catching your broader point though.

[smitpatel]

@ixtreon - I copy-pasted your SQL only. (it was not nvarchar(1), just github's formatting failed)

I still hit the error

It requires SqlServer 2017. We will investigate on our end further. https://docs.microsoft.com/en-us/sql/t-sql/statements/create-columnstore-index-transact-sql?view=sql-server-ver15#LimitRest

[smitpatel]

@AndriySvyryd verified that output posted above is correct.

Column Body is part of an index. According to SqlServerTypeMappingSource indexes have limited max length. 450 for unicode string hence nvarchar(450). Across all SqlServer versions supported by EF Core, some of them requires such constraint and disallow creating index on column with datatype nvarchar(max) (As in above for SqlServer 2016). Which means that the type we encountered for this model in database is non-default hence we have to scaffold it. Since it is max size, we need to use HasColumnType rather than using HaxMaxLength.

While the invariant that string column participating in index must be limited size may not be true across all versions. Cost of making typeMapping version specific is high. The downside is, code need to specify column type explicitly which is not too bad.

Closing as by-design