Conversation
EffectiveDialect is the single dialect input rule evaluation will consult, kept distinct from the parser dialect carried by InputSource. The lock-risk rules added in Phase 4 require a concrete Postgres or MySQL setting and skip otherwise, so is_lock_risk_capable() is exposed upfront for callers and the rule dispatcher to gate evaluation. The type bridges to SqlDialect via From/Into in both directions so the existing CLI / TOML / wasm dialect inputs flow through unchanged.
Register risk/add-index-on-large-table, risk/add-fk-on-existing, risk/alter-column-type, and risk/rewrite-table on ReviewRuleId, all with the Caution default severity. The variants are appended to all_rules() so existing CLI listing / fixture / metadata ordering stays stable while the rule catalog grows from 8 to 12 entries. The actual detection logic and dialect gating land in follow-up work; this commit only wires up identifiers, descriptions, and the catalog-size assertions in the CLI integration tests.
DialectScope is a pub(crate) sibling to ReviewSeverity that the rule dispatcher will consult to silently filter lock-risk rules out when the effective dialect does not match. The helper is gated to the crate so it does not leak into ReviewRuleMetadata: the public listing surface still emits every rule, while skip semantics live behind a per-request runtime decision. Eight Phase 1-3 rules report DialectScope::Any, the PG/MySQL trio reports OneOf both, and the MySQL-only rewrite-table rule reports OneOf the single dialect. The helper is dead-code allowed until the run_rules wiring that consumes it lands in a follow-up.
- Reword AddIndexOnLargeTable description so the non-CONCURRENT/INPLACE build clause reads naturally and aligns with the list-rules wording. - Pin default_severity and description for each new lock-risk rule (AddIndexOnLargeTable, AddFkOnExisting, AlterColumnType, RewriteTable) in the JSON list-rules integration test rather than only checking the rule count.
Code Metrics Report
Details | | main (ce7084c) | #122 (771f74f) | +/- |
|---------------------|----------------|----------------|-------|
- | Coverage | 94.6% | 94.6% | -0.1% |
| Files | 81 | 81 | 0 |
| Lines | 37324 | 37420 | +96 |
+ | Covered | 35344 | 35431 | +87 |
| Test Execution Time | 1m34s | 1m34s | 0s |Code coverage of files in pull request scope (94.7% → 93.6%)
Reported by octocov |
Schema reviewTip ✅ No risk findings — schema changes look safe to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EffectiveDialectso review rules can reason about a concrete dialect instead ofSqlDialect::Auto, with bidirectionalFromimpls and anis_lock_risk_capable()predicate that gates lock-risk evaluation to PostgreSQL/MySQL.ReviewRuleIdvariants for upcoming lock-risk rules (AddIndexOnLargeTable,AddFkOnExisting,AlterColumnType,RewriteTable) withcautiondefaults and dialect-aware descriptions; surface them via--list-rulestext/JSON.DialectScopehelper andReviewRuleId::dialect_scope()so the dispatcher can skip rules whose dialect scope does not match, ready to wire in the follow-up PR.Changes
EffectiveDialectenum mirroringSqlDialectplusFromround-trips andDefault == Auto.is_lock_risk_capable()so future rule dispatchers can fast-path to no-op for non-lock-risk dialects.EffectiveDialectfromrelune-corefor downstream crates.AddIndexOnLargeTable,AddFkOnExisting,AlterColumnType,RewriteTablevariants with stablerisk/...ids,cautiondefaults, and dialect-flavored descriptions.as_str/parse/default_severity/description/all_rules/all_metadataso--list-rulestext and JSON outputs cover them.pub(crate)DialectScopeenum (AnyvsOneOf(&'static [SqlDialect])) and aconst fn dialect_scope()mapping each rule id to its scope.Any, scope the new lock-risk rules to PostgreSQL/MySQL, and restrictRewriteTableto MySQL.#[allow(dead_code)]while the dispatcher wiring lands in the follow-up PR.AddIndexOnLargeTabledescription so the non-CONCURRENT/INPLACE clause reads naturally and matches the list-rules wording.default_severityanddescriptionfor each new lock-risk rule in the JSON list-rules integration test instead of only checking the rule count.