Validation with column types is "wrong way around", DB::run should get column types from SLT

[tv42]

sqllogictest v0.21.0 (and earlier versions, with slightly different APIs) has Validator and ColumnTypeValidator, and the caller is supposed to use those the verify that the results match the expectations.

You're merely allowed to compare types from the SLT and the query against each other, and the stringified values against each other. So let's examine where the observed values come from:

The DB::run and AsyncDB::run methods must return a DBOutput. and to do that the runner has to decide the column type. It does not get the "expected column type" from the SLT file.

I fear this API is impossible to use correctly!

As far as I can tell, sqllogictest actually outputs different stringified results based on what column type the SLT file says. If the column type is I, the output string is an integer even when the underlying SQL datatype was REAL. If the column type is R, the output is a decimal string like 42.001.

Here's some examples from the sqllogictest fossil repository. Note how the stringified result of avg() seems to depend on the stated SLT column type:

query I nosort
SELECT avg(y) FROM t1
----
0

query R nosort
SELECT avg(DISTINCT x) FROM t1
----
1.000

In my runner, if I'm constructing a DBOutput, and my result column is a REAL, I have no way of knowing whether I'm supposed to output decimal or integer values. That information is in the SLT file, but sqllogictest-rs's API does not let me access it.

Am I missing something?

[xxchan]

sqllogictest actually outputs different stringified results based on what column type the SLT file says

This is a new input to me!

You can see how the API evolved from (and how other sqllogictest dialets handle the type stuff) #36:

• At the beginning, the type string isn't checked at all.
• Later, the community added support to compare the return type with the specified type.

Basically, under current design of sqllogictest-rs, the type string corresponds to the SQL query's return type (instead of another type system of the test format). So if we want to test different output formats, we may use cast explicitly, or use some control statements to change the setting.

BTW, in our usage (RisingWave, Postgres-compatible DB (#163)), we don't check type at all. We think just comparing the result string (and cast if needed) is enough.

Feel free to share your use case and propose a design change if needed.

[tv42]

My use case is running the actual upstream sqllogic test suite against a brand new database implementation. There's a lot of value in that test suite.

My proposal is to give stringifier the column type letter from the SLT file. It can give an error on nonsensical expectations, for example when looking at a float: if 'I' then truncate to integer else if 'R' then format with 3 decimal points else return error.

[xxchan]

Sure. PR is welcome.

…

On Fri, 9 Aug 2024 at 09:27, Tv ***@***.***> wrote: My use case is running the actual upstream sqllogic test suite against a brand new database implementation. There's a lot of value in that test suite. My proposal is to give stringifier the column type letter from the SLT file. It can give an error on nonsensical expectations, for example when looking at a float: if 'I' then truncate to integer else if 'R' then format with 3 decimal points else return error. — Reply to this email directly, view it on GitHub <#227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJBQZNJ5FSSV6BZQEO7V6NTZQQLIHAVCNFSM6AAAAABKMEXOT2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZWHE4DGMRVGU> . You are receiving this because you commented.Message ID: ***@***.***>