8000 override id column · Issue #127 · ent/ent · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

override id column #127

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.

Sign up for GitHub

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

Jump to bottom
Closed
marwan-at-work opened this issue Oct 25, 2019 · 23 comments · Fixed by #338
Closed

override id column #127

marwan-at-work opened this issue Oct 25, 2019 · 23 comments · Fixed by #338

Comments

@marwan-at-work
Copy link
Contributor

Is there a way to redeclare the id column as a custom string type?

If I put the following in my schema:

field.String("id").Unique()

The generated code ends up broken because it creates two declarations of "id", but furthermore it ignores the fact that it's a string, and declares it as an integer instead.

This might be related to the UUID discussions, but it would be great if we can define our own custom primary keys as strings that we provide at runtime.
@alexsn
Copy link
Collaborator
alexsn commented Oct 25, 2019 •
edited
Loading

Currently the "id" field is implied so there's no way to override the underlying storage key. The above setting should give a better error message where a reserved field name is used.

Are you looking to rename the "id" column name, provide your own PKs or both?

@a8m a8m mentioned this issue Oct 25, 2019
Open
27 tasks
@a8m
Copy link
Member
a8m commented Oct 25, 2019

This is part of our roadmap and it should be addressed in the upcoming week (or 2).
I linked this issue to the roadmap item.

@marwan-at-work
Copy link
Contributor Author

@alexsn im not looking to rename "id". I'm looking to keep it "id" but change its type from integer to string, and also be able to provide my own value when i create a row.

Thanks ✌️

@a8m a8m mentioned this issue Nov 4, 2019
Closed
@BitPhinix
Copy link
BitPhinix commented Nov 19, 2019 •
edited
Loading

Seems to work with the latest matcher branch, but there isn't a uuid type available. Is there any way to use a Postgres UUID primary key with ent?

@a8m
Copy link
Member
a8m commented Nov 20, 2019

Hey @BitPhinix, this feature is still in development.
I'll update the issue (and the roadmap) once we finish the implementation.

@aca
Copy link
Contributor
aca commented Dec 11, 2019

@a8m Feature Proposal
I think it is related to this, so I write it here.
Can we customize struct tag for field "id" ?

I'm currently using type generated by ent, and I want to make json reponse with no ID field.
Which is achievable by Sensitive() for other fields.
But It seems it is impossible to make edit struct tag for ID field.

type Project struct {
    ent.Project
    ID `json:"-"`
}

I thought I could solve this by doing something like this. But didn't worked well.

@a8m
Copy link
Member
a8m commented Dec 11, 2019

Hey @aca, did you try to override the struct tag? See - https://entgo.io/docs/schema-fields/#struct-tags

@aca
Copy link
Contributor
aca commented Dec 11, 2019

@a8m I meant ID field which is generated by entc not by defining ent.Field

// Fields of the Namespace.
func (Namespace) Fields() []ent.Field {
	return []ent.Field{
		field.String("name").Unique().Match(regexp.MustCompile("[a-z0-9]+(?:[._-][a-z0-9]+)*")).NotEmpty(),
		field.Int64("id").StructTag("-").Unique(),
	}
}

Defining "id" field doesn't seems to work. Really sorry If I misunderstood.

@a8m
Copy link
Member
a8m commented Dec 11, 2019

There's no option to override the ID's struct-tag except using custom template (which is too complex in this case imo).

This definitely should be addressed. I'll update the issue once we get to it.

@a8m a8m mentioned this issue Dec 22, 2019
Closed
@gukz
Copy link
gukz commented Jan 10, 2020

Hey, dear @a8m , I am rewriting a project with go , I enjoy using ent, but this project has a table with a user_id primary key. Can ent allow specify any field as the primary key?

@a8m
Copy link
Member
a8m commented Jan 10, 2020 •
edited
Loading

Hey, dear @a8m , I am rewriting a project with go , I enjoy using ent, but this project has a table with a user_id primary key. Can ent allow specify any field as the primary key?

I ran the codegen with this option in the schema and it worked for me.

func (User) Fields() []ent.Field {
	return []ent.Field{
		field.Int("id").
			StorageKey("user_id"),
		// more fields..
	}
}

The database in MySQL was created as follows:
image

You are welcome to try it.

Like I mentioned above, once we get to this task, we'll handle all these cases.
Thanks!

@marwan-at-work
Copy link
Contributor Author
marwan-at-work commented Jan 28, 2020 •
edited
Loading

I know the issue is still being worked on, but just as an update:
Defining an ID as a string no longer creates a build error, but it now generates a runtime error when running a migration.

Given the following field.String("id").Unique()

The following error shows up from the create table operation:

sql/schema: create table "specs": Error 1071: Specified key was too long; max key length is 767 bytes

The interesting part, is that if I change the schema to say: field.String("id").Unique().MaxLen(25)

Then the same error happens. I noticed that the schema.go file doesn't populate the "id" Size the same way it would do for other fields.

If I manually change schema.go inside the migrate directory to include Size: 25 then the migration successfully works.

@marwan-at-work
Copy link
Contributor Author

@a8m another issue I discovered (just in case you missed it), is that where.go tries to convert an ID string to an integer while ignoring the error, which results in always bringing back the first row.

// IDEQ applies the EQ predicate on the ID field.
func IDEQ(id string) predicate.Spec {
	return predicate.Spec(func(s *sql.Selector) {
		id, _ := strconv.Atoi(id)
		s.Where(sql.EQ(s.C(FieldID), id))
	},
	)
}

