- Notifications
You must be signed in to change notification settings - Fork976
Expanding lint rules to parsed migrations#2512
-
Thelint rule functionality recently added is awesome! Would you consider expanding this beyond Since sqlc already parses various migration files one could also use the sqlc lint rules to vet migrations. rules: -name:no-drop-tablemessage:"never drop tables"rule:| migration.contains("DROP TABLE") |
BetaWas this translation helpful?Give feedback.
All reactions
Replies: 1 comment 3 replies
-
We could expand the scope of the rules: -name:no-drop-tablemessage:"never drop tables"rule:| query.contains("DROP TABLE") Would you expect to have a way to easily distinguish whether a rule will apply to a "query" (all |
BetaWas this translation helpful?Give feedback.
All reactions
-
I think I'd like to distinguish between I could see the argument where folks might want to apply a rule to both, and it'd be "annoying" to have to define duplicate rules for "query" vs "schema". But this might be an edge case. For some context, I was recently evaluating migration-specific linters like:
But given |
BetaWas this translation helpful?Give feedback.
All reactions
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
-
That makes sense. I think it's a good idea and probably not that hard to add. I haven't looked at the way sqlc handles schema migration parsing though. Even if we provide access via a single variable like rules:-name:no-drop-tablemessage:"never drop tables"rule:| statement.contains("DROP TABLE")if:!statement.is_query In any case I'll try to find a little time to implement a first version this morning to see what roadblocks I run into. |
BetaWas this translation helpful?Give feedback.
All reactions
-
Coming back to this, there are a few questions due to the way sqlc currently handles DDL statements. We parse them all and then "apply" them to aninternal database If the most important thing is to just run some simple text matching against DDL statement strings, I think that's pretty easy. sqlc can keep track of the original strings after parsing and hand those over to the CEL environment. I don't know how useful that is though, so I'm hoping to have some folks weigh in. We could also try to keep track of the AST from the parse step and pass that into the vet CEL context, but we'd need to massage the AST into a proto somehow. |
BetaWas this translation helpful?Give feedback.