Repository navigation
Add support for PostgreSQL DROP RULE - #2434
LucaCappelletti94 wants to merge 5 commits into
Conversation
ac7a7bd to
8cf512e
Compare
defd2dd to
5285729
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2434 +/- ##
==========================================
+ Coverage 82.67% 82.69% +0.01%
==========================================
Files 42 42
Lines 35737 35770 +33
Branches 35737 35770 +33
==========================================
+ Hits 29547 29580 +33
Misses 2671 2671
Partials 3519 3519 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
a37340d to
2071d91
Compare
There was a problem hiding this comment.
Like it, Span especially
Could you add a few more red tests, e.g.:
#[test]
fn parse_drop_rule_rejects_lists_and_extra_options() {
for (sql, expected) in [
("DROP RULE r1, r2 ON t", "Expected: ON, found: ,"),
("DROP RULE s.r ON t", "Expected: ON, found: ."),
("DROP RULE 1 ON t", "Expected: identifier, found: 1"),
("DROP RULE IF EXISTS", "Expected: identifier, found: EOF"),
("DROP RULE IF NOT EXISTS r ON t", "Expected: ON, found: NOT"),
("DROP RULE r t", "Expected: ON, found: t"),
("DROP RULE r ON (t)", "Expected: identifier, found: ("),
("DROP RULE r ON t1, t2", "Expected: end of statement, found: ,"),
("DROP RULE r ON t ON t", "Expected: end of statement, found: ON"),
("DROP RULE r ON t CASCADE RESTRICT", "Expected: end of statement, found: RESTRICT"),
("DROP RULE r ON t RESTRICT CASCADE", "Expected: end of statement, found: CASCADE"),
("DROP RULE r ON t PURGE", "Expected: end of statement, found: PURGE"),
] {
let err = all_dialects().parse_sql_statements(sql).unwrap_err().to_string();
assert!(err.starts_with(&format!("sql parser error: {expected}")), "{sql}: {err}");
}
}| } | ||
| } | ||
|
|
||
| impl Spanned for DropRule { |
There was a problem hiding this comment.
Added all the tests, but I have not understood what is there to like in particular of the Span? Unfortunately most of the spans are objectively to be refactored, likely including this one, but that is something I will only touch at some point after the tokenizer refactoring
There was a problem hiding this comment.
Mainly just that Spans weren’t forgotten 😅
Agreed that Spans and error handling both have room for improvement
ff7b36c to
8ecfce3
Compare
a9cdffc to
f6c14f6
Compare
The parser does not recognize
DROP RULE, so migration scripts and schema dumps that remove rewrite rules fail to parse. This is the companion of #2433, and the two are independent and can land in either order.Doc: https://www.postgresql.org/docs/current/sql-droprule.html