Minor schema changes #104
Minor schema changes #104
Conversation
- 'default' is a reserved keyword in mysql, so I changed it to is_default. I have not seen an occurrence of this column in the actual code, so impact seems minimal. - Although a parent column is present, it did not have a foreign key yet, so added that. - The character set was set to utf8, given that role names are something used internally, and the fact that they're used as primary keys, I figured they could better be latin1 (reasoning below) - Because Mysql reserves the maximum amount of bytes for a varchar field, it would reserve 3*255 bytes (3 because of charset, 255 because of maxlenght). This seems a tad overkill, hence I reduced it to a more sensible amount (I think).
| `parent` varchar(255) DEFAULT NULL, | ||
| PRIMARY KEY (`role_id`) | ||
| CREATE TABLE IF NOT EXISTS user_role ( | ||
| role_id varchar(32) CHARACTER SET latin1 NOT NULL, |
There was a problem hiding this comment.
Please get rid of latin1 and 32
|
About latin1: that kind of optimization is useless to most users - let it to DBAs |
|
I'm 👍 on @Ocramius's suggestions. Specifying Latin-1 is a micro-optimization at best, and devs/DBAs are free to modify field attributes as they wish. I agree with the |
|
Yup, needs to be documented in the upgrade notes for the coming 1.3 release though |
|
On second thought, I am thinking that a schema change would be more appropriate to be included in a 2.0 release. Because we (purposefully) do not provide a public API for modifying roles, it seems that changing the schema should require a major version bump (per SemVer). I will preemptively place this in the 2.0.0 milestone. |
|
@bjyoungblood we're already having BC breaks in the upcoming 1.3 |
|
Okay, then this sounds good for a 1.3 release, as long as all of the BC breaks are documented. I'd like to start following SemVer with 2.0 -- will be opening an RFC issue shortly. |
|
Closing, see #113 |
is_default. I have not seen an occurrence of this column in the
actual code, so impact seems minimal.
key yet, so added that.
something used internally, and the fact that they're used as
primary keys, I figured they could better be latin1 (reasoning below)
field, it would reserve 3*255 bytes (3 because of charset, 255
because of maxlenght). This seems a tad overkill, hence I reduced
it to a more sensible amount (I think).
Oh, I would propose to also change the table name to user_x_role (or role_x_user, alphabetically) instead of user_role_linker as that would be more in line with naming conventions, but I figured that would break BC, so left that out.