trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
10.43k stars 3k forks source link

SqlDataTypeTest with writes through remote system should always use explicit column type #7218

Open findepi opened 3 years ago

findepi commented 3 years ago

See for example this code block and especially comment at the top of it

https://github.com/trinodb/trino/blob/db68df2cf25b85689c148a497c22951e816d38be/plugin/trino-mysql/src/test/java/io/trino/plugin/mysql/TestMySqlTypeMapping.java#L539-L589

Relying on implicit types for Trino literals is OK, and readable, as Trino behavior is the same in all the tests.

Relying on implicit types for other databases yields test code that's still shorter to read, but causes uncertainty. It's also less copy-proof when we e.g copy test code from MySQL to MemSQL connector.

findepi commented 3 years ago

We should block the usage of implicit types in https://github.com/trinodb/trino/blob/174d4292e37f3782620da6ad2436817526cf0f2c/testing/trino-testing/src/main/java/io/trino/testing/datatype/CreateAndInsertDataSetup.java#L84-L93 when used against a system != Trino.