search

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.35k stars 2.98k forks source link

Migrate testDeleteWithXXX from BaseConnectorTest to AbstractTestEngineOnlyQueries #13210

Open Praveen2112 opened 2 years ago

Praveen2112 commented 2 years ago

BaseConnectorTest#testDeleteWithXXX verifies delete operation for various patterns of DELETE query - which makes it more suitable to be a part of AbstractTestEngineOnlyQueries

kokosing commented 1 year ago

@Praveen2112 Can you please elaborate? I am not sure if it is the way to go to migrate these tests.

Praveen2112 commented 1 year ago

BaseConnectorTest#testDeleteWithXXX generally deals with sub-queries (scalar, correlated,) which would be independent of the connector implementation - provided it supports DELETE operation

Praveen2112 commented 1 year ago

BaseConnectorTest#testDeleteWithXXX most of the tests validate whether the Trino could execute a specific plan type instead of a applyDelete implementation.

hashhar commented 1 year ago

Note that since complex expression pushdowns have been added some of them do involve connector as well:

You can see it requires per-connector overrides in some connectors.

Praveen2112 commented 1 year ago

Yeah !! But some of them like testDeleteWithSubquery should be agnostic to the connector impl.

Praveen2112 commented 1 year ago

It was more like for changes in the Trino planner and optimizer (wrt to sub-query planning) we do write the test code in one of the connector which shouldn't be the case.