Refactored how grant tables are stored #762
Conversation
Hydrocharged
force-pushed
the
daylon/users-4
branch
from
8f34377
to
0dc0d5d
Compare
| Host: host, | ||
| User: user, | ||
| return &User{ | ||
| User: row[userTblColIdxMap["User"]].(string), |
There was a problem hiding this comment.
Probably would be better to just define a set of const ints for these indexes, rather than putting them all in a big map like this
There was a problem hiding this comment.
I did it this way so that if the column ordering ever changed in the schema then we wouldn't have to update anything as it would all "just work", but I can make it all constant.
| ToRow(ctx *sql.Context) sql.Row | ||
| // Equals returns whether the calling entry is equivalent to the given entry. Equivalence should only be determined | ||
| // on values that have a direct mapping to and from a sql.Row. | ||
| Equals(ctx *sql.Context, otherEntry Entry) bool |
There was a problem hiding this comment.
This seems an odd addition given the warning about pointer types above.
Why not just use == with structs to compare them?
There was a problem hiding this comment.
So a Key is used in the internal map, so it can't be a pointer, otherwise map comparisons will use the pointer address rather than the struct contents. This is done to prevent potential bugs. If we had generics and I could constrain on non-pointer types, then the warning would be unnecessary.
An Entry, on the other hand, is used to represent some kind of data. We could technically constrain these to non-pointers as well and do struct comparisons, however entries may hold some "additional" data that cannot be directly manipulated by interacting with via SQL data manipulation statements (INSERT, etc.). Perhaps these are hidden columns, but AFAICT MySQL has some data in these internal tables that can't be seen from the outside. Basically this "additional" data mimics that, but it shouldn't affect equality. Like imagine it tracks some autoincrement ID internally, that would cause every entry to always be unique, but that may just be something for internal usage. That's the difference.
There was a problem hiding this comment.
I updated the comment to reflect this.
|
I'll address the feedback in my other PR, rather than update this one then have to rebase my other PR and deal with all of that. |
This refactors a lot of previously-written code to more manageably handle future modifications to the grant tables, and to ease their use and reduce potential development mistakes.