risinglightdb / sqllogictest-rs

Sqllogictest parser and runner in Rust.
Apache License 2.0
167 stars 42 forks source link

bin: enhance `skipif` #177

Closed xxchan closed 1 year ago

xxchan commented 1 year ago

We have a skipif, added in #27, which uses DB::engine_name https://github.com/risinglightdb/sqllogictest-rs/blob/27eb9f50993e10b36c1f4f68ad3afe499adbbb49/sqllogictest/src/runner.rs#L914-L919

27 also added a flag --engine. But we use --engine to switch driver now, and the engine_name is not implemented in sqllogictest-bin

https://github.com/risinglightdb/sqllogictest-rs/blob/27eb9f50993e10b36c1f4f68ad3afe499adbbb49/sqllogictest-bin/src/engines.rs#L96-L105

I think adding a new flag like --name or --label can be enough.


Use case here https://github.com/risingwavelabs/risingwave/pull/9013#issuecomment-1526873123

xxchan commented 1 year ago

I think the original use case is “SQL syntax varies between databases” (ref https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki), so the is just driver name. (But this is also break in out sqllogictest-bin now! 😄)

This new use case is that one driver’s different behavior under different configuration. Kind of like postgres+risingwave , postgres+risingwave-in-mem , postgres+cockroach

wangrunji0408 commented 1 year ago

I'm thinking of whether we can decouple onlyif/skipif with engine name, and make it something like conditional compilation in Rust.

Specifically, users can add one or more lables by --cfg <label>, and use them as conditions by skipif <label1> <label2> .... The condition takes effect only if all labels are present. Another syntax may be introduced to express the OR logic.

In the use case above, there could be two labels: risingwave and in-mem. Users can choose any one or both of them to make condition. I think this design allows for more flexibility.

mamcx commented 1 year ago

Also, could be nice to skipif/onlyif for the whole file, like with control