readyset icon indicating copy to clipboard operation
readyset copied to clipboard

nom-sql: Add support for DROP SNAPSHOT FROM table

Open altmannmarcelo opened this issue 1 year ago • 4 comments

Add support for DROP SNAPSHOT query. The underlining adaptor code is not yet implemented.

Relates #456

altmannmarcelo avatar Sep 12 '23 22:09 altmannmarcelo

$ cargo test --package nom-sql --lib -- drop::tests::parse_drop_snapshot_query parser::tests::drop_snapshot  --exact --nocapture
    Finished test [unoptimized + debuginfo] target(s) in 0.19s
warning: the following packages contain code that will be rejected by a future version of Rust: criterion v0.3.5, structmeta-derive v0.1.4
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 4`
     Running unittests src/lib.rs (target/debug/deps/nom_sql-46b9931b5f12b1ca)

running 2 tests
test drop::tests::parse_drop_snapshot_query ... ok
test parser::tests::drop_snapshot ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 505 filtered out; finished in 0.00s

altmannmarcelo avatar Sep 12 '23 22:09 altmannmarcelo

Some bikeshed thoughts: how about ALTER TABLE <table> RESNAPSHOT?

@aglover got any opinions?

glittershark avatar Sep 15 '23 14:09 glittershark

@altmannmarcelo fyi - we're going to be writing a "SQL extension philosophy" doc at some point soonish which will likely affect the specific syntax here.

glittershark avatar Sep 15 '23 18:09 glittershark

@glittershark Sure, once that is finalized I can adjust the PR. DROP SNAPSHOT FROM table indeed might not be the best syntax here. Perhaps READYSET keyword should be mandatory for the new syntax when dealing with RS only statements.

altmannmarcelo avatar Sep 15 '23 18:09 altmannmarcelo