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

Reverse engineering: Scaffolding reads and creates "hypothetical" indexes #7665

Closed
MateiNenciu opened this issue Feb 21, 2017 · 18 comments
Closed
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@MateiNenciu
Copy link

MateiNenciu commented Feb 21, 2017

I used EF Core CLI to generate my classes and DBContext from my production database but I observed some behaviour that I do not understand: for each table where I have statistics EF Core generates a new HasIndex entry. For example:

I have a table named Employees with 2 indexes (one clustered one non-clustered) and about 100 statistics generated.
EF core generates an entity.HasIndex(e=> new {....}).HasName("StatisticName") for every statistic. I observed this issue when I tried to make a migration but I recived an error: "The index StatisticName on the table Employees has 49 columns in that key list. The maximum limit for index key column list is 16".

I also noticed that it defines the PK with the name "StatisticName". I attached a picture with the code.

Steps to reproduce

Scaffold a database with table statistics.

Further technical details

Version:
"Microsoft.EntityFrameworkCore": "1.1.0",
"Microsoft.EntityFrameworkCore.Design": "1.1.0",
"Microsoft.EntityFrameworkCore.SqlServer": "1.1.0",
"Microsoft.EntityFrameworkCore.SqlServer.Design": "1.1.0",
"Microsoft.EntityFrameworkCore.Tools": "1.1.0-preview4-final"

Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 x64 latest version
IDE: Visual Studio Enterprise 2015 Update 3

capture1
capture2

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 21, 2017

These are not "statistics", these are user indexes created by a user via the Database Tuning Advisor (notice _dta in the name) - so they are "normal" indexes...

@MateiNenciu
Copy link
Author

MateiNenciu commented Feb 21, 2017

Then why the "The index StatisticName on the table Employees has 49 columns in that key list. The maximum limit for index key column list is 16" error if this is a normal index? And how about .HasKey naming?

@MateiNenciu
Copy link
Author

Thanks for explanation !

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 21, 2017

Aah, missed that - then please provide the CREATE TABLE and CREATE INDEX statements for the table in question

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 21, 2017

And what SQL Server version are you on?

@MateiNenciu MateiNenciu reopened this Feb 21, 2017
@MateiNenciu
Copy link
Author

For Table:
CREATE TABLE [Employees] (
[idEmployee] int NOT NULL IDENTITY,
....
CONSTRAINT [_dta_index_Employees_5_1429580131__K1] PRIMARY KEY ([idEmployee]),
....
)
For Indexes:

CREATE INDEX [dta_index_Employees_5_1429580131__K14_K34_K1_K2_K3_K4_K5_K9_K10_K11_K13_K15_K33_6_7_8_12_16_17_18_19_20_21_22_23_24_25_26_27] ON [Employees] ([name], [CNP], [employeePhone], [notes], [idMedInst_Lab], [saveInProgress], [beingEditedBy], [editStartTimestamp], [lastDDTProcessedTask], [datePlannedMedExR_man], [datePlannedMedExD_man], [datePlannedMedExC_man], [datePassedMedExP_man], [dateNextMedExP_calc], [datePlannedPsiExR_man], [datePassedPsiExP_man], [dateNextPsiExP_calc], [minMedExDate], [minMedExType], [minPsiExDate], [minPsiExType], [isDeleted], [idCurrentEmployeeState], [idEmployee], [nextMedicalCheckUp], [idMedicalCheckUpType], [nextPsycologicalCheckUp], [isControlledByDispatchPoint], [idMedInst_Poli], [currentWeeklyWorkHours]);
GO
CREATE INDEX [dta_index_Employees_5_1429580131__K1_K14_K2_K3_K4_K5_K9_K10_K11_K13_K15_K16_K34_6_7_8_12_17_18_19_20_21_22_23_24_25_26_27_28] ON [Employees] ([name], [CNP], [employeePhone], [notes], [saveInProgress], [beingEditedBy], [editStartTimestamp], [lastDDTProcessedTask], [datePlannedMedExR_man], [datePlannedMedExD_man], [datePlannedMedExC_man], [datePassedMedExP_man], [dateNextMedExP_calc], [datePlannedPsiExR_man], [datePassedPsiExP_man], [dateNextPsiExP_calc], [minMedExDate], [minMedExType], [minPsiExDate], [minPsiExType], [currentWeeklyWorkHours], [idEmployee], [isDeleted], [nextMedicalCheckUp], [idMedicalCheckUpType], [nextPsycologicalCheckUp], [isControlledByDispatchPoint], [idMedInst_Poli], [idMedInst_Lab], [idCurrentEmployeeState]);
GO
CREATE INDEX [dta_index_Employees_5_1429580131__K1_K2_K3_K4_K5_K9_K10_K11_K13_K14_K15_K16_K34_6_7_8_12_17_18_19_20_21_22_23_24_25_26_27_28] ON [Employees] ([name], [CNP], [employeePhone], [notes], [saveInProgress], [beingEditedBy], [editStartTimestamp], [lastDDTProcessedTask], [datePlannedMedExR_man], [datePlannedMedExD_man], [datePlannedMedExC_man], [datePassedMedExP_man], [dateNextMedExP_calc], [datePlannedPsiExR_man], [datePassedPsiExP_man], [dateNextPsiExP_calc], [minMedExDate], [minMedExType], [minPsiExDate], [minPsiExType], [currentWeeklyWorkHours], [idEmployee], [nextMedicalCheckUp], [idMedicalCheckUpType], [nextPsycologicalCheckUp], [isControlledByDispatchPoint], [isDeleted], [idMedInst_Poli], [idMedInst_Lab], [idCurrentEmployeeState]);
.....

