should `control sortmode` be file-level or global?

[xxchan]

One more thing. Do you feel set VARIABLE in sqllogictest is a file-level or statement or a statement affecting global state? Now it's implemented as latter, so if someone do:

include a.slt include b.slt

If a.slt sets a row sort mode, b.slt will also be affected. If you feel this is counter-intuitive, maybe we can change the behavior of sqllogictest.

Originally posted by @skyzh in https://github.com/singularity-data/risingwave/issues/3582#issuecomment-1171528000

[wangrunji0408]

+1 for file-level

[xxchan]

btw, this includes slt's internal config (control sortmode), and DB session's config (SET xxx) 🤔

It may be confusing if we only let one of them file-level. But the latter one seems tricky if we want to let include inherit from parent file, but isolated from other include. We cannot inherit session..?

control sortmode rowsort
SET RW_IMPLICIT_FLUSH TO false;

include a.slt
# have these inside
# control sortmode nosort
# SET RW_IMPLICIT_FLUSH TO true;

include b.slt

[xxchan]

Actually include by design propagates side-effect (for CREATE, INSERT). So if the user forgets to clean up (DELETE) in a.slt, they will also have problem in b.slt. But this isn't a problem since CREATE in b.slt will have error and reminds the user.

For variable side-effects, it's more error-prone.

[skyzh]

We can only allow control statements in top-level files 😋

[skyzh]

I remembered that LaTeX have both \input and \include macros. I think the include is more like \input 🤣

[skyzh]

And test cases should be as short as possible. I believe current RisingWave’s batch test is a bad example. Include should be used to include things in common, instead of including new test cases. This is where the confusion originates: each test case share a different session level setting. By the way, parallelism is exploited on file level, which means that sqllogictest cannot parallelize RisingWave’s batch test.

[xxchan]

We can have a style suggestion document first 🥸

[xxchan]

When file-level behavior is expected, I guess a different semantics for include (described here https://github.com/singularity-data/risingwave/issues/3629#issuecomment-1215625871) is actually expected.

[BugenZhao]

We may treat the root file with its inclusions as a single session, and make the control session-level, which will be consistent with the database's session configurations if we use different sessions per-file.

[xxchan]

We may treat the root file with its inclusions as a single session, and make the control session-level, which will be consistent with the database's session configurations if we use different sessions per-file.

Isn't this already the current behavior? (... for parallel mode -j1)