melgenek commented on issue #4499: URL: https://github.com/apache/arrow-datafusion/issues/4499#issuecomment-1421387233
Hey, guys! I'd like to work on this issue, but I am not sure which way to go. DuckDB suggests using 'I' just to verify the number of columns. Sqllogic test defines 'T'(text), 'I' (integer), 'R' (floating number). [CocroachDB](https://github.com/cockroachdb/cockroach/blob/899c9112188c22ec140adbeb0cf7d5761da3827d/pkg/sql/logictest/logic.go#L282-L289) defines even more types `T`, `F`(floating), `R` (decimal), `B` (boolean), `O` (oid), `_` (any). I think having more than one type is useful. For example, in order to verify that `null and null` produce a NULL of type boolean, I needed to use `pg_type` and `arrow_type` to verify the result types. https://github.com/apache/arrow-datafusion/blob/7d2d51b651d076d720e2879661346babcda9734a/datafusion/core/tests/sqllogictests/test_files/pg_compat/pg_compat_type_coercion.slt#L98-L115 However, having a boolean type would've allowed writing a more concise test which would run for both Datafusion and Postgres ``` query BB select null and null, null or null; ---- NULL NULL ``` Do you have preferences regarding the set of types? ------ `sqllogictest-rs` doesn't have type validation now, but defines columns `I` (integer), `T` (text), `R` (floating point), `?` (any). The `?` type skips validation for a column. In order to use `sqllogictest-rs` one would need to define a type and a validation strategy. This would likely mean aligning with the other users of `sqllogictest-rs`. I thought it would be more flexible to have a pluggable type and a validation strategy, so I opened a pr in `sqllogictest-rs` https://github.com/risinglightdb/sqllogictest-rs/pull/160. I would appreciate your feedback. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
