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

Why doesn't casbin_rule add an auto increment primary key? #35

Closed
aofengli opened this issue Jul 8, 2020 · 11 comments · Fixed by #54
Closed

Why doesn't casbin_rule add an auto increment primary key? #35

aofengli opened this issue Jul 8, 2020 · 11 comments · Fixed by #54
Assignees

Comments

@aofengli
Copy link

aofengli commented Jul 8, 2020

One should be added from a performance perspective

@aofengli
Copy link
Author

aofengli commented Jul 8, 2020

I don’t quite understand why you don’t increase the primary key field. According to the mysql innodb index design, it is recommended to provide an ID primary key

@nodece
Copy link
Member

nodece commented Jul 8, 2020

I guess the Casbin don't need the ID primary key.

@hsluoyz
Copy link
Member

hsluoyz commented Jul 8, 2020

@aofengli can you make a PR to add an increment primary key?

@hsluoyz hsluoyz self-assigned this Jul 8, 2020
@hsluoyz
Copy link
Member

hsluoyz commented Jul 18, 2020

@00LT00 can you take this issue?

@00LT00
Copy link
Member

00LT00 commented Jul 18, 2020

I'm learning about it and solving it as soon as possible

@00LT00
Copy link
Member

00LT00 commented Jul 31, 2020

  1. When casbin is loaded, it has already loaded from database to memory. So the database is not selected during runtime. The primary key has no effect on select operations.
  2. Because the gorm-adapter just is a tool to get policy. The struct in memory must be the same as "policy.csv". In fact, the primary key cannot be used at all when deleting.

In summary, I think we shouldn't add ID primary key for casbin.
What do you think? @aofengli @hsluoyz

@hsluoyz
Copy link
Member

hsluoyz commented Aug 1, 2020

I don’t quite understand why you don’t increase the primary key field. According to the mysql innodb index design, it is recommended to provide an ID primary key

@aofengli can you make a PR to fix it?

@nodece
Copy link
Member

nodece commented Aug 1, 2020

@hsluoyz We should add unique index.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 1, 2020

OK. Can anyone work on a PR?

@00LT00
Copy link
Member

00LT00 commented Aug 2, 2020

@hsluoyz @nodece add unique index

@00LT00 00LT00 mentioned this issue Sep 26, 2020
@hsluoyz
Copy link
Member

hsluoyz commented Sep 27, 2020

PR made: #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants