Add PK or set replication identity

[fgblomqvist]

Since the table that is created does not have a PK, it's impossible to do DELETE/UPDATE statements on it in case the DB instance has a publication set for ALL TABLES (https://www.postgresql.org/docs/current/logical-replication-publication.html). This is not uncommon in more complex environments.

In order to prevent people from having to fix the table created by this adapter, I would propose either adding a PK to it or setting the REPLICA IDENTITY to FULL. If you go with the PK, you could either do it on the full row (not sure if there are any bad side effects from that) or simply add an ID column (seems a bit useless perhaps). Setting the REPLICA IDENTITY can be done using ALTER TABLE ... REPLICA IDENTITY FULL.

[hsluoyz]

@fgblomqvist we should add a unique ID column like how we did for Gorm Adapter: casbin/gorm-adapter#54

[fgblomqvist]

One question I got is since Casbin wants adapters to fully manage their DBs/tables, I'm guessing that means that a migration has to be created for this? How will that work out?

[hsluoyz]

@closetool do you think we need migration?

[kilosonc]

@hsluoyz I don't get it why it's necessary

[hsluoyz]

@fgblomqvist I think we can put a notice about the db format change, like how we did it previously: jcasbin/casbin-spring-boot-starter#25

[fgblomqvist]

It's just a bit uggly to force the user to add the field when Casbin does the table creation. If it was completely managed by the user it would make sense. But oh well.

[hsluoyz]

@fgblomqvist what kind of DB migration do you refer to? Can you provide some details?

[fgblomqvist]

https://www.cloudbees.com/blog/database-migration/
Obviously you wouldn't just "manually add the column" in a production system. You'd need a migration. If you guys add that column without automatically migrating the existing table, it becomes a breaking change. Technically you could easily check when Casbin loads if that column exists, and if it doesn't run the SQL to add it. You could strip that code in a later major version.

[hsluoyz]

@fgblomqvist OK. Then I think we are on the same page. The question on my mind is that is there any production use for this adapter given its popularity? If not, the migration code will become useless efforts.

[fgblomqvist]

Fair enough, perhaps the adoption is not there yet. I can figure things out on my end. But regardless, having a PK column would be very nice indeed.

[hsluoyz]

@fgblomqvist thanks for understanding. Currently still waiting for: #8 to be merged, then we will fix this issue.