Skip to content

Primary key name not taken into account when scaffolding #12821

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

Closed
roji opened this issue Jul 27, 2018 · 8 comments
Closed

Primary key name not taken into account when scaffolding #12821

roji opened this issue Jul 27, 2018 · 8 comments
Assignees
Labels
customer-reported good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Jul 27, 2018

Originally raised by @WarrenFerrell in npgsql/efcore.pg#538 (comment).

When scaffolding primary key constraints, the constraint name currently seems to be ignored. In other words, scaffolding the following table:

CREATE TABLE my_table (
	id INTEGER,
	CONSTRAINT table_pk PRIMARY KEY (id)
);

results in a model where the constraint name isn't set. If the database is recreated from this model, the PK constraint is called PK_my_table rather than table_pk, which is great if round-trippability is a goal. I've verified and the DatabasePrimaryKey returned by NpgsqlDatabaseModelFactory indeed contains the correct constraint name.

I'm guessing this is probably the case for other constraint types (foreign key, unique).

@smitpatel
Copy link
Contributor

Only case with PK. AK/Index/FK works correctly.

@bricelam bricelam self-assigned this Jul 27, 2018
@smitpatel smitpatel added the help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. label Jul 27, 2018
@smitpatel
Copy link
Contributor

We need to do similar to this in VisitPrimaryKey and VisitUniqueConstraint (which is also slightly buggy)

https://github.com/aspnet/EntityFrameworkCore/blob/5c8600e440498e992a7fc3c02d447534fc146334/src/EFCore.Design/Scaffolding/Internal/RelationalScaffoldingModelFactory.cs#L600-L604

Would you like to send a PR @roji ?

@roji
Copy link
Member Author

roji commented Jul 27, 2018

Sure, if @bricelam doesn't mind giving this up, although it might take me a bit of time to get around to it...

@bricelam
Copy link
Contributor

It'll take me a while to get around to it too, so just whoever gets to it first...

roji added a commit to roji/efcore that referenced this issue Jul 29, 2018
Also added scaffolding test for unique constraints.

Fixes dotnet#12821
roji added a commit to roji/efcore that referenced this issue Jul 29, 2018
Also added scaffolding test for unique constraints.

Fixes dotnet#12821
@roji
Copy link
Member Author

roji commented Jul 29, 2018

Submitted #12836

@ajcvickers ajcvickers added this to the 2.2.0 milestone Jul 30, 2018
smitpatel pushed a commit that referenced this issue Jul 30, 2018
Also added scaffolding test for unique constraints.

Fixes #12821
@bricelam bricelam removed their assignment Jul 31, 2018
@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label May 31, 2019
@ajcvickers ajcvickers modified the milestones: 2.2.0-preview2, 2.2.0 Nov 11, 2019
@OskarKlintrot
Copy link

Sorry for posting on a closed issue but I'm having a similar problem to this. I'm trying to use EF Core on an existing database that has a table with this schema:

CREATE TABLE [dbo].[Example] (
    [Id]                  INT                IDENTITY (1, 1) NOT NULL,
    PRIMARY KEY CLUSTERED ([Id] ASC)
);

However EF Core reverse engineers the table to

CREATE TABLE [dbo].[Example] (
    [Id]                  INT                IDENTITY (1, 1) NOT NULL,
    CONSTRAINT [PK_Example] PRIMARY KEY CLUSTERED ([Id] ASC)
);

Is there any way, using fluent api for example, to get rid of the name and constraint so that it actually matches the schema of the existing database? I'm not sure the constraint name actually matters in real life? It gives me a lot of false diffs though when I use SqlSchemaCompare in VS to make sure the new entities matches the database.

@shvmgpt116
Copy link

@roji @ajcvickers This issue seems to be present in 2.1 code line of EFCore (I verified it with 2.1.11 and 2.1.14 version of EFCore). Since 2.1 is an LTS, is there any plan to backport the fix to 2.1 code line?

@ajcvickers
Copy link
Contributor

@shvmgpt116 We have no plans to port this change to 2.1. At this point in 2.1's release cycle it would have to be an extremely serious issue to be ported there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. type-bug
Projects
None yet
Development

No branches or pull requests

6 participants