I will talk tomorrow with my data admin ( database management isn't my responsability ) for solving with this indexes... Anyway those indexes are not visible in the Index folder under the table Employees so I didn't know about their existence.
I don't understant why he named the constraint _dta_index_Employees_5_1429580131__K1... It should be PK_Employees.

I'm running SQL Server 2014 Enterprise, version 12.2.5000.0

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 21, 2017

You need to share your Migration code, I do not see anything here that would cause Migrations to fail - but notice that roundtripping from Scaffolding to Migrations is not really fully supported, it is either the code first migrations workflow or scaffolding workflow that are the choices

@MateiNenciu
Copy link
Author

MateiNenciu commented Feb 22, 2017

Code that is in my datehours_init migration :

        migrationBuilder.CreateTable(
            name: "Employees",
            ```
        columns: table => new
            {
                    idEmployee = table.Column<int>(nullable: false)
                    .Annotation("SqlServer:ValueGenerationStrategy",                                    SqlServerValueGenerationStrategy.IdentityColumn),
         ...............
            },
             constraints: table =>
            {
              //Why mapping a FK Constrain
                 table.PrimaryKey("_dta_index_Employees_5_1429580131__K1", x =>x.idEmployee);
                      .........
                }

               ......

                migrationBuilder.CreateIndex(
                name: "_dta_index_Employees_5_1429580131__K14_K34_1_2_3_4_5_6_7_8_9_10_11_12_13_15_16_17_18_19_20_21_22_23_24_25_26_27_28_29_30_31_32_",
                table: "Employees",
                columns: new[] { "idEmployee", "name", "CNP", "employeePhone", "nextMedicalCheckUp", "idMedicalCheckUpType", "nextPsycologicalCheckUp", "notes", "isControlledByDispatchPoint", "idMedInst_Poli", "idMedInst_Lab", "saveInProgress", "beingEditedBy", "editStartTimestamp", "lastDDTProcessedTask", "datePlannedMedExR_man", "datePlannedMedExD_man", "datePlannedMedExC_man", "datePassedMedExP_man", "dateNextMedExP_calc", "datePlannedPsiExR_man", "datePassedPsiExP_man", "dateNextPsiExP_calc", "minMedExDate", "minMedExType", "minPsiExDate", "minPsiExType", "currentWeeklyWorkHours", "isDeleted", "idCurrentEmployeeState" });
               .........
 When I'm trying to run this migration to update/generate my database it fails at creating those X column indexes ( X > 16 ).

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 22, 2017

OK, so first:

  • Using Scaffolding and Migrations together is not really a fully supported scenario yet, I think.
  • In a properly managed database you should never uncritically implement all indexes proposed by the Database Tuning Advisor (DTA)
  • That index; "_dta_index_Employees_5_1429580131__K14_K34" you should simply remove from the database (it has way too many included columns) (and remove from you model and from the migration)

So you have two questions:

1: A primary key is actually a Unique Index - so that just happens to be the (crazy) name of your primary key.

2: It looks like there is an issue with included columns being generated by the Reverse engineer process, causing too many index columns @lajones @bricelam ??

@MateiNenciu
Copy link
Author

I noticed that those indexes are hypothetical, they are not actual indexes. "_dta_index_Employees_5_1429580131__K14_K34" is marked as hypothetical for example.

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 22, 2017

Interesting - Where do you see that?

@MateiNenciu
Copy link
Author

Running a select from sys.indexes shows me that info. Column is_hypothetical is 1 for that index example.

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 22, 2017

Found it: sys.index is_hypothetical ! OK, suggest you drop it (then index) then, and I will do a PR for a fix to exclude hypothetical indexes - could you change the title of this issue?

@MateiNenciu MateiNenciu changed the title Scaffolding generates indexes from table statistics Scaffolding materialize hypothetical indexes Feb 22, 2017
@MateiNenciu
Copy link
Author

MateiNenciu commented Feb 22, 2017

Done. I started to drop them but they are about ~ 200 ...

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 22, 2017

And promise never to use DTA again! 😄

@MateiNenciu
Copy link
Author

MateiNenciu commented Feb 22, 2017

I promise as I am a developer!

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 22, 2017

ErikEJ pushed a commit to ErikEJ/EntityFramework that referenced this issue Feb 22, 2017
(for example created by Database Tuning Advisor - DTA)

fixes dotnet#7665
@MateiNenciu
Copy link
Author

Thanks for helping!! An old colleague of mine played with DTA and it seems he created those indexes. I found about hy indexes 20 minutes ago :))

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 22, 2017
@ajcvickers ajcvickers added this to the 2.0.0 milestone Feb 22, 2017
@divega divega added the type-bug label May 9, 2017
@ajcvickers ajcvickers changed the title Scaffolding materialize hypothetical indexes Reverse engineering: Scaffolding reads and creates "hypothetical" indexes May 9, 2017
@divega divega added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels May 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

4 participants