[feat] introduce classifysql package #284
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #284 +/- ##
==========================================
- Coverage 84.66% 84.02% -0.64%
==========================================
Files 15 15
Lines 1343 1421 +78
==========================================
+ Hits 1137 1194 +57
- Misses 118 131 +13
- Partials 88 96 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dncohen
left a comment
dncohen
left a comment
There was a problem hiding this comment.
FYI, the title made me think the point was a Go interface. Maybe call this "introduce classifysql package" to be more explicit.
LGTM!
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/muir/sqltoken" |
There was a problem hiding this comment.
just noticing "sql" is prefix in "sqltoken" and suffix in "classifysql". Should it be consistent? Should this package be simply "classify"?
classifysql/classify.go
Outdated
| type Dialect int | ||
|
|
||
| const ( | ||
| DialectMySQL Dialect = iota |
There was a problem hiding this comment.
Shouldn't the zero be DialectGeneric or DialectUnknown?
There was a problem hiding this comment.
I added a DialectInvalid as the zero.
muir
changed the title
muir
changed the title
| IsDML: "DML", | ||
| IsNonIdempotent: "NonIdem", | ||
| IsMultipleStatements: "Multi", | ||
| IsEasilyIdempotentFix: "EasyFix", |
There was a problem hiding this comment.
it's cool repo,
This one I am not quite understand, are you going to do some special action to 'fix'/make it idempotent ( I didn't found the fix code yet) ?
This had been internal. Making public. Includes
Regroup()that groups statements together that can be part of the same migration.