feat(vet): Add default query parameters for explain queries #2543
Conversation
kyleconroy
changed the title
internal/cmd/vet.go
Outdated
| case "float", "double": | ||
| return 0.1 | ||
| case "date": | ||
| t := time.Time{} |
There was a problem hiding this comment.
If we just want the zero time values I think we can dispense with instantiating the Go structs and just use hardcoded strings (see bottom of this page: https://dev.mysql.com/doc/refman/8.1/en/date-and-time-types.html). I wasn't sure that the zero values would be the best choice though.
| switch col.Type.Name { | ||
| case "any": | ||
| return nil | ||
| case "bool": |
There was a problem hiding this comment.
There is no "bool" type for MySQL; it's just an alias for tinyint with 0/1 values (which my Println() testing bears out). But maybe I'm not thinking correctly about the possible ways the input col gets constructed, and we might see something come through this code path with col.Type.Name == "bool"?
There was a problem hiding this comment.
It's an alias, but a cool columns will have that type at this stage. When you pass false to the MySQL driver, it converts it to a 0. I found this case somewhere in the tests.
Before this change, all parameters to a query would be set to NULL when explaining the query. For both PostgreSQL and MySQL, this caused the explain output to either error or return unhelpful results.
For MySQL, these NULL parameters would cause "Impossible WHERE clause" errors. For PostgreSQL, the EXPLAIN output would return plans without indexes.
The default parameters aren't complete and won't cover all cases. We're working on a fallback mechanism that would allow you to specify the explain parameters values directly. Stay tuned!