Refactor grant_tables to mysql_db
#1007
Conversation
jycor
changed the title
grant_tables to mysql_dbgrant_tables to mysql_db
jycor
changed the title
grant_tables to mysql_dbgrant_tables to mysql_db
sql/mysql_db/mysql_db.go
Outdated
| func (g *GrantTables) LoadData(ctx *sql.Context, users []*User, roleConnections []*RoleEdge) error { | ||
| g.Enabled = true | ||
| func (t *MySQLDb) LoadPrivilegeData(ctx *sql.Context, users []*User, roleConnections []*RoleEdge) error { | ||
| // TODO: this is bad do something else |
There was a problem hiding this comment.
Random comment? Looks like some leftovers, I'd remove if it's no longer relevant.
sql/mysql_db/mysql_db.go
Outdated
| g.persistFunc = persistFunc | ||
| // loadPrivilegeTypes is a helper method that loads privilege types given the length and loading function | ||
| // and returns them as a set | ||
| func loadPrivilegeTypes(n int, f func(j int) int32) map[sql.PrivilegeType]struct{} { |
There was a problem hiding this comment.
These load... functions should be in their own file.
sql/mysql_db/mysql_db.go
Outdated
| // persisted. | ||
| type PersistCallback func(ctx *sql.Context, users []*User, roleConnections []*RoleEdge) error | ||
| type PrivilegePersistCallback func(ctx *sql.Context, users []*User, roleConnections []*RoleEdge) error | ||
| type DataPersistCallback func(ctx *sql.Context, users []*User, roleConnections []*RoleEdge) error |
There was a problem hiding this comment.
I think I see what you were trying to get at here. If an integrator only has privileges (such as a database before this change), then they should still call DataPersistCallback, and they'll just pass nils to the other parameters. Having two callbacks that are functionally equivalent will complicate things.
This also extends to the LoadMySQLData call, we should only have this one and not the other one for privileges as well.
sql/mysql_db/fbs/mysql_db.fbs
Outdated
|
|
||
| table PrivilegeSet { | ||
| global_static:[int]; | ||
| //TODO: global_dynamic:[string]; |
There was a problem hiding this comment.
global_dynamic is fine as a string array. They'll all be unique entries so it'll be fine.
sql/mysql_db/fbs/mysql_db.fbs
Outdated
| password:string; | ||
| password_last_changed:int64; // time.Time? | ||
| locked:bool; | ||
| //attributes:string; // *string? |
There was a problem hiding this comment.
So since FlatBuffers don't support *string directly, you can mimic it with a struct. See https://stackoverflow.com/questions/53803880/flatbuffers-and-null-value
There was a problem hiding this comment.
Also, password_last_changed is fine as an int64.
There was a problem hiding this comment.
Looking at dolthub/dolt#3411 (review), we should make a few changes detailed in that review.
sql/mysql_db/mysql_db.go
Outdated
| ) | ||
|
|
||
| // PersistCallback represents the callback that will be called when the Grant Tables have been updated and need to be | ||
| // PrivilegePersistCallback represents the callback that will be called when the Grant Tables have been updated and need to be |
sql/mysql_db/mysql_db.go
Outdated
| // TODO: fill in other tables when they exist | ||
| return nil | ||
| } | ||
|
|
||
| // SetPersistCallback sets the callback to be used when the Grant Tables have been updated and need to be persisted. |
There was a problem hiding this comment.
Didn't update the comment here, still says Grant Tables
sql/mysql_db/mysql_db.go
Outdated
| persistFunc := g.persistFunc | ||
| if persistFunc == nil { | ||
| func (t *MySQLDb) Persist(ctx *sql.Context) error { | ||
| // TODO: future databases will no longer persist to privilege file, only to mysql.db; should document this |
There was a problem hiding this comment.
This is a Dolt comment, shouldn't be in GMS
| return serial.PrivilegeSetEnd(b) | ||
| } | ||
|
|
||
| func serializeAttributes(b *flatbuffers.Builder, attributes *string) flatbuffers.UOffsetT { |
There was a problem hiding this comment.
Should probably leave a comment explaining why the zero offset is necessary, and also that this should always be called as a result
Mostly renaming variables and files to better reflect what they are now.
Added flatbuffer files for MySQL DB.