This should just be

// IDEQ applies the EQ predicate on the ID field.
func IDEQ(id string) predicate.Spec {
	return predicate.Spec(func(s *sql.Selector) {
		s.Where(sql.EQ(s.C(FieldID), id))
	},
	)
}

But in what scenario do you expect an ID that is a string but must be converted to an integer? Will removing that one line completely not be sufficient?

Thanks!

@a8m
Copy link
Member
a8m commented Feb 9, 2020

Hey @marwan-at-work and thanks for reporting these issues. Regarding your comments:

Given the following field.String("id").Unique()

The following error shows up from the create table operation:

sql/schema: create table "specs": Error 1071: Specified key was too long; max key length is 767 bytes

I'm aware about this and it should be addressed. I'm pretty sure this is the last thing left for closing this issue.

@a8m another issue I discovered (just in case you missed it), is that where.go tries to convert an ID string to an integer while ignoring the error, which results in always bringing back the first row.

Where did you encounter this code? in your generated code or in this repository? If you don't run codegen with --idtype string, the logic shouldn't be there.

This (legacy) option was added to allow using the same generate ent package for both AWS Neptune (gremlin) and SQL dialects. Neptune uses UUID strings for its IDs, and MySQL is configured with numeric types for its PKs (in our deployment). Therefore, the type in the code must be string, and we convert it to int before/after the data were loaded.

I don't see any reason for using this option, except our exceptional case that was used for migrating from Neptune to MySQL. I'll suggest to not use it because we plan to drop it soon.

@marwan-at-work
Copy link
Contributor Author

@a8m

Where did you encounter this code?

I'm using the latest commit of entc and I was not aware of the --idtype flag at all. So I was just using entc generate ./ent/schema

I created a reproduction repository here: https://github.com/marwan-at-work/entrepro

You can see that my schema has the following ID definition https://github.com/marwan-at-work/entrepro/blob/master/ent/schema/spec.go#L14:L22

But then then the code that was generated from entc generate ./ent/schema gave these results: https://github.com/marwan-at-work/entrepro/blob/master/ent/spec/where.go#L21:L26

If that can just be removed from the template, I'm happy to contribute a fix, here's what I did locally and it seemed to work: marwan-at-work@e3b004b#diff-a01877e4cc82393fb3d3f48188992b02

Thanks!

@a8m
Copy link
Member
a8m commented Feb 10, 2020 •
edited
Loading

Thanks for helping debugging this @marwan-at-work.
That looks like a bug (I need to deep dive into it), but it can't be solved with just removing this part from the template (for the reason I've commented above).

Can you please run the codegen as describe here, or just use:

go run github.com/facebookincubator/ent/cmd/entc generate ./ent/schema

Just to make sure we're running the same version (as in go.mod).

I'll update once it's fixed.

a8m added a commit that referenced this issue Feb 10, 2020
@a8m
entc/gen: currently handle user-defined string ids
e42d40a 
Fixes #127
@a8m a8m mentioned this issue Feb 10, 2020
Merged
@a8m a8m closed this as completed in #338 Feb 10, 2020
a8m added a commit that referenced this issue Feb 10, 2020
@a8m
entc/gen: currently handle user-defined string ids (#338)
9733051 
Fixes #127
8000 @a8m a8m reopened this Feb 10, 2020
@a8m
Copy link
Member
a8m commented Feb 10, 2020

@marwan-at-work - I've added a fix for this, and checked it on your repo using this version: github.com/facebookincubator/ent@9733051cc361.

Please check it and update me if there's any issue with this.

@marwan-at-work
Copy link
Contributor Author

@a8m just tried it with that commit and it looks great!!

@a8m
Copy link
Member
a8m commented Feb 16, 2020

All comments that were raised in this issue addressed (hopefully), although there are some docs changes that need to be done.

Please give it a try and update us if there's any issue with this. Closing.

@a8m a8m closed this as completed Feb 16, 2020
@a8m a8m mentioned this issue Mar 11, 2020
Closed
@a8m a8m mentioned this issue Jul 23, 2020
Closed
@jiamo
Copy link
jiamo commented Aug 16, 2022

If we have multi schema in same dic. I only want one schema replace id. Can we put idtype in schema file instead using option --idtype?

@nvhai245
Copy link
nvhai245 commented Aug 19, 2022 •
edited
Loading

I think we could support some options to define a different primary key for schema, for example:
field.String("uid").PrimaryKey()

@a8m
Copy link
Member
a8m commented Aug 19, 2022

You can. See the second option here: https://entgo.io/docs/schema-fields#id-field

@seinshah
Copy link
seinshah commented May 5, 2025

This works as you mentioned, but just for your information, generating migration diff for the following scenario is not as expected:

  • Already have an int primary key (identity)
  • Deciding to remove that column
  • Use the existing oid column as the new key by renaming it internally to id and setting StorageKey as oid

Actual Migration File: ALTER TABLE ADD PRIMARY KEY ...
Expected Migration File: ALTER TABLE ... DROP COLUMN "id"

I confirmed that id is removed from table columns in the generated .../migrate/schema.go. I also confirmed there is no change when adding with-drop-column and the unique key on the oid column is droped if I use with-drop-index options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

entc/gen: currently handle user-defined string ids ent/ent
9 participants
@jiamo @alexsn @seinshah @a8m @gukz @BitPhinix @marwan-at-work @aca @nvhai245
0