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

fix: duplication on adding policies to db #60

Closed
wants to merge 5 commits into from

Conversation

ndodanli
Copy link

No description provided.

@casbin-bot
Copy link
Member

@tangyang9464 @JalinWang @imp2002 please review

adapter.go Outdated Show resolved Hide resolved
Copy link
Member

@hsluoyz hsluoyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndodanli fix the CI:

image

@ndodanli
Copy link
Author

ndodanli commented Sep 16, 2023

@hsluoyz
There is a column naming difference between databases that i think xorm column naming is responsible for this. For example, 'pgtype' for mysql and 'pg_type' for postgres. I determine by checking driver name but setting a fixed column name for all databases is better, but setting fixed column names is not up to me.

Also i saw a usage for directly calling "pgtype" on UpdateFilteredPolicies()(adapter.go:550)->queryString()(adapter.go:643) and this can cause problems on postgres. You might want to check on this.

@hsluoyz
Copy link
Member

hsluoyz commented Sep 17, 2023

@ndodanli this PR is becoming more complicated, even involving home-made column names now. And the tests still failed. I have no confidence for this PR. So closed here. You can do de-duplication on your side anyway.

@hsluoyz hsluoyz closed this Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants