search

jcasbin / jdbc-adapter

JDBC adapter for Casbin
https://github.com/casbin/jcasbin
Apache License 2.0
34 stars 38 forks source link

`casbin_sequence` is generated even when `casbin_rule` table is made beforehand using `GenerationType.IDENTITY` #59

Closed offset-null1 closed 2 years ago

offset-null1 commented 2 years ago

I implicitly created casbin_rule table with Id generation type as IDENTITY. But it created casbin_sequence which wasn't required and happened because of case "PostgreSQL": sql = renderActualSql("CREATE SEQUENCE IF NOT EXISTS CASBIN_SEQUENCE START 1;");

in JDBCBaseAdapter.java . It should only be executed when the table doesn't exist otherwise the generation of sequence is redundant.

casbin-bot commented 2 years ago

@tangyang9464 @imp2002

hsluoyz commented 2 years ago

@OutOfEastGate

OutOfEastGate commented 2 years ago

@offset-null1 Thank you for your feedback. I will deal with it soon.

hsluoyz commented 2 years ago

@offset-null1 are you a user or contributor?

offset-null1 commented 2 years ago

@hsluoyz I'm using this module to contribute to this issue.

offset-null1 commented 2 years ago

Yes, that prevents the creation of casbin_sequence but as the default is true, I feel before generating the sequence there should be a check if the table exists or not. In my case I didn't set it to false but had a table already created, still the sequence was generated i.e., I ended up having 2 sequences in the database.

OutOfEastGate commented 2 years ago

Yes, that prevents the creation of casbin_sequence but as the default is true, I feel before generating the sequence there should be a check if the table exists or not. In my case I didn't set it to false but had a table already created, still the sequence was generated i.e., I ended up having 2 sequences in the database.

I know what you mean.By default, we create casbin_sequence. If you have your own requirements, you can modify the parameters of casbin_sequence directly.In mybatis adapter, sequence is not created by default. Of course, we will refine this section to check the existence of the table before creating the casbin_sequence.

offset-null1 commented 2 years ago

Okay, thank you for the clarification.

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 2.4.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: